1430/listings management, fcfs and lottery #1485
Conversation
emilyjablonski
added
the
wip
|
✔️ Deploy Preview for jovial-davinci-1d67a0 ready! 🔨 Explore the source changes: 5af5a35 🔍 Inspect the deploy log: https://app.netlify.com/sites/jovial-davinci-1d67a0/deploys/60f0caa4bf4ee100084b782b 😎 Browse the preview: https://deploy-preview-1485--jovial-davinci-1d67a0.netlify.app |
|
✔️ Deploy Preview for dev-storybook-bloom ready! 🔨 Explore the source changes: 5af5a35 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-storybook-bloom/deploys/60f0caa450bc2d0008cb9704 😎 Browse the preview: https://deploy-preview-1485--dev-storybook-bloom.netlify.app |
|
✔️ Deploy Preview for dev-bloom ready! 🔨 Explore the source changes: 5af5a35 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-bloom/deploys/60f0caa46d169d0008270fef 😎 Browse the preview: https://deploy-preview-1485--dev-bloom.netlify.app |
|
❌ Deploy Preview for dev-partners-bloom failed. 🔨 Explore the source changes: 5af5a35 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-partners-bloom/deploys/60f0caa457448d00076d575c |
emilyjablonski
force-pushed
the
1430/lm-fcfs-and-lottery
branch
from
7edb8d6
to
40d443e
Compare
emilyjablonski
added
ready for review
and removed
wip
|
Large prettier diff is making it hard to review, taking a look at that |
emilyjablonski
force-pushed
the
1430/lm-fcfs-and-lottery
branch
from
0b19ad0
to
0c52627
Compare
There was a problem hiding this comment.
@emilyjablonski ,
Overall this looks good. The main change that's needed is how reviewOrderType is defined in the backend. Please see my comment on that.
My machine is probably a little slow at the moment, but it does seem to take a long time to switch between the first come first serve and lottery options in the form. If it seems a little slow to you too, consider moving those pieces to a memoized component(s).
| @Expose() | ||
| @IsOptional({ groups: [ValidationsGroupsEnum.default] }) | ||
| @ApiProperty() | ||
| get reviewOrderType(): ListingReviewOrder { | ||
| let lotteryEventExists = false | ||
| this.events.forEach((event) => { | ||
| if (event.type === ListingEventType.publicLottery) { | ||
| lotteryEventExists = true | ||
| } | ||
| }) | ||
| return lotteryEventExists ? ListingReviewOrder.lottery : ListingReviewOrder.firstComeFirstServe | ||
| } | ||
|
|
There was a problem hiding this comment.
I think this should be moved to the listing DTO. See the other definitions in there with the @Transform decorator. Also, with the way it is now generate client doesn't properly add reviewOrderType or ListingReviewOrder, which causes issues, so the definition should include reviewOrderType: ListingReviewOrder.
|
@emilyjablonski Could you please try to revert the formatting commits? I can make it a separate PR. |
emilyjablonski
added
wip
…ousing/bloom into 1430/lm-fcfs-and-lottery
Pull Request Template
Issue
Addresses #1430
Description
Adds the lottery and first come first serve radio button selection to the rankings and results portion of listings management.
If you select FCFS, an additional radio question
Is there an application due date?will appear - if you selectNo, the application due date field at the bottom of the page should become disabledIf you select Lottery, three additional fields for the date, start time, and end time, as well as a textbox field for notes should appear
I added a computed backend field to be able to tell if a listing is FCFS.
Type of change
How Can This Be Tested/Reviewed?
Add a new listing with FCFS and no due date, and check that the public listing displays "First come first serve"
Add a new listing with Lottery and check out the additional lottery section with the time range and notes in the right hand panel
Checklist: