feat: listings management draft and publish validation backend & frontend #1850

emilyjablonski · 2021-09-17T18:53:15Z

Pull Request Template

Issue

This change addresses the issue in full
This change addresses only certain aspects of the issue
This change is a dependency for another issue
This change has a dependency from another issue

Description

Adds the backend and frontend pieces for validations on listings management on both draft and publish. I also refactored a little how we are submitting data so we don't need to keep track of status or submitData in state.

For draft, required fields are:

Listing name
Jurisdiction (only shows if user has more than one, otherwise defaults)

For publish, required fields are:

Listing name
Jurisdiction
Developer
Image
Is there a digital application?
Is there a paper application?
Is this a referral opportunity?
Building Address (specifically street, city, state, zipCode)
Units - At least one unit
Deposit Min (has a default)
Deposit Max (has a default)
Rental Assistance (has a default)
How is the application review order determined? (has a default)
Is the waitlist open?
Leasing agent email
Leasing agent name
Leasing agent phone

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Prototype/POC (not to merge)
This change is a refactor/address technical debt
This change requires a documentation update
This change requires a SQL Script

How Can This Be Tested/Reviewed?

Create a new listing from scratch and without filling anything out and try to save as draft and publish and the frontend validations should kick in. Also try to edit an existing listing.

The one kinda wonky frontend validation is buildingAddress, because if you send nothing we get a generic "buildingAddress" error but if at least one field is filled out we get errors specific to the four required fields. Should show as individual errors tho.

Checklist:

backend/core/src/listings/dto/listing-published-create.dto.ts

backend/core/src/shared/entities/address.entity.ts

netlify · 2021-09-21T00:34:55Z

✔️ Deploy Preview for dev-bloom ready!

🔨 Explore the source changes: 45c3aa3

🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-bloom/deploys/6158d072d540b70008e5cf3f

😎 Browse the preview: https://deploy-preview-1850--dev-bloom.netlify.app

netlify · 2021-09-21T00:34:55Z

✔️ Deploy Preview for dev-storybook-bloom ready!

🔨 Explore the source changes: 45c3aa3

🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-storybook-bloom/deploys/6158d072ead1500008dedc6d

😎 Browse the preview: https://deploy-preview-1850--dev-storybook-bloom.netlify.app

netlify · 2021-09-21T00:34:55Z

✔️ Deploy Preview for dev-partners-bloom ready!

🔨 Explore the source changes: 45c3aa3

🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-partners-bloom/deploys/6158d072da94050007060a2f

😎 Browse the preview: https://deploy-preview-1850--dev-partners-bloom.netlify.app

sites/partners/src/listings/PaperListingForm/index.tsx

dominikx96

@emilyjablonski

1. I think the error message should be different than generic:
#1542
"Please resolve any errors before publishing your listing."

2. Noticed a problem when I trigger errors and then fill the form with correct values:
See video: https://share.getcloudapp.com/NQuYXn1R
So the correct jurisdiction value has been removed and I still see an error.

3. When I put a user agent phone, then the phone should be validated using phone regex
Currently, I am able to put a phone with the incorrect format: https://share.getcloudapp.com/o0uP01R0

4. "Is the waitlist open?" field is not validated
https://share.getcloudapp.com/d5uR5QgB

5. Lottery fields
When I choose the lottery option, I suppose date fields also should be required (?)
https://share.getcloudapp.com/nOuvO476

6. Add unit
When I tried to add a new unit type (I've just selected unit type "studio" from the select field, other fields were empty), I've noticed the error: https://share.getcloudapp.com/wbubgOgw

7. Edit and delete unit don't work
Nothing happened after click
https://share.getcloudapp.com/E0ujvN1B

8. Building address validation
https://share.getcloudapp.com/NQuYXn84

When I filled only street address, other errors have gone:
https://share.getcloudapp.com/9ZuQp65e

9. Application types validation doesn't work:
https://share.getcloudapp.com/bLuqL21x

emilyjablonski · 2021-09-23T23:14:14Z

@dominikx96

Everything is fixed except for 6 which is waiting on a product update / followup ticket :)

Good catch, updated
This was an existing problem with the Jurisdiction select before this PR. It would occasionally just….not select? But I fixed it by removing the Controller.
We actually don’t have any existing backend validations for phone number, elsewhere in our apps where we validate phone number fields we must be validating on the frontend. I added a backend phone number validator for this field though so it works now!
Oops! Missed this because we assign it “false” on submit once all other fields are validated. I removed this default value so we can tell if they actually clicked the field yes/no.
They shouldn’t be, we actually made a fix a few weeks ago so that they wouldn’t be required because apparently often leasing agents won’t know the lottery date yet on listing publish, but thanks for asking!
My bad, didn’t expect anything in units to be validating yet, and I always submitted units with an AMI chart! You can now submit a unit without an AMI chart. Unclear if AMI on every unit should be required on publish of a listing. I’m still waiting to hear back from Kathy/Jesse on any info concerning what should be required in the unit form, and I think ideally that’s handled in a separate ticket since we’ll be doing a sort of pre-publish backend call to validate.
Similar to 6. Edit and delete now work.
The screenshot was expected behavior and I mentioned this a little in the PR description, but I agree w your feedback that it’s weird and I refactored it a little so now I think it probably meets what you would expect.
Same as 4. Fixed!

Ty again for this ✨ amazing ✨ review! :D

dominikx96

@emilyjablonski

I still have a problem with the phone number, it's validated, but then I trigger an error (e.g. phone field is empty) - then fill it correctly, I'm still facing the error:
https://share.getcloudapp.com/v1u07GWO
When I'm trying to save as a draft, I see building address errors (I think they shouldn't be required in the draft state):
https://share.getcloudapp.com/L1urAldZ
When I'm editing a listing and triggering publish action (with some issues, empty fields), then listing unit actions (edit, delete) don't work.

Otherwise looks good to me, it's a large amount of great work, @emilyjablonski ! 💪

emilyjablonski · 2021-09-27T15:35:12Z

@dominikx96 Thank you again 🙏

I can't repro this :( Will slack you
Fixed! Introduced by latest rebase, hadn't seen that
Discovered there's even more issues beyond that with units while testing this. The units will straight up just disappear sometimes while editing. Testing / fixing this now.

emilyjablonski · 2021-09-27T19:13:18Z

