Closed brianalloyd closed 3 years ago
Adding blocked until we get the green light to proceed: https://dsva.slack.com/archives/C52CL1PKQ/p1619547398313600?thread_ts=1619536672.303000&cid=C52CL1PKQ
Please see my Slack comment here: https://dsva.slack.com/archives/C52CL1PKQ/p1619557476325100?thread_ts=1619536672.303000&cid=C52CL1PKQ
@kelsonic @brianalloyd
Ah one more thing - this ticket to search by word-not-characters
should be applied not only to the intro text field, but to any field being scanned for search, now that we've expanded the surface area of search:
@kelsonic - going back to our very old slack convo - we discussed how we don't want to impact searches so that for e.g., searches on "payments", it ends up ignoring articles with the word "payment". We definitely do not want that.
We simply want to prevent searches from bringing back anything with the letter i
(or whatever letter) anywhere. So could we first try something very narrow to address the edge case of the letter i vs. word I situation?
Whether that is to omit the letter i
by only searching on the word - space-i-space - or by adding the letter i
on our block list of words?
@jenniferlee-dsva implementation-wise I was planning on using Lodash's words
method and removing all instances of I
, and create a config file where you can add any blacklist words (e.g. i
) in the future. Does this align with what you're wanting?
~@kelsonic~ - ~I'm not sure I can ever hope to understand Lodash method, not being an engineer! I believe @ncksllvn already had a blacklisted list of words implemented for RS search. So if there's a way to continue to add on to that, that would probably keep things simpler than having a different blacklist list for RS search.~
~@ncksllvn - do you remember how we previously implemented the exclude-list of words on RS search?~
@jenniferlee-dsva PR merged that moves the experimental sorting logic to production: https://github.com/department-of-veterans-affairs/vets-website/pull/16990 It will be live today following the daily deploy 🙂
Please disregard my comment (struck out) above. Kelson and I have a modified plan in place after our sync.
per convo with Kelson - this ticket has nothing to do with sort order or sort behavior, so I've edited the ticket name for clarity.
PR is up for:
countInstancesFound
method to not count keyword instances by character matches but instead by word matches. e.g. If your query is sit
, then the word deposit
should not be considered a keyword instance match.i
to a new EXPERIMENTAL_SEARCH_IGNORE_LIST
and use that constant to filter here without affecting production.@jenniferlee-dsva Could you let me know when you have time to show you the changes locally before we merge in the PR? 🙂
@kelsonic validated on Prod, looks like results down from 31 to 23 as short logic has been achieved as reviewed during this weeks session. @jenniferlee-dsva let me know if we're good to close this ticket in support.
Validated and closing.
Acceptance Criteria
FIRST:
ONCE ABOVE IS MERGED + DEPLOYED:
countInstancesFound
method to not count keyword instances by character matches but instead by word matches. e.g. If your query issit
, then the worddeposit
should not be considered a keyword instance match.i
to a newEXPERIMENTAL_SEARCH_IGNORE_LIST
and use that constant to filter here without affecting production.JUMP ON A CALL WITH @jenniferlee-dsva BEFORE MERGING TO SHOW HER THE CHANGES! 🎉