feat: support code lists search in library

[standeren]

Description

Add functionality to search for code lists in library. The filter method iterates over the verbose codeList object (which also contains the actual code lists) due to enable automatic update of the search result if changing the id of a code list. This is also the reason for needing to keep the search pattern in a state; to be able to use the original search pattern on the new code lists object.

The search is case-insensitive

Skjermopptak.2024-12-19.kl.15.31.26.mov

Related Issue(s)

• Support search for code lists in library #14273

Verification

• Your code builds clean without any errors or warnings
• Manual testing done (required)
• Relevant automated test added (if you find this hard, leave it and we'll help out)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.54%. Comparing base (1545c2d) to head (7073367).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #14319   +/-   ##
=======================================
  Coverage   95.53%   95.54%           
=======================================
  Files        1860     1862    +2     
  Lines       24099    24112   +13     
  Branches     2782     2782           
=======================================
+ Hits        23024    23037   +13     
  Misses        817      817           
  Partials      258      258           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TomasEng]

Fin og konsentrert PR, men jeg har noen spørsmål til valgene du har tatt i koden, spesielt rundt bruken av regulære uttrykk.

Videre lurer jeg på om du har vurdert å gradere resultatene etter hvor godt de matcher søkeordet, i stedet for å bare filtrere ut alt som ikke matcher eksakt? Som bruker ville jeg i hvert fall opplevd det som en feil dersom jeg søkte etter "biler motorsykler" og listen med navnet "biler_og_motorsykler" ikke dukket opp. Samtidig, hvis jeg søkte etter bare "motorsykler", ville jeg forventet at listen med navnet "motorsykler" kom foran "biler_og_motorsykler" i resultatlisten. Jeg sier ikke at dette er ting vi må løse i denne PR-en, men det er i hvert fall noe vi bør tenke på når vi starter å implementere søk.

En komplett søkefunksjon kan altså bli ganske omfattende, men det finnes eksterne biblioteker vi kan bruke for å spare oss for arbeid. (Jeg har faktisk laget et selv, men det er ikke akkurat etablert eller mye brukt, så det strider mot våre egne prinsipper å ta det i bruk.)

[TomasEng]

Hvorfor er det nødvendig å bruke useEffect her?

Husk at vi bruker prefikset handle kun for funksjoner som blir utløst av hendelser, altså props med on-prefiks.

[standeren]

Hvorfor er det nødvendig å bruke useEffect her?

For å lytte til endringer på codeLists dersom man har endret id på en kodeliste, laster opp en ny kodeliste eller sletter en kodeliste. Slik det er implementert nå så må det til fordi hvilke kodelister som matcher søket ligger i en state og det er disse kodelistene som blir vist.
Jeg er veldig åpen for andre løsnigner, men etter litt ulike fremgangsmåter landet jeg på at dette ble best 🤔

Husk at vi bruker prefikset handle kun for funksjoner som blir utløst av hendelser, altså props med on-prefiks.

Er searchCodeLists et bedre navn feks?

[TomasEng]

Vi kan bruke useMemo til å lytte til endringer i props og oppdatere listen deretter. Da blir koden enklere og vi unngår at komponenten laster seg på nytt to ganger på grunn av tilstandsoppdatering i useEffect.

export function CodeListPage({
  codeLists,
  onUpdateCodeListId,
  onUpdateCodeList,
  onUploadCodeList,
}: CodeListPageProps): React.ReactElement {
  const { t } = useTranslation();
  const [searchString, setSearchString] = useState<string>('');

  const filteredCodeLists: CodeListWithMetadata[] = useMemo(
    () => filterCodeLists(codeLists, searchString),
    [codeLists, searchString],
  );

  ...

  return (
    <div className={classes.codeListsContainer}>
      <StudioHeading size='small'>{t('app_content_library.code_lists.page_name')}</StudioHeading>
      <CodeListsCounterMessage codeListsCount={codeLists.length} />
      <CodeListsActionsBar
        onUploadCodeList={handleUploadCodeList}
        onUpdateCodeList={onUpdateCodeList}
        codeListNames={codeListTitles}
        onSetCodeListSearchPattern={setSearchString}
      />
      <CodeLists
        codeLists={filteredCodeLists}
        onUpdateCodeListId={handleUpdateCodeListId}
        onUpdateCodeList={onUpdateCodeList}
        codeListInEditMode={codeListInEditMode}
        codeListNames={codeListTitles}
      />
    </div>
  );
}

Jeg ser for øvrig at det gjøres feilhåndtering i samme komponent. Jeg vil anbefale å holde det på et høyere nivå, slik at vi unngår å måtte nullsjekke codeLists.

[standeren]

Har oppdatert PRen nå.

Feilhåndteringen er allerede løst i en annen PR som jeg har flettet inn nå.

[TomasEng]

Hva er hensikten med denne? Fjerne alle regex-koder? Hvis vi gjør det, hvorfor bruke regex i det hele tatt?

[standeren]

Var vel for å unngå å tillate tegn som kan brukes til å misbruke løsningen, men vet ikke om det er reelt at et søkefelt kan misbrukes 🤷 Evt også dersom man skriver inn ugyldige regex og trigger feil.
Tenkte uansett regex var greit å bruke sammenlignet med includes ettersom at jeg da kunne bruke .* i de tilfellene hvor jeg vil vise alt.
Men absolutt åpen for alternative løsninger.

