Add unit size filter to backend (#368)

* initial thoughts

* initial bedrooms filter (untested)

* Fix code style issues with Prettier

* fix typo in 'studio'

* remove custom filter handling, switch to bedrooms field on unit table

* add additional check for invalid comparisons, outside the filter handler

* use unit_type table for bedrooms, add field to unit_type table, remove toLowerCase() from filter name comparisons to allow camelCase filter names

* Fix code style issues with Prettier

* add bedrooms filter to frontend comparison map

* fix comparison type for frontend map

* fix test and migration indentation

* update changelog

* regenerate swagger

* fix tests

* try to fix more ui tests

* try to fix more ui tests

* add comment for custom filter handling

Co-authored-by: Lint Action <lint-action@samuelmeuli.com>

CHANGELOG.md

@@ -34,6 +34,7 @@ All notable changes to this project will be documented in this file. The format
   - Filtering, pagination, and tests for listings endpoint (Parts of Detroit Team [#18](https://github.com/CityOfDetroit/bloom/pull/18), [#133](https://github.com/CityOfDetroit/bloom/pull/133), [#180](https://github.com/CityOfDetroit/bloom/pull/180), [#257](https://github.com/CityOfDetroit/bloom/pull/257), [#264](https://github.com/CityOfDetroit/bloom/pull/264), [#271](https://github.com/CityOfDetroit/bloom/pull/271)) [#1578](https://github.com/CityOfDetroit/bloom/pull/1578)
   - Units summary table ([#1607](https://github.com/bloom-housing/bloom/pull/1607))
   - Add support for comma-separated lists to filters, ensure comparison is valid ([Detroit Team #356](https://github.com/CityOfDetroit/bloom/pull/356), [#1634](https://github.com/bloom-housing/bloom/pull/1634))
+  - Add bedrooms/unit size filter to backend ([Detroit Team #368](https://github.com/CityOfDetroit/bloom/pull/368), [#1660](https://github.com/bloom-housing/bloom/pull/1660))
 
 - Changed:
 

backend/core/src/listings/dto/listing.dto.ts

@@ -9,6 +9,7 @@ import {
   IsString,
   IsUUID,
   ValidateNested,
+  IsNumberString,
 } from "class-validator"
 import moment from "moment"
 import {
@@ -767,7 +768,16 @@ export class ListingFilterParams extends BaseFilter {
     example: "Fox Creek",
     required: false,
   })
-  [ListingFilterKeys.neighborhood]?: string
+  [ListingFilterKeys.neighborhood]?: string;
+
+  @Expose()
+  @ApiProperty({
+    type: Number,
+    example: "3",
+    required: false,
+  })
+  @IsNumberString({}, { groups: [ValidationsGroupsEnum.default] })
+  [ListingFilterKeys.bedrooms]?: number
 }
 
 export class ListingsQueryParams extends PaginationAllowsAllQueryParams {
@@ -799,4 +809,5 @@ export const filterTypeToFieldMap: Record<keyof typeof ListingFilterKeys, string
   status: "listings.status",
   name: "listings.name",
   neighborhood: "property.neighborhood",
+  bedrooms: "unitTypeRef.num_bedrooms",
 }

backend/core/src/listings/listings.service.ts

@@ -56,6 +56,8 @@ export class ListingsService {
       .createQueryBuilder("listings")
       .select("listings.id", "listings_id")
       .leftJoin("listings.property", "property")
+      .leftJoin("property.units", "units")
+      .leftJoin("units.unitType", "unitTypeRef")
       .groupBy("listings.id")
       .orderBy({ "listings.id": "DESC" })
 

backend/core/src/listings/types/listing-filter-keys-enum.ts

@@ -3,4 +3,5 @@ export enum ListingFilterKeys {
   status = "status",
   name = "name",
   neighborhood = "neighborhood",
+  bedrooms = "bedrooms",
 }

backend/core/src/migration/1628631437422-AddNumBedroomsFieldToUnitType.ts

@@ -0,0 +1,22 @@
+import { MigrationInterface, QueryRunner } from "typeorm"
+
+export class AddNumBedroomsFieldToUnitType1628631437422 implements MigrationInterface {
+  name = "AddNumBedroomsFieldToUnitType1628631437422"
+
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    queryRunner.query(`ALTER TABLE "unit_types" ADD "num_bedrooms" integer`)
+    queryRunner.query(`UPDATE "unit_types" SET "num_bedrooms" = CASE
+  WHEN "name" = 'studio' THEN 0
+  WHEN "name" = 'oneBdrm' THEN 1
+  WHEN "name" = 'twoBdrm' THEN 2
+  WHEN "name" = 'threeBdrm' THEN 3
+  WHEN "name" = 'fourBdrm' THEN 4
+  WHEN "name" = 'fiveBdrm' THEN 5
+END`)
+    queryRunner.query(`ALTER TABLE "unit_types" ALTER COLUMN "num_bedrooms" SET NOT NULL`)
+  }
+
+  public async down(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`ALTER TABLE "unit_types" DROP COLUMN "num_bedrooms"`)
+  }
+}

backend/core/src/shared/dto/filter.dto.ts

@@ -6,6 +6,8 @@ export enum Compare {
   "=" = "=",
   "<>" = "<>",
   "IN" = "IN",
+  ">=" = ">=",
+  "NA" = "NA", // For filters that don't use the comparison param
 }
 
 export class BaseFilter {

backend/core/src/shared/filter/index.ts

@@ -34,6 +34,10 @@ export function addFilters<FilterParams, FilterFieldMap>(
       } else if (typeof value === "string") {
         comparisons = [value]
       }
+      // Ensure none of the user provided comparisons are invalid
+      if (comparisons.some((c) => !Object.keys(Compare).includes(c))) {
+        throw new HttpException("Comparison Not Implemented", HttpStatus.NOT_IMPLEMENTED)
+      }
     } else {
       if (value !== undefined) {
         let values: string[]
@@ -51,41 +55,46 @@ export function addFilters<FilterParams, FilterFieldMap>(
         comparisonCount += values.length
 
         // Throw if this is not a supported filter type
-        if (!(filterType.toLowerCase() in filterTypeToFieldMap)) {
+        if (!(filterType in filterTypeToFieldMap)) {
           throw new HttpException("Filter Not Implemented", HttpStatus.NOT_IMPLEMENTED)
         }
 
-        values.forEach((val: string, i: number) => {
+        values.forEach((filterValue: string, i: number) => {
           // Each WHERE param must be unique across the entire QueryBuilder
           const whereParameterName = `${filterType}_${i}`
-
           const comparison = comparisonsForCurrentFilter[i]
+          const filterField = filterTypeToFieldMap[filterType as string]
+
+          // Handle custom filters here, before dropping into generic filter handler
+
+          // Generic filter handler
           // Explicitly check for allowed comparisons, to prevent SQL injections
           switch (comparison) {
             case Compare.IN:
-              qb.andWhere(
-                `LOWER(CAST(${
-                  filterTypeToFieldMap[filterType.toLowerCase()]
-                } as text)) IN (:...${whereParameterName})`,
-                {
-                  [whereParameterName]: val
-                    .split(",")
-                    .map((s) => s.trim().toLowerCase())
-                    .filter((s) => s.length !== 0),
-                }
-              )
+              qb.andWhere(`LOWER(CAST(${filterField} as text)) IN (:...${whereParameterName})`, {
+                [whereParameterName]: filterValue
+                  .split(",")
+                  .map((s) => s.trim().toLowerCase())
+                  .filter((s) => s.length !== 0),
+              })
               break
             case Compare["<>"]:
             case Compare["="]:
+            case Compare[">="]:
               qb.andWhere(
-                `LOWER(CAST(${
-                  filterTypeToFieldMap[filterType.toLowerCase()]
-                } as text)) ${comparison} LOWER(:${whereParameterName})`,
+                `LOWER(CAST(${filterField} as text)) ${comparison} LOWER(:${whereParameterName})`,
                 {
-                  [whereParameterName]: val,
+                  [whereParameterName]: filterValue,
                 }
               )
               break
+            case Compare.NA:
+              // If we're here, it's because we expected this filter to be handled by a custom filter handler
+              // that ignores the $comparison param, but it was not.
+              throw new HttpException(
+                `Filter "${filterType}" expected to be handled by a custom filter handler, but one was not implemented.`,
+                HttpStatus.NOT_IMPLEMENTED
+              )
             default:
               throw new HttpException("Comparison Not Implemented", HttpStatus.NOT_IMPLEMENTED)
           }

backend/core/src/unit-types/entities/unit-type.entity.ts

@@ -1,6 +1,6 @@
 import { Column, Entity } from "typeorm"
 import { Expose } from "class-transformer"
-import { IsString, MaxLength } from "class-validator"
+import { IsNumber, IsString, MaxLength } from "class-validator"
 import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
 import { AbstractEntity } from "../../shared/entities/abstract.entity"
 
@@ -11,4 +11,9 @@ export class UnitType extends AbstractEntity {
   @IsString({ groups: [ValidationsGroupsEnum.default] })
   @MaxLength(256, { groups: [ValidationsGroupsEnum.default] })
   name: string
+
+  @Column({ type: "integer" })
+  @Expose()
+  @IsNumber({}, { groups: [ValidationsGroupsEnum.default] })
+  numBedrooms: number
 }

backend/core/test/factories/unit.ts

@@ -30,6 +30,7 @@ export default Factory.define<NonDbUnit>(() => ({
     createdAt: new Date(),
     updatedAt: new Date(),
     name: "oneBdrm",
+    numBedrooms: 1,
   },
   createdAt: new Date(),
   updatedAt: new Date(),

backend/core/test/unit-types/unit-types.e2e-spec.ts

@@ -52,16 +52,19 @@ describe("UnitTypes", () => {
 
   it(`should create and return a new unit type`, async () => {
     const unitTypeName = "new unit type"
+    const numBedrooms = 7
     const res = await supertest(app.getHttpServer())
       .post(`/unitTypes`)
       .set(...setAuthorization(adminAccesstoken))
-      .send({ name: unitTypeName })
+      .send({ name: unitTypeName, numBedrooms: numBedrooms })
       .expect(201)
     expect(res.body).toHaveProperty("id")
     expect(res.body).toHaveProperty("createdAt")
     expect(res.body).toHaveProperty("updatedAt")
     expect(res.body).toHaveProperty("name")
     expect(res.body.name).toBe(unitTypeName)
+    expect(res.body).toHaveProperty("numBedrooms")
+    expect(res.body.numBedrooms).toBe(numBedrooms)
 
     const getById = await supertest(app.getHttpServer())
       .get(`/unitTypes/${res.body.id}`)