Update filter code to include upstream changes, fix filter property type

CHANGELOG.md

@@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. The format
 ## Detroit Team M7
 - Added:
   - Debug flags for public and partner site ([#195](https://github.com/CityOfDetroit/bloom/pull/195))
+  - Upstream filter param parsing, with changes to support pagination params and filters that aren't on the listings table ([#180](https://github.com/CityOfDetroit/bloom/pull/180))
 
 ## Unreleased
 

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

@@ -691,4 +691,12 @@ export class ListingFilterParams extends BaseFilter {
     required: false,
   })
   status?: ListingStatus
+
+  @Expose()
+  @ApiProperty({
+    type: String,
+    example: "Fox Creek",
+    required: false,
+  })
+  neighborhood?: string
 }

backend/core/src/listings/listings.controller.ts

@@ -17,13 +17,14 @@ import {
   ValidationPipe,
 } from "@nestjs/common"
 import { ListingsService } from "./listings.service"
-import { ApiBearerAuth, ApiOperation, ApiProperty, ApiTags } from "@nestjs/swagger"
+import { ApiBearerAuth, ApiOperation, ApiProperty, ApiTags, getSchemaPath } from "@nestjs/swagger"
 import { Cache } from "cache-manager"
 import {
   ListingCreateDto,
   ListingDto,
   ListingUpdateDto,
   PaginatedListingsDto,
+  ListingFilterParams,
 } from "./dto/listing.dto"
 import { ResourceType } from "../auth/decorators/resource-type.decorator"
 import { OptionalAuthGuard } from "../auth/guards/optional-auth.guard"
@@ -36,17 +37,19 @@ import { ValidationsGroupsEnum } from "../shared/types/validations-groups-enum"
 import { PaginationQueryParams } from "../shared/dto/pagination.dto"
 import { clearCacheKeys } from "../libs/cacheLib"
 
