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.

Sign up for GitHub

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

Jump to bottom

Add filtering for rent price #531

Merged
merged 7 commits into from
Sep 3, 2021
Merged

Add filtering for rent price #531

merged 7 commits into from
Sep 3, 2021

Conversation

abbiefarr
Copy link

Issue

Description

Adds ability to filter by rent price based on data in the units summary table.

UI:
image

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?

Performed some filtering a verified that the query was hitting the backend and the data was being filtered based on the units summary table.

NOTE: this is a little broken in certain cases, e.g. if you want to search for 2-bedrooms that are less than $600 a month, because the bedroom filter is looking at the units table, and the rent filter is looking at the summary table. We need way to generally determine which tables we should be joining to make queries. That's tracked in #530.

  • Desktop View
  • Mobile View

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
  • I have run yarn generate:client if I made backend changes

@abbiefarr abbiefarr added the epic: filtering Tickets related to Filtering label Sep 3, 2021
avaleske
avaleske approved these changes Sep 3, 2021
View reviewed changes
Copy link

@avaleske avaleske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I had a few small nits, one of which is cause of some upstream changes that just happened after you mailed this PR.

@abbiefarr abbiefarr merged commit c3c81b4 into main Sep 3, 2021
@abbiefarr abbiefarr deleted the rent-price-filtering branch September 3, 2021 20:53
seanmalbert pushed a commit that referenced this pull request Jun 23, 2022
@abbiefarr @lint-action
Add filtering for rent price (#531)
942b977 
* add rent price filtering

* update changelog and comments

* Fix code style issues with Prettier

* address comments

* address comments

Co-authored-by: Lint Action <lint-action@samuelmeuli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avaleske avaleske avaleske approved these changes

@willrlin willrlin willrlin approved these changes

Assignees
No one assigned
Labels
epic: filtering Tickets related to Filtering
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rent price filtering
4 participants
@abbiefarr @avaleske @willrlin @lint-action