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

1774 preferred unit (same as one from Sean's fork) #1787

Merged
merged 24 commits into from
Sep 10, 2021
Merged

Conversation

seanmalbert
Copy link
Collaborator

@seanmalbert seanmalbert commented Sep 1, 2021

Issue

Addresses #1774

  • 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

Updates both frontend and backend to save unitType id's (they come from the listing units property) instead of strings like studio in the preferredUnit property (application).

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 with different unit types (e.g studio, 4 bedrooms), then post an application using public and partners. Check if both applications showing properly in partners in the Household Details > Preferred Unit Sizes section.

  • Desktop View
  • Mobile View
  • Test A
  • Test B

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have reviewed the changes in a desktop view
  • I have reviewed the changes in a mobile view
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have assigned reviewers
  • I have updated the changelog to include a description of my changes

@netlify
Copy link

netlify bot commented Sep 1, 2021

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

🔨 Explore the source changes: 2f99546

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

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

@netlify
Copy link

netlify bot commented Sep 1, 2021

✔️ Deploy Preview for dev-bloom ready!

🔨 Explore the source changes: 2f99546

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

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

@netlify
Copy link

netlify bot commented Sep 1, 2021

✔️ Deploy Preview for clever-edison-cd22c1 ready!

🔨 Explore the source changes: 8d4c0f5

🔍 Inspect the deploy log: https://app.netlify.com/sites/clever-edison-cd22c1/deploys/613866877dc61c00086e8cb7

😎 Browse the preview: https://deploy-preview-1787--clever-edison-cd22c1.netlify.app

@netlify
Copy link

netlify bot commented Sep 1, 2021

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

🔨 Explore the source changes: 2f99546

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

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

@dominikx96
Copy link
Collaborator

@seanmalbert I updated the frontend for the public & partners app. It looks like the backend tests are failing, can you check it before merge?

@seanmalbert
Copy link
Collaborator Author

@dominikx96 , all tests are passing now.

@seanmalbert
Copy link
Collaborator Author

@emilyjablonski , can you review when you get a chance. Thank you!

@emilyjablonski
Copy link
Collaborator

emilyjablonski commented Sep 9, 2021

Screen Shot 2021-09-08 at 6 13 35 PM

The sorting on partners looks like a little off, but otherwise looking pretty good!

Not sure if this is an issue with this PR or just our seed data, but if I try an edit an application in partners that was created with the seed data, on save I get the error below:
Screen Shot 2021-09-08 at 6 15 59 PM

@dominikx96
Copy link
Collaborator

@emilyjablonski I sorted these values in Partners, also tried to prevent undefined error (but it's probably seeds problem), can you check again? :)

@seanmalbert
Copy link
Collaborator Author

Needs to be rebased.

@seanmalbert seanmalbert merged commit 4b6718e into dev Sep 10, 2021
seanmalbert added a commit to CityOfDetroit/bloom that referenced this pull request Jun 23, 2022
* backend changes for preferredUnit -> UnitType[]

* Fix code style issues with Prettier

* Update unit type translations

* Prevent unit name undefined

* Create helper to extract unique unit types

* Update public app to use new unit types structure

* Refactor unit helper

* Update form to send array of objects with id property

* Update partners form to support new

* Update e2e test

* Fix code style issues with Prettier

* Fix test

* Update unit using id from the listing

* Fix code style issues with Prettier

* Update terms test

* Fix code style issues with Prettier

* Add removed command

* Fix test

* Fix code style issues with Prettier

* Update changelog

* backend updates to fix tests and cleanup

* Fix code style issues with Prettier

* Sort unitType values and fix undefined issue

* Fix code style issues with Prettier

Co-authored-by: Lint Action <lint-action@samuelmeuli.com>
Co-authored-by: Dominik Barcikowski <dominik@airnauts.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants