Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove ability to edit close and lottery dates on a published listing #623

Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
49ae1b7
fix: block editing listing dates by non admin
KrissDrawing May 7, 2024
929d99b
fix: block editing listing dates by non admin backend
KrissDrawing May 14, 2024
3ec5e81
Merge remote-tracking branch 'refs/remotes/origin/main' into 570/Remo…
KrissDrawing May 14, 2024
4c99b10
fix: tests
KrissDrawing May 14, 2024
106b58d
Merge remote-tracking branch 'refs/remotes/origin/main' into 570/Remo…
KrissDrawing May 15, 2024
ec0b22b
test: adjust tests to new requirements
KrissDrawing May 16, 2024
499a759
Merge remote-tracking branch 'refs/remotes/origin/main' into 570/Remo…
KrissDrawing May 21, 2024
ef56b69
refactor: add helper function to check if dates changed
KrissDrawing Jun 4, 2024
ea6b4d7
refactor: use enum for active status
KrissDrawing Jun 4, 2024
ec8577b
fix: disable listing availability switch
KrissDrawing Jun 20, 2024
4128713
Merge remote-tracking branch 'refs/remotes/origin/main' into 570/Remo…
KrissDrawing Jun 20, 2024
681ca76
test: adjust edit object to match requirements
KrissDrawing Jun 25, 2024
5f1029a
test: remove dates to update listing
KrissDrawing Jun 25, 2024
2d8e7f7
Merge remote-tracking branch 'refs/remotes/origin/main' into 570/Remo…
KrissDrawing Jun 26, 2024
117dc0f
fix: format props
KrissDrawing Jun 26, 2024
d76d876
fix: typo
KrissDrawing Jun 26, 2024
e118250
fix: show selected value for blocked fields
KrissDrawing Jun 27, 2024
febc129
Merge remote-tracking branch 'refs/remotes/origin/main' into 570/Remo…
KrissDrawing Jun 27, 2024
5bbec72
Merge remote-tracking branch 'refs/remotes/origin/main' into 570/Remo…
KrissDrawing Jul 1, 2024
d3dde22
fix: remove redundant password
KrissDrawing Jul 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions api/src/services/listing.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ConfigService } from '@nestjs/config';
import { SchedulerRegistry } from '@nestjs/schedule';
import {
LanguagesEnum,
ListingEventsTypeEnum,
ListingsStatusEnum,
Prisma,
ReviewOrderTypeEnum,
Expand Down Expand Up @@ -48,6 +49,7 @@ import { startCronJob } from '../utilities/cron-job-starter';
import { PermissionService } from './permission.service';
import { permissionActions } from '../enums/permissions/permission-actions-enum';
import Unit from '../dtos/units/unit.dto';
import { checkIfDatesChanged } from '../utilities/listings-utilities';

export type getListingsArgs = {
skip: number;
Expand Down Expand Up @@ -1138,6 +1140,32 @@ export class ListingService implements OnModuleInit {
*/
async update(dto: ListingUpdate, requestingUser: User): Promise<Listing> {
const storedListing = await this.findOrThrow(dto.id, ListingViews.details);
const isNonAdmin = requestingUser?.userRoles?.isAdmin === false;
KrissDrawing marked this conversation as resolved.
Show resolved Hide resolved
const isActiveListing = dto.status === 'active';
KrissDrawing marked this conversation as resolved.
Show resolved Hide resolved

//check if the user has permission to edit dates
if (isNonAdmin && isActiveListing) {
const lotteryEvent = dto.listingEvents?.find(
(event) => event?.type === ListingEventsTypeEnum.publicLottery,
);
const storedLotteryEvent = storedListing.listingEvents?.find(
(event) => event?.type === ListingEventsTypeEnum.publicLottery,
);

if (
KrissDrawing marked this conversation as resolved.
Show resolved Hide resolved
checkIfDatesChanged(
lotteryEvent,
storedLotteryEvent,
dto,
storedListing.applicationDueDate?.toISOString(),
)
) {
throw new HttpException(
'You do not have permission to edit dates',
403,
);
}
}

await this.permissionService.canOrThrow(
requestingUser,
Expand Down Expand Up @@ -1579,7 +1607,7 @@ export class ListingService implements OnModuleInit {
clears the listing cache of either 1 listing or all listings
@param storedListingStatus the status that was stored for the listing
@param incomingListingStatus the incoming "new" status for a listing
@param savedResponseId the id of the listing
@param savedResponseId the id of the listing
*/
async cachePurge(
storedListingStatus: ListingsStatusEnum | undefined,
Expand Down Expand Up @@ -1703,7 +1731,7 @@ export class ListingService implements OnModuleInit {

/**
marks the db record for this cronjob as begun or creates a cronjob that
is marked as begun if one does not already exist
is marked as begun if one does not already exist
*/
async markCronJobAsStarted(): Promise<void> {
const job = await this.prisma.cronJob.findFirst({
Expand Down
37 changes: 37 additions & 0 deletions api/src/utilities/listings-utilities.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { ListingEventsTypeEnum } from '@prisma/client';
import { ListingUpdate } from '../dtos/listings/listing-update.dto';
import { ListingEventCreate } from '../dtos/listings/listing-event-create.dto';
import { ListingEvent } from '../dtos/listings/listing-event.dto';

export const checkIfDatesChanged = (
lotteryEvent: ListingEventCreate,
storedLotteryEvent: ListingEvent,
dto: ListingUpdate,
storedApplicationDueDate: string,
) => {
const isPublicLotteryEvent =
lotteryEvent?.type === ListingEventsTypeEnum.publicLottery;

const isSameStartDate =
lotteryEvent?.startDate?.toISOString() ===
storedLotteryEvent?.startDate?.toISOString();
const isSameStartTime =
lotteryEvent?.startTime?.toISOString() ===
storedLotteryEvent?.startTime?.toISOString();
const isSameEndTime =
lotteryEvent?.endTime?.toISOString() ===
storedLotteryEvent?.endTime?.toISOString();

const isDifferentEventType = lotteryEvent?.type !== storedLotteryEvent?.type;
const isDifferentApplicationDueDate =
dto.applicationDueDate?.toISOString() !== storedApplicationDueDate;
const isDifferentLotteryEventTimes =
isPublicLotteryEvent &&
!(isSameStartDate && isSameStartTime && isSameEndTime);

return (
isDifferentEventType ||
isDifferentLotteryEventTimes ||
isDifferentApplicationDueDate
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,7 @@ describe('Testing Permissioning of endpoints as Jurisdictional Admin in the corr
});

const val = await constructFullListingData(prisma, listing.id, jurisId);
val.applicationDueDate = listing.applicationDueDate;

await request(app.getHttpServer())
.put(`/listings/${listing.id}`)
Expand Down Expand Up @@ -1230,7 +1231,7 @@ describe('Testing Permissioning of endpoints as Jurisdictional Admin in the corr

it('should succeed for process endpoint', async () => {
/*
Because so many different iterations of the process endpoint were firing we were running into collisions.
Because so many different iterations of the process endpoint were firing we were running into collisions.
Since this is just testing the permissioning aspect I'm switching to mocking the process function
*/
applicationFlaggedSetService.process = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,7 @@ describe('Testing Permissioning of endpoints as partner with correct listing', (
userListingId,
jurisId,
);
val.applicationDueDate = new Date('05-16-2024 01:25:18PM GMT+2');

await request(app.getHttpServer())
.put(`/listings/${userListingId}`)
Expand Down Expand Up @@ -1143,7 +1144,7 @@ describe('Testing Permissioning of endpoints as partner with correct listing', (

it('should succeed for process endpoint', async () => {
/*
Because so many different iterations of the process endpoint were firing we were running into collisions.
Because so many different iterations of the process endpoint were firing we were running into collisions.
Since this is just testing the permissioning aspect I'm switching to mocking the process function
*/
applicationFlaggedSetService.process = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type ListingFormProps = {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const ListingForm = ({ listing, editMode }: ListingFormProps) => {
const defaultValues = editMode ? listing : formDefaults
const isListingActive = listing?.status === ListingsStatusEnum.active
const formMethods = useForm<FormListing>({
defaultValues,
shouldUnregister: false,
Expand Down Expand Up @@ -150,7 +151,7 @@ const ListingForm = ({ listing, editMode }: ListingFormProps) => {
newData?: Partial<FormListing>
) => {
if (confirm && status === ListingsStatusEnum.active) {
if (listing?.status === ListingsStatusEnum.active) {
if (isListingActive) {
setListingIsAlreadyLiveModal(true)
} else {
setPublishModal(true)
Expand Down Expand Up @@ -368,14 +369,18 @@ const ListingForm = ({ listing, editMode }: ListingFormProps) => {
</div>
</TabPanel>
<TabPanel>
<RankingsAndResults listing={listing} />
<RankingsAndResults
listing={listing}
disableDueDates={isListingActive && !profile.userRoles.isAdmin}
/>
<LeasingAgent />
<ApplicationTypes listing={listing} />
<ApplicationAddress listing={listing} />
<ApplicationDates
listing={listing}
openHouseEvents={openHouseEvents}
setOpenHouseEvents={setOpenHouseEvents}
disableDueDate={isListingActive && !profile.userRoles.isAdmin}
/>

<div className="-ml-8 -mt-8 relative" style={{ top: "7rem" }}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ type ApplicationDatesProps = {
openHouseEvents: TempEvent[]
setOpenHouseEvents: (events: TempEvent[]) => void
listing?: FormListing
disableDueDate?: boolean
}

const ApplicationDates = ({
listing,
openHouseEvents,
setOpenHouseEvents,
disableDueDate,
}: ApplicationDatesProps) => {
const openHouseHeaders = {
date: "t.date",
Expand Down Expand Up @@ -114,7 +116,7 @@ const ApplicationDates = ({
register={register}
watch={watch}
note={t("listings.whenApplicationsClose")}
disabled={enableDueDate === YesNoEnum.no}
disabled={disableDueDate || enableDueDate === YesNoEnum.no}
defaultDate={{
month: listing?.applicationDueDate
? dayjs(new Date(listing?.applicationDueDate)).format("MM")
Expand All @@ -135,7 +137,7 @@ const ApplicationDates = ({
id={"applicationDueTimeField"}
register={register}
watch={watch}
disabled={enableDueDate === YesNoEnum.no}
disabled={disableDueDate || enableDueDate === YesNoEnum.no}
defaultValues={{
hours: listing?.applicationDueDate
? dayjs(new Date(listing?.applicationDueDate)).format("hh")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ import SectionWithGrid from "../../../shared/SectionWithGrid"

type RankingsAndResultsProps = {
listing?: FormListing
disableDueDates?: boolean
}

const RankingsAndResults = ({ listing }: RankingsAndResultsProps) => {
const RankingsAndResults = ({ listing, disableDueDates }: RankingsAndResultsProps) => {
const formMethods = useFormContext()

// eslint-disable-next-line @typescript-eslint/unbound-method
Expand Down Expand Up @@ -79,13 +80,15 @@ const RankingsAndResults = ({ listing }: RankingsAndResultsProps) => {
label: t("listings.firstComeFirstServe"),
value: "reviewOrderFCFS",
id: "reviewOrderFCFS",
disabled: disableDueDates,
defaultChecked:
listing?.reviewOrderType === ReviewOrderTypeEnum.firstComeFirstServe,
},
{
label: t("listings.lotteryTitle"),
value: "reviewOrderLottery",
id: "reviewOrderLottery",
disabled: disableDueDates,
defaultChecked: listing?.reviewOrderType === ReviewOrderTypeEnum.lottery,
},
]}
Expand All @@ -105,11 +108,13 @@ const RankingsAndResults = ({ listing }: RankingsAndResultsProps) => {
{
...yesNoRadioOptions[0],
id: "dueDateQuestionYes",
disabled: disableDueDates,
defaultChecked: listing && listing.applicationDueDate !== null,
},
{
...yesNoRadioOptions[1],
id: "dueDateQuestionNo",
disabled: disableDueDates,
defaultChecked: listing && !listing.applicationDueDate,
},
]}
Expand All @@ -127,6 +132,7 @@ const RankingsAndResults = ({ listing }: RankingsAndResultsProps) => {
id={"lotteryDate"}
register={register}
watch={watch}
disabled={disableDueDates}
defaultDate={{
month: lotteryEvent?.startDate
? dayjs(new Date(lotteryEvent?.startDate)).utc().format("MM")
Expand All @@ -147,6 +153,7 @@ const RankingsAndResults = ({ listing }: RankingsAndResultsProps) => {
id={"lotteryStartTime"}
register={register}
watch={watch}
disabled={disableDueDates}
defaultValues={{
hours: lotteryEvent?.startTime
? dayjs(new Date(lotteryEvent?.startTime)).format("hh")
Expand All @@ -168,6 +175,7 @@ const RankingsAndResults = ({ listing }: RankingsAndResultsProps) => {
id={"lotteryEndTime"}
register={register}
watch={watch}
disabled={disableDueDates}
defaultValues={{
hours: lotteryEvent?.endTime
? dayjs(new Date(lotteryEvent?.endTime)).format("hh")
Expand Down
Loading