improve PosterHall search by adjusting fuse.js options + related #1460

mike-lvov · 2021-05-27T13:40:14Z

Note: This is a re-created PR (to fix CI issues/etc) for the work tested & developed by @yarikoptic in #1443

Edit: While the original PR accidentally overwrote the commit history, that has since been fixed and restored. Details in #1460 (comment)

This is basically a copy of #1443 + some additions/tweaks.

The reason for copying it was to test it & slightly clean up the code

closes #1391
closes #1453

Original PR Description

At large closes #1391 (Edit by @0xdevalias: also closes https://github.com/sparkletown/internal-sparkle-issues/issues/745) although remains notably suboptimal. With settings @margulies came up with seems to go a few steps further than my exploration (initial commit)

additional explicit trimming of the search query (we aren't into Whitespace here aren't we? but adding a space trailing space has an immediate detrimental effect)

split by space into words and search for each within $and across all keys. See below for more info on how plain fuse.js seems not capable of that

allow for " " to enclose phrase to not be split

With the above changes search becomes pragmatically useful! That splitting resolved following gotchas:

LastName FirstName would return no result whenever FirstName LastName is ok.

results often make no real sense from UX PoV: e.g. there is a poster by Tsai with "connectivity" in the title. Entering Tsai - shows the poster. Entering "Tsai connectivity" shows bunch of posters none of which is of Tsai

edits:

I think this is the most related issue Match all tokens should look in different keys as well. krisk/Fuse#235 (closed without resolution pointing to built-in deficiency of fuse.js for our use case)

to workaround people breed monsters like Match all tokens should look in different keys as well. krisk/Fuse#235 (comment)

