Fix ZAP Search radius filter bug

[TylerMatteo]

This Issue is to research and potentially fix a long standing bug causing radius search in ZAP Search to not function.

See ADO bug and comments for finding from previous research: https://dev.azure.com/NYCPlanning/ITD/_workitems/edit/931

You can safely ignore portions of comments that refer to geometry fields being added in CRM. Those are in reference to potential enhancements that were never implemented. Our back end code for the radius search uses geospatial data in Carto for geospatial searching.

As explained there, the front end sends distance_from_point and radius_from_point query params when the user tries to filter by radius. Starting with the blocksWithinRadius function, the back end code tries to find city blocks in that radius and then find projects with associated bbls in those blocks. I was able to log out the blocks coming back from Carto so that part seems to be working, but the filter is never actually sent to CRM for some reason.

I was able to get to the radius filter to work for an initial search by adding blocks_in_radius to the ALLOWED_FILTERS array but that causes errors when trying to turn off the radius filter.

Outcomes

• [ ] Identify and document root cause of non-functioning ZAP Search radius filter
• [ ] If we can't identify the root cause by the end of the sprint, document finding so far and any relevant code paths or relevant past PRs

[pratishta]

Work on this ticket is happening on this branch: pratishta/radius-filter

I didn't read Tyler's last comment about adding blocks_in_radius to ALLOWED_FILTERS so I spent earlier today 'figuring that out' on my own 🥸

But the main error we get from just making the above change is:

TypeError: strings.map is not a function
    at Object.containsAnyOf (/Users/pratishtayerakala/dcp_ae/labs-zap-search/server/dist/crm/crm.utilities.js:69:10)
    at Object.blocks_in_radius (/Users/pratishtayerakala/dcp_ae/labs-zap-search/server/dist/project/project.service.js:135:58)

And it's because of two things happening in the same file project.service.ts:

1. We have to manually delete the blocks query attribute. The query attribute name changed from block to blocks_in_radius a while back, but we never updated the manual deleting with the new attribute name. I updated the QUERY_TEMPLATES, the geometry.service.ts file and other places to reflect that.
2. The condition to delete that attribute if (!query.block) delete query.block; doesn't really work. The most canonical way I found to check for an empty object seemed to be to check the length of its keys !Object.keys(query.blocks_in_radius).length) so I changed it to that.

[pratishta]

Potential solution here https://github.com/NYCPlanning/labs-zap-search/pull/1500

I worked on getting the existing radius filter to work. David updated it so that only projects in the borough of the radius filter show up.

I'm not sure how to add or update tests for this. Having trouble getting the tests to run locally.

[pratishta]

With this current fix, there is still a bug: when the radius is very small (~25-140ft) carto doesn't pull any valid blocks and we have an empty blocks_in_radius list. As described in an earlier comment, we then delete this attribute from the query. But doing so makes it so that there's no radius filter at all and thus returns all projects.

It might be worthwhile to restructure how we handle the filter:

• Keep the empty block list and force a filter with the CRM API (e.g. when radius filter is on and block list is empty, set it to [null])
• Removing the deprecated blocks code so that blocks_in_radius is the only source of truth for the blocks contained in the radius filter
• Don't allow adjusting filters or radius size when loading

[TylerMatteo]

@pratishta I did a little more digging after our call. I have some changes that seem to solve all but one of our issues, and I think I have a path forward for the one remaining.

• The first thing I did was update blocksWithinRadius to return false instead of {} when distance_from_point and radius_from_point are undefined. Empty objects are truthy so passing literal false seemed like it might help simplify queryProjects
• I updated the first few lines of queryProjects to read:

  const normalizedQuery = blocks === false ? {...query} : {
    blocks_in_radius: blocks,
    ...query
  };

• I removed if (!Object.keys(query.blocks_in_radius).length) delete query.blocks_in_radius; from generateProjectsFilterString because it should no longer be necessary???

Now it seems to be returning all projects that match the other filters when the radius filter is off or when it's on but you haven't picked a lat/long yet. The issue I still have is that when you select a radius that returns an empty list of blocks, you get back every project that matches the other criteria.

I noticed that when I log out the $filter that is send to the OData API in this scenario, it doesn't have the clause for projectbbls at all

(dcp_visibility eq 717170003) and (dcp_dcp_project_dcp_projectbbl_project/any()) and ((dcp_publicstatus eq 717170001) or (dcp_publicstatus eq 717170005))

That's because the startsWithAnyOf utility in crm.utilities.ts is mapping over an empty list of bbls, so it doesn't add anything. If we wanted to implement this correctly in the OData query, we really need that utility to always add a condition that says essentially "only give me back projects that have at least one associate project bbl, even if this list of blocks is empty". That would give us a way to filter out the "unmapped" projects.

Implementing that in OData might be tricky but I brought this up with @dhochbaum-dcp and he made a good point - if the radius filter is on with a radius selected but no blocks exist within that radius, we would always expect the filter to return 0 projects. With that in mind, there may be an easier way to achieve this but "short circuiting" the query code to just return an empty search result when the blocks query to Carto returns an empty list.

[TylerMatteo]

@pratishta @dhochbaum-dcp We've firmly crossed over into actually fixing this issue so I renamed it and will carry it over into sprint J

[TylerMatteo]

Moving back to In Progress until the above bugs are resolved

[pratishta]

@TylerMatteo I agree with your first three bullet points and updated project.service.ts to reflect that

"short circuiting" the query code to just return an empty search result when the blocks query to Carto returns an empty list.

I agree, I've been thinking about how and where in the code we could return empty and early. I added a commit with a check for empty_filter where we set projects to empty when the radius filter is flipped on (radius_from_point), the user has chosen a point on the map (distance_from_point) and Carto returns an empty block list (blocks == false). I think this avoids actually sending an odata request via the crm.service.ts ?

Looking for feedback for this option before cleaning it up and documenting it in the code and the PR

[TylerMatteo]

Yes I think that is the best approach. I confirmed the code in your branch is skipping the odata request in the scenario I would expect it to. Maybe we could pair on the clean up? I have some thoughts about how we can refactor the existing code to be a little easier to follow.

[TylerMatteo]

I merged @pratishta's PR for this so the fix is testable at https://zap-staging.planninglabs.nyc/. I did some testing and seems to generally be working fine but I'd like to get another pair of eyes on it before we release. @dhochbaum-dcp can you spend some time trying to break it? I'm thinking an hour, tops.

[dhochbaum-dcp]

Seems good to me, I didn't find issues aside from the already documented bug.