-export class ListingsListQueryParams extends PaginationQueryParams {
+export class ListingsQueryParams extends PaginationQueryParams {
   @Expose()
   @ApiProperty({
-    type: String,
-    example: "Fox Creek",
+    name: "filter",
     required: false,
-    description: "The neighborhood to filter by",
+    type: [String],
+    items: {
+      $ref: getSchemaPath(ListingFilterParams),
+    },
+    example: { $comparison: ["=", "<>"], status: "active", name: "Coliseum" },
   })
   @IsOptional({ groups: [ValidationsGroupsEnum.default] })
-  @IsString({ groups: [ValidationsGroupsEnum.default] })
-  neighborhood?: string
+  filter?: ListingFilterParams
 
   @Expose()
   @ApiProperty({
@@ -73,12 +76,13 @@ export class ListingsController {
     this.cacheKeys = ["/listings", "/listings?filter[$comparison]=%3C%3E&filter[status]=pending"]
   }
 
+  // TODO: limit requests to defined fields
   @Get()
   @ApiOperation({ summary: "List listings", operationId: "list" })
   @UseInterceptors(CacheInterceptor)
   public async getAll(
     @Headers("origin") origin: string,
-    @Query() queryParams: ListingsListQueryParams
+    @Query() queryParams: ListingsQueryParams
   ): Promise<PaginatedListingsDto> {
     return await this.listingsService.list(origin, queryParams)
   }

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

@@ -4,6 +4,8 @@ import { getRepositoryToken } from "@nestjs/typeorm"
 import { Listing } from "./entities/listing.entity"
 import { mapTo } from "../shared/mapTo"
 import { ListingDto } from "./dto/listing.dto"
+import { Compare } from "../shared/dto/filter.dto"
+import { ListingsQueryParams } from "./listings.controller"
 
 // Cypress brings in Chai types for the global expect, but we want to use jest
 // expect here so we need to re-declare it.
@@ -65,7 +67,14 @@ describe("ListingsService", () => {
     it("should add a WHERE clause if the neighborhood filter is applied", async () => {
       const expectedNeighborhood = "Fox Creek"
 
-      const listings = await service.list(origin, { neighborhood: expectedNeighborhood })
+      const queryParams: ListingsQueryParams = {
+        filter: {
+          $comparison: Compare["="],
+          neighborhood: expectedNeighborhood,
+        },
+      }
+
+      const listings = await service.list(origin, queryParams)
 
       expect(listings.items).toEqual(mockListingsDto)
       expect(mockQueryBuilder.andWhere).toHaveBeenCalledWith(

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

@@ -7,14 +7,16 @@ import {
   ListingCreateDto,
   ListingUpdateDto,
   PaginatedListingsDto,
+  ListingFilterParams,
 } from "./dto/listing.dto"
 import { InjectRepository } from "@nestjs/typeorm"
 import { Repository } from "typeorm"
 import { plainToClass } from "class-transformer"
 import { PropertyCreateDto, PropertyUpdateDto } from "../property/dto/property.dto"
 import { arrayIndex } from "../libs/arrayLib"
-import { ListingsListQueryParams } from "./listings.controller"
+import { ListingsQueryParams } from "./listings.controller"
 import { mapTo } from "../shared/mapTo"
+import { addFilter } from "../shared/filter"
 
 @Injectable()
 export class ListingsService {
@@ -35,21 +37,16 @@ export class ListingsService {
       .leftJoinAndSelect("listings.reservedCommunityType", "reservedCommunityType")
   }
 
-  public async list(
-    origin: string,
-    params: ListingsListQueryParams
-  ): Promise<PaginatedListingsDto> {
-    let query = this.getQueryBuilder().orderBy({
+  public async list(origin: string, params: ListingsQueryParams): Promise<PaginatedListingsDto> {
+    let qb = this.getQueryBuilder()
+    if (params.filter) {
+      addFilter<ListingFilterParams>(params.filter, "listings", qb)
+    }
+
+    qb.orderBy({
       "listings.id": "DESC",
     })
 
-    if (params.neighborhood) {
-      // This works because there's only one property per listing. If that
-      // weren't true for a field (for example, if we filtered on a unit's
-      // fields), we couldn't use this type of where clause.
-      query.andWhere("property.neighborhood = :neighborhood", { neighborhood: params.neighborhood })
-    }
-
     let currentPage: number = params.page
     let itemsPerPage: number = params.limit
 
@@ -59,9 +56,9 @@ export class ListingsService {
     if (currentPage > 0 && itemsPerPage > 0) {
       // Calculate the number of listings to skip (because they belong to lower page numbers)
       const skip = (currentPage - 1) * itemsPerPage
-      query = query.skip(skip).take(itemsPerPage)
+      qb = qb.skip(skip).take(itemsPerPage)
 
-      listings = await query.getMany()
+      listings = await qb.getMany()
 
       itemCount = listings.length
 
@@ -71,7 +68,7 @@ export class ListingsService {
     } else {
       // If currentPage or itemsPerPage aren't specified (or are invalid), issue the SQL query to
       // get all listings (no pagination).
-      listings = await query.getMany()
+      listings = await qb.getMany()
 
       currentPage = 1
       totalPages = 1

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

@@ -7,6 +7,12 @@ export enum Compare {
   "<>" = "<>",
 }
 
+export enum Filters {
+  "status" = "STATUS",
+  "name" = "NAME",
+  "neighborhood" = "NEIGHBORHOOD",
+}
+
 export class BaseFilter {
   @Expose()
   @ApiProperty({

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

@@ -1,4 +1,6 @@
+import { HttpException, HttpStatus } from "@nestjs/common"
 import { WhereExpression } from "typeorm"
+import { Filters } from "../../shared/dto/filter.dto"
 
 /**
  *
@@ -9,15 +11,14 @@ import { WhereExpression } from "typeorm"
 /**
  * This is super simple right now, but we can expand to include complex filter with ands, ors, more than one schema, etc
  */
-export function addFilter<Filter>(filter: Filter[], schema: string, qb: WhereExpression): void {
+export function addFilter<Filter>(filter: Filter, schema: string, qb: WhereExpression): void {
   const operator = "andWhere"
-  /**
-   * By specifying that the filter is an array, it keeps the keys in order, so we can iterate like below
-   */
   let comparisons: unknown[],
     comparisonCount = 0
 
-  // eslint-disable-next-line @typescript-eslint/no-for-in-array
+  // TODO(#210): This assumes that the order of keys is consistent across browsers,
+  // that the key order is the insertion order, and that the $comaprison field is first.
+  // This may not always be the case.
   for (const key in filter) {
     const value = filter[key]
     if (key === "$comparison") {
@@ -36,14 +37,36 @@ export function addFilter<Filter>(filter: Filter[], schema: string, qb: WhereExp
           values = [value]
         }
 
-        values.forEach((val: unknown) => {
-          // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
-          qb[operator](`${schema}.${key} ${comparisons[comparisonCount]} :${key}`, {
-            [key]: val,
-          })
-          comparisonCount++
-        })
+        // TODO(#202): Refactor this out into a provided map, so addFilter() is generic again
+        switch (key.toUpperCase()) {
+          case Filters.status:
+          case Filters.name:
+            addFilterClause(values, key)
+            break
+          case Filters.neighborhood:
+            values.forEach((val: unknown) => {
+              // TODO add support for multiple neighborhoods by using a sub orWhere expression
+              // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
+              qb[operator](`property.neighborhood ${comparisons[comparisonCount]} :neighborhood`, {
+                neighborhood: val,
+              })
+              comparisonCount++
+            })
+            break
+          default:
+            throw new HttpException("Filter Not Implemented", HttpStatus.NOT_IMPLEMENTED)
+        }
       }
     }
   }
+
+  function addFilterClause(values: [string], key: string) {
+    values.forEach((val: unknown) => {
+      // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
+      qb[operator](`${schema}.${key} ${comparisons[comparisonCount]} :${key}`, {
+        [key]: val,
+      })
+      comparisonCount++
+    })
+  }
 }