the approach of the bot closing issues without resolution there is really annoying, keep running into them (tokenize was removed but no changelog Can you add information about the "tokenize" option? krisk/Fuse#511, so not clear why/how to workaround... started to be removed in krisk/Fuse@4343f71 in favor of "extended" mode I guess; but in extended mode you seems need to specify fields for logical operation)

with that in mind it might need some JS foo to first split query string, and prep it to be fed to "advanced" mode

may be dirty sufficient workaround would be to meld all 3 fields into one to give to fuse

or just perform separate queries per each split term and take intersection

… keys

now search works more sensibly so no need to restrict

…p equal

kept name as is so mortals could also enter posterX and get result. Makes it also fun - with fuzzy finds then posters in "near" number

codeclimate · 2021-05-27T13:41:11Z

Code Climate has analyzed commit 95f4c39 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	1
Duplication	2

View more on Code Climate.

github-actions · 2021-05-27T13:45:04Z

Visit the preview URL for this PR (updated for commit 11cca35):

https://co-reality-staging--preview-pr-1460-601g4q77.web.app

_{(expires Fri, 11 Jun 2021 08:04:55 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

yurii-lubynets

LGTM

yarikoptic · 2021-05-27T14:23:07Z

FWIW: 👍 from me! ;-)
Note: Would close #1453 ~~a yet not filed issue~~ about "More posters" button persistently visible.

src/hooks/posters.ts

yarikoptic · 2021-05-27T16:04:16Z

Note: Would close #1453 ~~a yet not filed issue~~ about "More posters" button persistently visible.

FWIW -- confirming! (after full reload to get rid of Memos)

src/hooks/posters.ts

mike-lvov · 2021-05-27T21:27:55Z

@0xdevalias you did the review on a bit outdated code

0xdevalias · 2021-05-27T21:36:40Z

@0xdevalias you did the review on a bit outdated code

@mike-lvov It wasn’t at the time I was doing it. It wasn’t intended as a deep thorough review anyway, more to point out some things I noticed on my phone while in bed.

0xdevalias · 2021-05-28T05:21:17Z

RE: #1460 (comment)

If @mike-lvov had a moment: for this solution I did $or across all keys unconditionally.
But if split words started with "{key}:" , restricting only to that key (with making to more obscure DB names), would've been awesome. Then I could say eg

@yarikoptic I'm going to call that OOS for this PR in the interest of landing it sooner and avoiding scope creep. If you wanted to open a new backlog issue describing the request, we can handle that as a potential future improvement.

Edit: Backlogged in #1467

0xdevalias

[CRO] @mike-lvov I'm going to preemptively approve this to unblock you, as I don't see any huge issues, but note that I haven't tested it yet, and there are a few notes in here that are probably worth checking/fixing before it gets merged.

Edit: The discussion in #1460 (comment) is probably still worth checking through + potentially fixing still.

src/hooks/posters.ts

src/utils/text.ts

src/hooks/posters.ts

src/components/templates/PosterHall/PosterHall.tsx

src/hooks/posters.ts

0xdevalias · 2021-05-28T08:08:54Z

Copying this out of the comment thread for visibility purposes:

In the interest of getting this merged, I'm going to call it OOS to muse on/change the regex/splitting method anymore here.

I've added a @debt comment to it in 11cca35 referencing github/codeql#5964 so that we have the relevant details captured there in future if needed.

Originally posted by @0xdevalias in #1460 (comment)

Changes have been resolved

0xdevalias · 2021-05-28T08:53:34Z

Thanks for all of your work on this @yarikoptic! (and @mike-lvov for your additional cleanup/polish) 🎉

yarikoptic · 2021-05-28T12:51:50Z

Woohoo -- thank you @mike-lvov and @0xdevalias !!! de-@dept (it was a false positive) PR #1470

* origin/staging: add my engineering QoL helper scripts (sparkletown#1466) improve PosterHall search by adjusting fuse.js options + related (sparkletown#1460) add ability to share a PosterPage on social media (sparkletown#1396) add optional scheduleStartDate to DB + force NavBarSchedule to start from that date when present (sparkletown#1419) Conflicts: src/hooks/posters.ts - replaced sampleSize with shuffle around the entire set

* origin/staging: Fixes object freezing err (sparkletown#1474) add chat 'question' mode to highlight questions in the chat (sparkletown#1424) Add chat 'poll' mode + related ChatPoll feature (sparkletown#1343) add loading when editing venue table (sparkletown#1473) add EventModal + cleanup from new schedule implementation (sparkletown#1426) add sorting + proper 'show more' button in ScreeningRoom (sparkletown#1464) add table topic manage functionality (sparkletown#1377) Remove @DEPT - it was a false positive (sparkletown#1470) refactor openUrl/openRoomUrl/enterVenue to allow custom openers + use React Router for navigation in PosterPreview / NavBar's back button (sparkletown#1468) add my engineering QoL helper scripts (sparkletown#1466) improve PosterHall search by adjusting fuse.js options + related (sparkletown#1460) add ability to share a PosterPage on social media (sparkletown#1396) add optional scheduleStartDate to DB + force NavBarSchedule to start from that date when present (sparkletown#1419) ENH: now that posters loading/display is fast - boost to 48 (sparkletown#1445) fix logic in Auditorium (v1) where row/column 0 is treated as false instead of being a valid seat (sparkletown#1439) Conflicts: seems still trivialto resolve src/components/molecules/NavSearchBar/NavSearchBar.tsx src/components/organisms/ChatSidebar/components/VenueChat/VenueChat.tsx src/settings.ts

…-export * upstream/staging: remove venue hook from party map template (sparkletown#1482) Fix profile questions page (sparkletown#1481) Fixes object freezing err (sparkletown#1474) add chat 'question' mode to highlight questions in the chat (sparkletown#1424) Add chat 'poll' mode + related ChatPoll feature (sparkletown#1343) add loading when editing venue table (sparkletown#1473) add EventModal + cleanup from new schedule implementation (sparkletown#1426) add sorting + proper 'show more' button in ScreeningRoom (sparkletown#1464) add table topic manage functionality (sparkletown#1377) Remove @DEPT - it was a false positive (sparkletown#1470) refactor openUrl/openRoomUrl/enterVenue to allow custom openers + use React Router for navigation in PosterPreview / NavBar's back button (sparkletown#1468) add my engineering QoL helper scripts (sparkletown#1466) improve PosterHall search by adjusting fuse.js options + related (sparkletown#1460) add ability to share a PosterPage on social media (sparkletown#1396)

yarikoptic and others added 12 commits May 25, 2021 20:09

WiP: trying to make sense of fuse options

0191602

ENH: Settings for search from @margulies

d8b1141

Trim search query so adding space does not "break" the search

520e5bf

ENH: make search useful by searching on separate words and across all…

56c8586

… keys

ENH: allow to put phrases into "" to not be splitted

b59da36

ENH: remove restriction of at least 3 letter matches

3a749cd

now search works more sensibly so no need to restrict

ENH: regex - make . into [^"] to avoid exponential matching

392007f

no weights: we do not know what user's intent is, so I would just kee…

ee90ba7

…p equal

ENH: also search by poster number

ecad257

kept name as is so mortals could also enter posterX and get result. Makes it also fun - with fuzzy finds then posters in "near" number

obey to the prettify

f693e63

BF: safe-guard to not pass empty string to fuse -- it does not like it

2aba0c5

Merge branch 'staging' into enh-search

8064c56

mike-lvov requested a review from a team May 27, 2021 13:40

mike-lvov self-assigned this May 27, 2021

mike-lvov temporarily deployed to feature-preview May 27, 2021 13:40 Inactive

mike-lvov mentioned this pull request May 27, 2021

ENH: provide fuse.js options to improve search for some adequacy #1443

Closed

mike-lvov temporarily deployed to feature-preview May 27, 2021 14:04 Inactive

yurii-lubynets approved these changes May 27, 2021

View reviewed changes

yarikoptic mentioned this pull request May 27, 2021

[env/ohbm] refactor usePosters to display a random selection of posters when no searchQuery #1452

Closed

denisdimitrov previously requested changes May 27, 2021

View reviewed changes

src/hooks/posters.ts Outdated Show resolved Hide resolved

src/hooks/posters.ts Outdated Show resolved Hide resolved

src/hooks/posters.ts Show resolved Hide resolved

yarikoptic approved these changes May 27, 2021

View reviewed changes

mike-lvov temporarily deployed to feature-preview May 27, 2021 21:22 Inactive

mike-lvov temporarily deployed to feature-preview May 27, 2021 21:24 Inactive

0xdevalias reviewed May 27, 2021

View reviewed changes

src/hooks/posters.ts Show resolved Hide resolved

src/hooks/posters.ts Show resolved Hide resolved

src/hooks/posters.ts Outdated Show resolved Hide resolved

0xdevalias self-assigned this May 28, 2021

0xdevalias added the 💪 enhancement Enhancements/improvements to existing functionality label May 28, 2021

0xdevalias changed the title ~~Improve poster hall search~~ improve poster hall search by adjusting fuse.js options + related May 28, 2021

0xdevalias changed the title ~~improve poster hall search by adjusting fuse.js options + related~~ improve PosterHall search by adjusting fuse.js options + related May 28, 2021

0xdevalias mentioned this pull request May 28, 2021

PosterHall 'show more posters' is shown even when there are no more posters to be displayed #1453

Closed

0xdevalias approved these changes May 28, 2021

View reviewed changes

fix typing hack

5668677

0xdevalias temporarily deployed to feature-preview May 28, 2021 07:20 Inactive

extract logic into shouldShowMorePosters

8051f61

0xdevalias temporarily deployed to feature-preview May 28, 2021 07:23 Inactive

cleanup variable names + add documentation comments

dd07998

0xdevalias temporarily deployed to feature-preview May 28, 2021 07:42 Inactive

add fuse options documentation link

9d293ac

0xdevalias temporarily deployed to feature-preview May 28, 2021 07:45 Inactive

0xdevalias mentioned this pull request May 28, 2021

LGTM.com - false positive javascript/ql/src/Performance/ReDoS.ql github/codeql#5964

Closed

add @debt comment to tokeniseStringWithQuotesBySpaces regex

11cca35

0xdevalias temporarily deployed to feature-preview May 28, 2021 08:02 Inactive

0xdevalias mentioned this pull request May 28, 2021

[PosterHall] We could add search token prefixes like author:, etc #1467

Open

0xdevalias merged commit c0e7e40 into staging May 28, 2021

0xdevalias deleted the feature/improve-posterhall-search branch May 28, 2021 08:52

0xdevalias mentioned this pull request May 28, 2021

Match all tokens should look in different keys as well. krisk/Fuse#235

Closed

mike-lvov mentioned this pull request May 28, 2021

deploy staging -> master #1472

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve PosterHall search by adjusting fuse.js options + related #1460

improve PosterHall search by adjusting fuse.js options + related #1460

mike-lvov commented May 27, 2021 •

edited by 0xdevalias

Loading

codeclimate bot commented May 27, 2021 •

edited

Loading

github-actions bot commented May 27, 2021 •

edited

Loading

yurii-lubynets left a comment

yarikoptic commented May 27, 2021 •

edited

Loading

yarikoptic commented May 27, 2021

mike-lvov commented May 27, 2021

0xdevalias commented May 27, 2021

0xdevalias commented May 28, 2021 •

edited

Loading

0xdevalias left a comment •

edited

Loading

0xdevalias commented May 28, 2021 •

edited

Loading

0xdevalias commented May 28, 2021

yarikoptic commented May 28, 2021 •

edited

Loading

improve PosterHall search by adjusting fuse.js options + related #1460

improve PosterHall search by adjusting fuse.js options + related #1460

Conversation

mike-lvov commented May 27, 2021 • edited by 0xdevalias Loading

Original PR Description

codeclimate bot commented May 27, 2021 • edited Loading

github-actions bot commented May 27, 2021 • edited Loading

yurii-lubynets left a comment

Choose a reason for hiding this comment

yarikoptic commented May 27, 2021 • edited Loading

yarikoptic commented May 27, 2021

mike-lvov commented May 27, 2021

0xdevalias commented May 27, 2021

0xdevalias commented May 28, 2021 • edited Loading

0xdevalias left a comment • edited Loading

Choose a reason for hiding this comment

0xdevalias commented May 28, 2021 • edited Loading

0xdevalias commented May 28, 2021

yarikoptic commented May 28, 2021 • edited Loading

mike-lvov commented May 27, 2021 •

edited by 0xdevalias

Loading

codeclimate bot commented May 27, 2021 •

edited

Loading

github-actions bot commented May 27, 2021 •

edited

Loading

yarikoptic commented May 27, 2021 •

edited

Loading

0xdevalias commented May 28, 2021 •

edited

Loading

0xdevalias left a comment •

edited

Loading

0xdevalias commented May 28, 2021 •

edited

Loading

yarikoptic commented May 28, 2021 •

edited

Loading