feat: property name search

[ColinBuyck]

This PR addresses #938

• Addresses the issue in full
• Addresses only certain aspects of the issue

Description

This PR adds the FE property name search fields as well as the BE support to search by text on the listings combined fields. There is existing search functionality for the listings table but since the public side uses the combined table, this new approach had to be added. Also, you'll notice custom handling of the semicolon and colon character because of their use in our url formatting and the single apostrophe since it could break the sql query respectively.

Also, flagged some a11y concerns that seem to be bigger than this implementation here: #1045

Outstanding question

1. I previously had all inputs cleaned with regex like .replace(/[^a-zA-Z0-9:;"' -]/g, "") but was feeling frustrated on how this would interact with edge cases. We don't restrict the partner on what they can name listings and if a character outside of this list would be used, a user would get no results if they typed the name out since Homes@Main would be searched as HomesMain which would not be a match. Without validation, does this create a vulnerability? Particularly in the way that listings combined is creating a more explicit query rather than how we use prisma with the listings table.

2. When a user successfully searches a listing, the map centers closely around just that pin. This creates a state where if they change the listings search text, they will see no matching listings or if they clear the text in the modal, they will still only see that listing since we're searching on the current region. It seems too confusing to assume the user would zoom out in and then clear the search to see all listings again.

UPDATE: The map now doesn't zoom in as much and design is exploring future improvements which will be handled in another ticket.

How Can This Be Tested/Reviewed?

This can be tested on the homepage and the listing search modal by searching for existing listings, listings that don't exist, search strings short enough to display multiple listings (ie. single letter), general gibberish, and an empty string.

Author Checklist:

• Added QA notes to the issue with applicable URLs
• Reviewed in a desktop view
• Reviewed in a mobile view
• Reviewed considering accessibility
• Added tests covering the changes
• Made corresponding changes to the documentation
• Ran yarn generate:client and/or created a migration when required

Review Process:

• Read and understand the issue
• Ensure the author has added QA notes
• Review the code itself from a style point of view
• Pull the changes down locally and test that the acceptance criteria is met
• Either (1) explicitly ask a clarifying question, (2) request changes, or (3) approve the PR, even if there are very small remaining changes, if you don't need to re-review after the updates

[emilyjablonski]

This functionally looks great - there is one small bug around the clear filters button in the modal. To be fair, I kind of wonder if the button should clear the filter, submit it, and auto close the modal, but with the current pattern the input field should clear (video in Slack, github is being upsetty spaghetti).

We also do need to make sure all the data is sanitized, but I remember @YazeedLoonat mentioning at some point that Prisma gives us some of that for free. Just wanna confirm w him!

[ColinBuyck]

@emilyjablonski Thanks for the thorough review! I fixed that clear filters and spelling bugs and left some thoughts on the other comment 🛸

[emilyjablonski]

Wanna confirm the santization w Yazeed otherwise lgtm!

[mcgarrye]

WOOOO

[YazeedLoonat]

@emilyjablonski @mcgarrye @ColinBuyck I think the way the search was implemented here leaves us vulnerable to sql injection

specifically I think the way we are sanitizing the string isn't quite strong enough and leaves room for people to inject deletes or updates or whatever.

I don't have an immediate "do this instead" but peeping what prisma offers: https://www.prisma.io/docs/orm/prisma-client/type-safety/prisma-validator

you could possibly use that, but this may need a bit more investigation into how to properly sanitize that if the prisma validator fails us

edit: further reading seems to be: https://www.prisma.io/docs/orm/prisma-client/using-raw-sql/raw-queries

[ColinBuyck]

@YazeedLoonat After our conversation about parameterizing the query, I ran into a bit of a block parametrizing the county array that is used in the filter flow (some references of the challenges below). It's still a bit unclear to me but it seems that the IN operator does not support accepting a parameter in the way that the prisma docs recommend protecting the unsafe queries (source). How do you feel about just passing the search as a parameter and write up a ticket to refactor the overall filter process? Especially since this ticket was originally FE in nature? I've pushed up that change if you want to check it out!

https://stackoverflow.com/a/73298789

https://stackoverflow.com/questions/337704/parameterize-an-sql-in-clause

[ColinBuyck]

@YazeedLoonat I've updated the listing service to handle having multiple search filters as we discussed. I did want to flag that the sites/public search.ts file does not allow for this even if you manually add multiple searches via the url params. It stores the search value with the filter name as the key on the object.

As for parameterizing the whole query, I decided to wait on this since due to this post. It seems this would only be possible to do with switching the counties where clause to ANY rather than IN. Given the sensitivity of this query with the map along with the original issue being frontend focused that work is captured in #1058

[YazeedLoonat]

LGTM 🫶