-
Notifications
You must be signed in to change notification settings - Fork 26
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
Contribute Detroit filters to Bloom #1884
Contribute Detroit filters to Bloom #1884
Conversation
✔️ Deploy Preview for dev-partners-bloom ready! 🔨 Explore the source changes: da01c87 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-partners-bloom/deploys/615b975a62815600089ef16c 😎 Browse the preview: https://deploy-preview-1884--dev-partners-bloom.netlify.app |
✔️ Deploy Preview for dev-bloom ready! 🔨 Explore the source changes: da01c87 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-bloom/deploys/615b975ac245810008b6ca5f 😎 Browse the preview: https://deploy-preview-1884--dev-bloom.netlify.app |
✔️ Deploy Preview for dev-storybook-bloom ready! 🔨 Explore the source changes: da01c87 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-storybook-bloom/deploys/615b975a17f78700073c4702 😎 Browse the preview: https://deploy-preview-1884--dev-storybook-bloom.netlify.app |
…Detroit/bloom into sync-backend-filters-with-detroit
9b6a92a
to
25a7df4
Compare
@seanmalbert failures don't seem related, but I can't find how to retrigger a build. So I'll say this is ready for review, and if you have any insight into the test failures that would be very appreciated! |
@seanmalbert Test failures look unrelated to this PR to me :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contributing these filters, @abbiefarr!
* Contribute Detroit filters to Bloom * Contribute Detroit filters to Bloom * update changelog * fix malformed query * rename ami filter * Don't include nulls in senior housing filter * fix: cleanup after upstream merge Co-authored-by: Sean Albert <smabert@gmail.com>
Issue
Addresses CityOfDetroit#566
Description
Apologies for the large PR. For most of us this is the last week on the project, so I'm trying to make this contribution before I leave the team.
This PR contributes a number of Detroit's filters upstream, with the intention to add functionality and keep our code bases as synced as possible.
Some new filters follow the standard filters code path with regular comparisons (e.g.
minRent
). Others require special clauses, which have been pulled out into a separate file calledcustom_filters.ts
.Contributed filters include:
seniorHousing
: this is pulled from thereservedCommunityType
column. It checks for a match with "senior62" in the database. This is added as a custom filter.minRent
andmaxRent
: these are pulled fromunitsSummary.monthly_rent_[min|max]
, and implemented as normal>=
or<=
filters.ami
: pulls fromunitsSummary.ami_percentage
if that is populated, otherwise querieslistings.ami_percentage_max
. Checks for values>=
the provided AMI.availability
: There are 3 possible values for availability:hasAvailability
,noAvailability
, andwaitlist
. Results are queried fromunitsSummary.total_available
if the filter is for has or no availability. Pulls fromlistings.is_waitlist_open
if the filter is for a waitlist.The addition of
$include_nulls
toBaseFilter
is also included. If set totrue
, null values will be considered a match on the filter and will be returned in the results. This is useful if data is incomplete.We also trivially added a
<=
comparison filter.Type of change
How Can This Be Tested/Reviewed?
Unit tests were added for the
$include_nulls
functionality.Checklist:
yarn generate:client
if I made backend changes