[TomasEng]

includes blir alltid true når man sender inn en tom streng, så det blir vel enda enklere? Her er et forslag til forenklet funksjon:

export const filterCodeLists = (
  codeLists: CodeListWithMetadata[],
  searchString: string,
): CodeListWithMetadata[] => codeLists.filter((codeList) => codeListMatch(codeList, searchString));

function codeListMatch({ title }: CodeListWithMetadata, searchString: string): boolean {
  return caseInsensitiveMatch(title, searchString);
}

function caseInsensitiveMatch(target: string, searchString: string): boolean {
  const lowerCaseTarget = target.toLowerCase();
  const lowerCaseSearchString = searchString.toLowerCase();
  return lowerCaseTarget.includes(lowerCaseSearchString);
}

Hva synes du?

[standeren]

Ser bra ut det! Endret nå.

[TomasEng]

Lag gjerne en egen fil for denne funksjonen.

[TomasEng]

Jeg antar at dette er for å tilbakestille listen. Har du vurdert å håndtere det utenfor denne funksjonen, f.eks. ved å lage en egen funksjon som du sender inn til onClear?

[standeren]

Ja, kunne sikkert gjort det. Tenkte dette var vel så enkelt ettersom jeg uansett tok i bruk regex. I tillegg vil jo denne funksjonen bli trigget uten at noe søk er lagt til også for å kunne sette alle kodelistene som "søkeresultat". Mener det var mer knot å sette alle kodelistenavnene som søkeresultat og vedlikeholde det resultatet når man gjør endringer på kodelister-objektet ved sletting, id-endring og opplasting 🤔

[TomasEng]

Med endringene jeg har foreslått i de andre kommentarene, tror jeg ikke vi trenger å gjøre noe på onClear i det hele tatt. Resultatlisten vil inneholde alle kodelister når man sender inn en tom streng til søkefunksjonen.

[TomasEng]

Denne bør vi inkludere i vårt eget komponentbibliotek før vi tar den i bruk.

[standeren]

Den er allerede tatt i bruk. Så kanskje løse det i en egen PR?

[TomasEng]

Ja, lag gjerne en egen PR for det.

[standeren]

Her er PR; #14348

[standeren]

Fin og konsentrert PR, men jeg har noen spørsmål til valgene du har tatt i koden, spesielt rundt bruken av regulære uttrykk.

Videre lurer jeg på om du har vurdert å gradere resultatene etter hvor godt de matcher søkeordet, i stedet for å bare filtrere ut alt som ikke matcher eksakt? Som bruker ville jeg i hvert fall opplevd det som en feil dersom jeg søkte etter "biler motorsykler" og listen med navnet "biler_og_motorsykler" ikke dukket opp. Samtidig, hvis jeg søkte etter bare "motorsykler", ville jeg forventet at listen med navnet "motorsykler" kom foran "biler_og_motorsykler" i resultatlisten. Jeg sier ikke at dette er ting vi må løse i denne PR-en, men det er i hvert fall noe vi bør tenke på når vi starter å implementere søk.

En komplett søkefunksjon kan altså bli ganske omfattende, men det finnes eksterne biblioteker vi kan bruke for å spare oss for arbeid. (Jeg har faktisk laget et selv, men det er ikke akkurat etablert eller mye brukt, så det strider mot våre egne prinsipper å ta det i bruk.)

Veldig gode poenger. Har ikke tenkt på at det. Men samtidig vet jeg ikke om jeg hadde forventet det av et søkefelt i en applikasjon som ikke er en søkemotor, men hadde absolutt blitt glad om det funket. Tenker jo kanskje uansett at det er fordelaktig for de som har et par kodelister, at det er mulig å gjøre enkle søk fremfor ingen søk 🤷
Her har kanskje @Annikenkbrathen noen tanker også

[TomasEng]

Tenker jo kanskje uansett at det er fordelaktig for de som har et par kodelister, at det er mulig å gjøre enkle søk fremfor ingen søk

Hvis man bare har et par kodelister, er det vel ikke nødvendig å søke? Dette er vel funksjonalitet som først og fremst er aktuell for brukere med mange kodelister?

[standeren]

Tenker jo kanskje uansett at det er fordelaktig for de som har et par kodelister, at det er mulig å gjøre enkle søk fremfor ingen søk

Hvis man bare har et par kodelister, er det vel ikke nødvendig å søke? Dette er vel funksjonalitet som først og fremst er aktuell for brukere med mange kodelister?

Ja, det var dårlig ordlegging av meg, mente ikke "et par" som et faktisk par, men mange.

[Annikenkbrathen]

Veldig gode poenger. Har ikke tenkt på at det. Men samtidig vet jeg ikke om jeg hadde forventet det av et søkefelt i en applikasjon som ikke er en søkemotor, men hadde absolutt blitt glad om det funket. Tenker jo kanskje uansett at det er fordelaktig for de som har et par kodelister, at det er mulig å gjøre enkle søk fremfor ingen søk 🤷 Her har kanskje @Annikenkbrathen noen tanker også

Enig med dere her! dette blir jo som sagt mest relevant om man har mange kodelister, og da vil nok et enkelt søk hjelpe masse i første omgang!