Skip to content

Commit

Permalink
Contribute Detroit filters to Bloom (bloom-housing#1884)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
abbiefarr and seanmalbert authored Oct 5, 2021
1 parent 9a1b51c commit 7086b8e
Show file tree
Hide file tree
Showing 12 changed files with 521 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ All notable changes to this project will be documented in this file. The format
- ** Breaking Change**: Endpoint `PUT /user/:id` is admin only now, because it allows edits over entire `user` table ([#1862](https://github.com/bloom-housing/bloom/pull/1862))
- Changes to applications done through `PUT /applications/:id` are now reflected in AFS ([#1810](https://github.com/bloom-housing/bloom/pull/1810))
- Adds confirmationCode to applications table ([#1854](https://github.com/bloom-housing/bloom/pull/1854))
- Add various backend filters ([#1884](https://github.com/bloom-housing/bloom/pull/1884))

## Frontend

Expand Down
35 changes: 34 additions & 1 deletion backend/core/src/listings/dto/filter-type-to-field-map.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,44 @@
// Using a record lets us enforce that all types are handled in addFilter
import { ListingFilterKeys } from "../../.."

export const filterTypeToFieldMap: Record<keyof typeof ListingFilterKeys, string> = {
/**
* Fields for the Availability and AMI filters are determined based on the value
* of the filter or by checking multiple columns. Since we can't specify a single
* field the filters correspond to, we remove them from the filterTypeToFieldMap.
*/
type keysWithMappedField = Exclude<
keyof typeof ListingFilterKeys,
"minAmiPercentage" | "availability"
>

export const filterTypeToFieldMap: Record<keysWithMappedField, string> = {
status: "listings.status",
name: "listings.name",
neighborhood: "property.neighborhood",
bedrooms: "unitTypeRef.num_bedrooms",
zipcode: "buildingAddress.zipCode",
leasingAgents: "leasingAgents.id",
seniorHousing: "reservedCommunityType.name",
// This is the inverse of the explanation for maxRent below.
minRent: "unitsSummary.monthly_rent_max",
/**
* The maxRent filter uses the monthly_rent_min field to avoid missing units
* in the unitsSummary's rent range. For example, if there's a unitsSummary with
* monthly_rent_min of $300 and monthly_rent_max of $800, we could have a
* real unit with rent $500, which would look like:
*
* $300 ---------------- $500 ------ $600 ----------- $800
* ^ ^ ^ ^
* ^ ^ ^ ^
* | | | unitsSummary.monthly_rent_max
* | | maxRent filter value
* | actual unit's rent
* unitsSummary.monthly_rent_min
*
* If a user sets the maxRent filter to $600 we should show this potential unit.
* To make sure we show this potential unit in results, we want to search for
* listings with a monthly_rent_min that's <= $600. If we used the
* monthly_rent_max field, we'd miss it.
*/
maxRent: "unitsSummary.monthly_rent_min",
}
56 changes: 53 additions & 3 deletions backend/core/src/listings/dto/listing-filter-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
import { BaseFilter } from "../../shared/dto/filter.dto"
import { Expose } from "class-transformer"
import { ApiProperty } from "@nestjs/swagger"
import { IsEnum, IsNumberString, IsOptional, IsString } from "class-validator"
import { IsBooleanString, IsEnum, IsNumberString, IsOptional, IsString } from "class-validator"
import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
import { ListingFilterKeys } from "../../.."
import { AvailabilityFilterEnum, ListingFilterKeys } from "../../.."
import { ListingStatus } from "../types/listing-status-enum"

// add other listing filter params here
Expand Down Expand Up @@ -66,5 +66,55 @@ export class ListingFilterParams extends BaseFilter {
})
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsString({ groups: [ValidationsGroupsEnum.default] })
[ListingFilterKeys.leasingAgents]?: string
[ListingFilterKeys.leasingAgents]?: string;

@Expose()
@ApiProperty({
enum: Object.keys(AvailabilityFilterEnum),
example: "hasAvailability",
required: false,
})
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsEnum(AvailabilityFilterEnum, { groups: [ValidationsGroupsEnum.default] })
[ListingFilterKeys.availability]?: AvailabilityFilterEnum;

@Expose()
@ApiProperty({
type: Boolean,
example: "true",
required: false,
})
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsBooleanString({ groups: [ValidationsGroupsEnum.default] })
[ListingFilterKeys.seniorHousing]?: boolean;

@Expose()
@ApiProperty({
type: Number,
example: "300",
required: false,
})
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsNumberString({}, { groups: [ValidationsGroupsEnum.default] })
[ListingFilterKeys.minRent]?: number;

@Expose()
@ApiProperty({
type: Number,
example: "700",
required: false,
})
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsNumberString({}, { groups: [ValidationsGroupsEnum.default] })
[ListingFilterKeys.maxRent]?: number;

@Expose()
@ApiProperty({
type: Number,
example: "40",
required: false,
})
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsNumberString({}, { groups: [ValidationsGroupsEnum.default] })
[ListingFilterKeys.minAmiPercentage]?: number
}
20 changes: 18 additions & 2 deletions backend/core/src/listings/dto/listing.dto.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
import { Listing } from "../entities/listing.entity"
import { Expose, plainToClass, Transform, Type } from "class-transformer"
import { IsDefined, IsNumber, IsOptional, IsString, ValidateNested } from "class-validator"
import {
ArrayMaxSize,
IsDate,
IsDefined,
IsNumber,
IsOptional,
IsString,
IsUUID,
ValidateNested,
IsNumberString,
IsEnum,
IsArray,
IsBooleanString,
} from "class-validator"
import moment from "moment"
import { PreferenceDto } from "../../preferences/dto/preference.dto"
import { OmitType } from "@nestjs/swagger"
import { AddressDto } from "../../shared/dto/address.dto"
import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
import { ListingStatus } from "../types/listing-status-enum"
import { UnitDto } from "../../units/dto/unit.dto"
import { AvailabilityFilterEnum, ListingFilterKeys } from "../types/listing-filter-keys-enum"
import { PaginationFactory, PaginationAllowsAllQueryParams } from "../../shared/dto/pagination.dto"
import { BaseFilter } from "../../shared/dto/filter.dto"
import { UnitCreateDto, UnitDto, UnitUpdateDto } from "../../units/dto/unit.dto"
import { ReservedCommunityTypeDto } from "../../reserved-community-type/dto/reserved-community-type.dto"
import { AssetDto } from "../../assets/dto/asset.dto"
import { ListingEventDto } from "./listing-event.dto"
Expand Down
63 changes: 57 additions & 6 deletions backend/core/src/listings/listings.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Compare } from "../shared/dto/filter.dto"
import { TranslationsService } from "../translations/translations.service"
import { AmiChart } from "../ami-charts/entities/ami-chart.entity"
import { OrderByFieldsEnum } from "./types/listing-orderby-enum"
import { AvailabilityFilterEnum } from "./types/listing-filter-keys-enum"
import { ListingFilterParams } from "./dto/listing-filter-params"
import { ListingsQueryParams } from "./dto/listings-query-params"

Expand Down Expand Up @@ -160,7 +161,7 @@ describe("ListingsService", () => {

expect(listings.items).toEqual(mockListings)
expect(mockInnerQueryBuilder.andWhere).toHaveBeenCalledWith(
"LOWER(CAST(property.neighborhood as text)) = LOWER(:neighborhood_0)",
"(LOWER(CAST(property.neighborhood as text)) = LOWER(:neighborhood_0))",
{
neighborhood_0: expectedNeighborhood,
}
Expand All @@ -171,15 +172,15 @@ describe("ListingsService", () => {
mockListingsRepo.createQueryBuilder
.mockReturnValueOnce(mockInnerQueryBuilder)
.mockReturnValueOnce(mockQueryBuilder)
const expectedNeighborhoodString = "Fox Creek, , Coliseum," // intentional extra and trailing commas for test
const zipCodeString = "10011, , 10014," // intentional extra and trailing commas for test
// lowercased, trimmed spaces, filtered empty
const expectedNeighborhoodArray = ["fox creek", "coliseum"]
const expectedZipCodeArray = ["10011", "10014"]

const queryParams: ListingsQueryParams = {
filter: [
{
$comparison: Compare["IN"],
neighborhood: expectedNeighborhoodString,
zipcode: zipCodeString,
},
],
}
Expand All @@ -188,9 +189,59 @@ describe("ListingsService", () => {

expect(listings.items).toEqual(mockListings)
expect(mockInnerQueryBuilder.andWhere).toHaveBeenCalledWith(
"LOWER(CAST(property.neighborhood as text)) IN (:...neighborhood_0)",
"(LOWER(CAST(buildingAddress.zipCode as text)) IN (:...zipcode_0))",
{
neighborhood_0: expectedNeighborhoodArray,
zipcode_0: expectedZipCodeArray,
}
)
})

it("should include listings with missing data if $include_nulls is true", async () => {
mockListingsRepo.createQueryBuilder
.mockReturnValueOnce(mockInnerQueryBuilder)
.mockReturnValueOnce(mockQueryBuilder)
const queryParams: ListingsQueryParams = {
filter: [
{
$comparison: Compare["="],
name: "minRent",
$include_nulls: true,
},
],
}

const listings = await service.list(queryParams)

expect(listings.items).toEqual(mockListings)
expect(mockInnerQueryBuilder.andWhere).toHaveBeenCalledWith(
"(LOWER(CAST(listings.name as text)) = LOWER(:name_0) OR listings.name IS NULL)",
{
name_0: "minRent",
}
)
})

it("should include listings with missing data if $include_nulls is true for custom filters", async () => {
mockListingsRepo.createQueryBuilder
.mockReturnValueOnce(mockInnerQueryBuilder)
.mockReturnValueOnce(mockQueryBuilder)
const queryParams: ListingsQueryParams = {
filter: [
{
$comparison: Compare["NA"],
availability: AvailabilityFilterEnum.waitlist,
$include_nulls: true,
},
],
}

const listings = await service.list(queryParams)

expect(listings.items).toEqual(mockListings)
expect(mockInnerQueryBuilder.andWhere).toHaveBeenCalledWith(
"(listings.is_waitlist_open = :availability OR listings.is_waitlist_open is NULL)",
{
availability: true,
}
)
})
Expand Down
11 changes: 11 additions & 0 deletions backend/core/src/listings/types/listing-filter-keys-enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,16 @@ export enum ListingFilterKeys {
neighborhood = "neighborhood",
bedrooms = "bedrooms",
zipcode = "zipcode",
availability = "availability",
seniorHousing = "seniorHousing",
minRent = "minRent",
maxRent = "maxRent",
minAmiPercentage = "minAmiPercentage",
leasingAgents = "leasingAgents",
}

export enum AvailabilityFilterEnum {
hasAvailability = "hasAvailability",
noAvailability = "noAvailability",
waitlist = "waitlist",
}
21 changes: 19 additions & 2 deletions backend/core/src/shared/dto/filter.dto.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ApiProperty } from "@nestjs/swagger"
import { Expose } from "class-transformer"
import { IsEnum } from "class-validator"
import { Expose, Transform } from "class-transformer"
import { IsEnum, IsOptional, IsBoolean } from "class-validator"
import { ValidationsGroupsEnum } from "../types/validations-groups-enum"

// Add other comparisons as needed (>, <, etc)
Expand All @@ -9,6 +9,7 @@ export enum Compare {
"<>" = "<>",
"IN" = "IN",
">=" = ">=",
"<=" = "<=",
"NA" = "NA", // For filters that don't use the comparison param
}

Expand All @@ -21,4 +22,20 @@ export class BaseFilter {
})
@IsEnum(Compare, { groups: [ValidationsGroupsEnum.default] })
$comparison: Compare

@Expose()
@ApiProperty({
type: Boolean,
example: "true",
required: false,
})
@Transform(
(value?: string) => {
return value === "true"
},
{ toClassOnly: true }
)
@IsOptional({ groups: [ValidationsGroupsEnum.default] })
@IsBoolean({ groups: [ValidationsGroupsEnum.default] })
$include_nulls?: boolean
}
Loading

0 comments on commit 7086b8e

Please sign in to comment.