Okay the unit issue is fixed! And as far as the phone number issue, the number 444-444-4444 fails (and it does too in the validator's official demo so it is supposed to http://libphonenumber.appspot.com/phonenumberparser?number=4444444444&country=US) and I believe this is because it is validating that the phone number is a valid US phone number down the area code, and 444 isn't a valid US area code. If I put in for example my phone number, it works. I assume we want to restrict to US phone numbers?

dominikx96 · 2021-09-28T15:02:48Z

@emilyjablonski you're right, I tried with the US number and it works better than I expected! :D

I noticed one thing, building address probably should be optional for draft, but I still see errors:
https://share.getcloudapp.com/6quYgoZ7

emilyjablonski · 2021-09-28T15:13:59Z

@dominikx96 I can't repro that, will slack you

dominikx96 · 2021-09-28T16:02:19Z

@dominikx96 I can't repro that will slack you

As we discussed at the meeting, it was my fault, I forgot to fetch changes, LGTM! 👍

seanmalbert

@emilyjablonski ,
Took an initial pass and added some comments. Going to finish review in the morning.

backend/core/src/shared/url-helper.ts

sites/partners/lib/helpers.ts

seanmalbert · 2021-09-29T04:15:23Z

sites/partners/src/listings/PaperListingDetails/sections/DetailListingPhoto.tsx

+    ? /https?:\/\//.exec(listingFormPhoto.fileId)
+      ? listingFormPhoto.fileId
+      : cloudinaryUrlFromId(listingFormPhoto.fileId)


I know this was already here, but regex.test might be better used here instead of exec.

sites/partners/src/listings/PaperListingDetails/sections/helpers.ts

sites/partners/src/listings/PaperListingForm/sections/BuildingDetails.tsx

jaredcwhite

Nice work @emilyjablonski! I think my main comment would be that if src/listings/PaperListingForm/index.tsx is getting changed to this degree, it'd be a great opportunity to refactor. The file's getting pretty large (~800 lines) and feels like it needs more of an architecture. What do you think?

emilyjablonski · 2021-10-01T13:53:28Z

@jaredcwhite I think that's an excellent idea that given the already large size and complexity of this PR should be handled in a separate issue

seanmalbert

Looks good @emilyjablonski . I created a new issue off of @jaredcwhite's comment on refactoring the main listing form.

…tend (bloom-housing#1850)

emilyjablonski added the wip This PR is not ready for review, do not review it's a “Work In Progress” label Sep 17, 2021

seanmalbert mentioned this pull request Sep 17, 2021

Listing Mgmts validation: required to publish #1796

Closed

27 tasks

emilyjablonski changed the title ~~feat: listings management publish validation frontend~~ feat: listings management draft and publish validation frontend Sep 18, 2021

pbn4 reviewed Sep 20, 2021

View reviewed changes

backend/core/src/listings/dto/listing-published-create.dto.ts Show resolved Hide resolved

pbn4 reviewed Sep 20, 2021

View reviewed changes

backend/core/src/shared/entities/address.entity.ts Outdated Show resolved Hide resolved

emilyjablonski changed the base branch from feature/listing_publish_validation to dev September 20, 2021 21:39

emilyjablonski changed the title ~~feat: listings management draft and publish validation frontend~~ feat: listings management draft and publish validation backend & frontend Sep 21, 2021

emilyjablonski requested review from pbn4, seanmalbert, dominikx96 and jaredcwhite September 21, 2021 23:22

emilyjablonski added ready for review and removed wip This PR is not ready for review, do not review it's a “Work In Progress” labels Sep 21, 2021

emilyjablonski force-pushed the feat/publish_validation_frontend branch 2 times, most recently from dcb229a to 4a241f8 Compare September 22, 2021 15:31

dominikx96 requested changes Sep 22, 2021

View reviewed changes

sites/partners/src/listings/PaperListingForm/index.tsx Outdated Show resolved Hide resolved

sites/partners/src/listings/PaperListingForm/index.tsx Outdated Show resolved Hide resolved

dominikx96 self-requested a review September 23, 2021 13:05

dominikx96 requested changes Sep 23, 2021

View reviewed changes

emilyjablonski added wip This PR is not ready for review, do not review it's a “Work In Progress” and removed ready for review labels Sep 23, 2021

pbn4 and others added 6 commits September 23, 2021 17:21

Add Listing draft/published validation

befbd03

Fix e2e tests

367dcfe

Fix tests failing after rebase

7e3d6a8

Fix code style issues with Prettier

0a26730

feat: publish validation frontend

8a19627

draft and publish is working as expected

baf98e4

emilyjablonski added ready for review and removed wip This PR is not ready for review, do not review it's a “Work In Progress” labels Sep 24, 2021

Merge branch 'dev' into feat/publish_validation_frontend

2083841

dominikx96 self-requested a review September 27, 2021 14:35

dominikx96 requested changes Sep 27, 2021

View reviewed changes

fix: rebase issues

c5bca07

fix: unit edit and delete links

720d63f

seanmalbert self-assigned this Sep 28, 2021

dominikx96 approved these changes Sep 28, 2021

View reviewed changes

seanmalbert reviewed Sep 29, 2021

View reviewed changes

jaredcwhite requested changes Oct 1, 2021

View reviewed changes

jaredcwhite mentioned this pull request Oct 1, 2021

Listings Mgmt: Add confirm to publish #1772

Closed

emilyjablonski mentioned this pull request Oct 2, 2021

Create unit tests for helpers in partners library #1915

Open

emilyjablonski added 2 commits October 2, 2021 12:35

refactor: pr feedback

bd8ea78

fix: build fix

45c3aa3

seanmalbert mentioned this pull request Oct 4, 2021

Refactor PaperListingForm/index #1916

Closed

seanmalbert approved these changes Oct 4, 2021

View reviewed changes

emilyjablonski merged commit ef67997 into dev Oct 4, 2021

This was referenced Nov 17, 2021

Add testing to exercise POST /listings CityOfDetroit/bloom#771

Closed

chore: test for create listing CityOfDetroit/bloom#778

Merged

This was referenced Nov 18, 2021

[Bug] listing import scripts are broken CityOfDetroit/bloom#779

Closed

Validation for listing creation (through POST /listings) CityOfDetroit/bloom#780

Open

seanmalbert pushed a commit to CityOfDetroit/bloom that referenced this pull request Jun 23, 2022

feat: listings management draft and publish validation backend & fron…

5ad1c79

…tend (bloom-housing#1850)

emilyjablonski deleted the feat/publish_validation_frontend branch July 20, 2022 22:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: listings management draft and publish validation backend & frontend #1850

feat: listings management draft and publish validation backend & frontend #1850

emilyjablonski commented Sep 17, 2021 •

edited

Loading

netlify bot commented Sep 21, 2021 •

edited

Loading

netlify bot commented Sep 21, 2021 •

edited

Loading

netlify bot commented Sep 21, 2021 •

edited

Loading

dominikx96 left a comment

emilyjablonski commented Sep 23, 2021 •

edited

Loading

dominikx96 left a comment

emilyjablonski commented Sep 27, 2021

emilyjablonski commented Sep 27, 2021 •

edited

Loading

dominikx96 commented Sep 28, 2021

emilyjablonski commented Sep 28, 2021

dominikx96 commented Sep 28, 2021

seanmalbert left a comment

seanmalbert Sep 29, 2021

jaredcwhite left a comment

emilyjablonski commented Oct 1, 2021

seanmalbert left a comment

feat: listings management draft and publish validation backend & frontend #1850

feat: listings management draft and publish validation backend & frontend #1850

Conversation

emilyjablonski commented Sep 17, 2021 • edited Loading

Pull Request Template

Issue

Description

Type of change

How Can This Be Tested/Reviewed?

Checklist:

netlify bot commented Sep 21, 2021 • edited Loading

netlify bot commented Sep 21, 2021 • edited Loading

netlify bot commented Sep 21, 2021 • edited Loading

dominikx96 left a comment

Choose a reason for hiding this comment

emilyjablonski commented Sep 23, 2021 • edited Loading

dominikx96 left a comment

Choose a reason for hiding this comment

emilyjablonski commented Sep 27, 2021

emilyjablonski commented Sep 27, 2021 • edited Loading

dominikx96 commented Sep 28, 2021

emilyjablonski commented Sep 28, 2021

dominikx96 commented Sep 28, 2021

seanmalbert left a comment

Choose a reason for hiding this comment

seanmalbert Sep 29, 2021

Choose a reason for hiding this comment

jaredcwhite left a comment

Choose a reason for hiding this comment

emilyjablonski commented Oct 1, 2021

seanmalbert left a comment

Choose a reason for hiding this comment

emilyjablonski commented Sep 17, 2021 •

edited

Loading

netlify bot commented Sep 21, 2021 •

edited

Loading

netlify bot commented Sep 21, 2021 •

edited

Loading

netlify bot commented Sep 21, 2021 •

edited

Loading

emilyjablonski commented Sep 23, 2021 •

edited

Loading

emilyjablonski commented Sep 27, 2021 •

edited

Loading