Create View for /listings endpoint

[seanmalbert]

From discussion in #1563 , we want to create a way to specify a "view" as a parameter to the GET listings endpoint (e.g. \listings?page=2&limit=10&view=simpleListings). This view should return the bare minimum needed for the main listings page, which uses the ListingsList component. These fields are:

• id
• name
• urlSlug
• showWaitList
• reservedCommunityType
  • name
• applicationDueDate
• applicationDueTime
• applicationOpenDate
• buildingAddress
  • city
  • state
  • street
  • zipCode
• unitsSummarized.byUnitTypeAndRent
  • unitType
  • minimumIncome
  • rent

I'm thinking that the listings service can have map of views, with each view being a function that takes a query builder like:

{
  simple: (qb) => {
    qb.select([
      "listings.id",
      "listings.name",
      [...]
    ])

    // leftJoins
  },
}

There are likely DTOs that need updating, because they probably require fields that won't be returned from this query.

Since we will only need byUnitTypeAndRent from unitsSummarized, the call for summarizedUnitsByTypeAndRent should be abstracted from transformUnits so we don't have to calculate all of it.

[avaleske]

This is essentially what I was picturing, yeah!

How often do you think a view might be a superset of another view? For example, the listing page will be a superset of the view used for the listings list, right? In that case, we could have something like

  listingView = {
    listView: (qb) => {
      qb.select([
        "listings.id",
        "listings.name",
        [...]
      ])
  
      // leftJoins
    },
    pageView: (qb) => {
      listingViews.listView(qb)
        .select([
          "listing.extraField",
        ])
        ... // joins and such
    },
  }

right? I'm not sure if that would override the original select() or add to it though.

Another way I had been considering (and I don't know if this is better) would be:
Instead of taking a querybuilder, the map could return an object shaped like

queryComponents = {
  selects: ["listings.id", "listings.name", ...],
  leftJoins: ["property", ...]
}

and then the querybuilder just uses it like qb.select(queryComponents.selects).leftJoin(queryComponents.leftJoins)...

In that world, the "inheritance" would be something like

  listView: { selects: ["listings.id", "listings.name"] ... },
  pageView: { selects: [...listView.selects, "listings.extraField"], ...},

Although a) I don't think you can reference fields on the same object so these might have to be properties of a class for that format to work? and b) it exposes the field names and such to the rest of the service (which is probably fine but could allow someone to add additional selects outside of this framework)

This also points toward moving the selects and joins for the query that will be added in #1578 to support filtering into their own view (maybe called filteringBase) or something, so all the query definitions are together.

[seanmalbert]

@avaleske ,
That's a good point with extending and being able to add filters. And what you said made me think of an additional option, which is to use classes. Something like (not tested):

class BaseView<T> {
	repo: T
	schema: string
	fields: string[]
	filter?: FilterInterface
	qb: WhereExpression
	constructor(repo: T, schema: string, fields: string[], filter: FilterInterface) {
		this.repo = repo
		this.schema = schema
		this.fields = fields
		this.filter = filter
		this.qb
	}

	queryBuilder() {
		this.qb = repo.createQueryBuilder(schema)
		this.qb.select(fields)
		
		// add filters to qb
		// add pagination to qb
		return qb
	}
}

class AnotherView<T> extends BaseView<T> {
	queryBuilder() {
		const qb = super.qb()
		qb.addSelcect(this.fields)
		
		// add/overwrite filters
		// add/overwrite options
		return qb
	}
}

Then in the service endpoints we can do:

const qb = new AnotherView<Listing>(args).queryBuilder()
const listings = await qb.getMany()

I'd like to start testing out these options tomorrow and have a PR ready at the beginning of next week, so let me know if your team feels strongly about one direction over the other, otherwise we can iterate off of the PR.

[avaleske]

I like your idea to use classes, @seanmalbert!

I don't think we have strong opinions about one way or the other, but tagging @willrlin, @anders-schneider, and @abbiefarr who've also been thinking about the data models.

Also I realize the code is just a sketch, but it inspires a few questions:

• In the BaseView constructor, where is the fields parameter coming from? In AnotherView it seems like the fields are a property on the class, which is what I was expecting to see.
• I don't quite understand what it would mean for BaseView to be generic with respect to the entity, since the fields are going to be pretty specific to each entity, right?

Additionally, if building these view grows to be complex or turns out to not lend itself well to this inheritance model, I could see it being helpful to have a wrapper ListingViewFactory class that handles actually assembling the querybuilders or view classes, and exposes methods like ListingViewFactory.getPageView()

[avaleske]

I was thinking about this more and I realized another thing:
For efficiency I've been assuming we'd set both the selects and the joins for each view, but I don't think we actually need to do that. With the changes in #1578, we only join all the related tables onto the 10 or so listings that are currently being returned, not all the listings, so the joins aren't actually that expensive.

Given that, for simplicity, the base* query could probably join everything we might need, and then each view just specifies the selects it wants (maybe after removing duplicates from the array of selects if TypeORM doesn't do that already). That'd probably be easier to reason about.

*I'm using "base" to refer to the minimal view for the Listings List, and "inner query" to refer to the subquery that returns a list of query ids that match the current filters and pagination params.

[seanmalbert]

In the BaseView constructor, where is the fields parameter coming from? In AnotherView it seems like the fields are a property on the class, which is what I was expecting to see.

I'm sorry I wasn't as clear as I could have been. In this example, it would come from the instantiation of AnotherView (passed as args in the example for simplicity sake), which extends BaseView and inherits its properties and methods. We may want to super props in the extender's constructor and pass args directly to the method, but I think that will become clear when actually implementing it.

I don't quite understand what it would mean for BaseView to be generic with respect to the entity, since the fields are going to be pretty specific to each entity, right?

I was thinking a little ahead here, as part of the steps to add our next jurisdiction, we need to update all of the relevant tables to reference jurisdiction_id. I was thinking that we could have a base view which included adding a where clause with the jurisdiction_id applied. I think for this case though, we can leave out the generic.

And I like the idea of the ListingViewFactory.

[seanmalbert]

Since I did a little work this morning, I think I better understand your questions and what you were getting at. It doesn't make since to have fields or any of those args, for that matter, passed in at all with the extension model, and that they are indeed just properties on the class.

[kathyccheng]

Closed in #1626