New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pam 2040 utgåtte søkekriterier #101

Merged
merged 24 commits into from Nov 22, 2018

Conversation

Projects
None yet
3 participants
@tonygj
Contributor

tonygj commented Nov 21, 2018

Hvis et søkekriterie er utgått vises det nederst i det gjeldende filteret, og det er kun mulig å fjerne kriteriet. Fjerner man det blir checkboksen disablet. Det mangler fortsatt litt styling i selve filterene, men ønsker tilbakemelding på funksjonaliteten.

@tonygj tonygj requested review from idaons, otenav and bjornarne Nov 21, 2018

@tonygj tonygj requested a review from navikt/pam-team-tuan as a code owner Nov 21, 2018

@oyvindstegard

This comment has been minimized.

Contributor

oyvindstegard commented Nov 21, 2018

Det er ganske mye kode for å få til dette. Men kanskje jeg ikke spotter helt best hva som er boiler-plate React/redux/style og hva som er reell ny kompleksistet ;).

Noen ting jeg tenker på.

  1. Hvis vi dette går i prod nå, hva skjer når indeksen tilfeldigvis ikke inneholder noen aktive stillinger med gitte lagrede søkekriterier ? Skal da brukeren få beskjed om utdatert kriterium og kun få lov til å fjerne ? Det vil jo ikke stemme.
  2. Jeg synes ikke dette bør tvinge brukeren til noe som helst, hva angår lagrede søk. Hvis brukeren vil beholde utgåtte kriterier, så må det også være lov. Og særlig ift. punkt 2. Det er dumt å være veldig rigid i forhold til hva vi "tillater" brukere å filtrere på. Hvis bruker vil filtrere på kommune "Foo" så bør det være lov. Varsel/hint om mulig utgått-problem med aktuelle kriterier bør være maks. Dette er bare en mening som er min ;).
  3. Kan vi starte forsiktig å kun legge dette på kun for kategorier og geografi. De andre fasettene vil neppe endre seg med det første.
  4. Bør vi starte med dette før vi har "fasit-indeksene" på plass ? Nåværende state i vanlig ad-indeks er ingen fasit på hva som kan komme og hva som ikke lenger er gyldig. Ref punkt 1.
  5. Hvor ofte gjøres disse sjekkene ? Kun når et lagret søk "lastes inn" ?
@tonygj

This comment has been minimized.

Contributor

tonygj commented Nov 21, 2018

Det er mulig det finnes enklere måte å sjekke dette på, og det er litt derfor jeg opprettet denne PR før hele tasken er ferdig. Her er mine tanker rundt det som ble kommentert i forrige kommentar:

  1. Per nå vil det være sånn så lenge det er null stillinger på et kriterie ja, men sånn jeg har skjønt det kommer vi til å vise alle tilgjengelige kriterier med det første slik at dette ikke vil være et problem.
  2. Det er ingen som blir tvunget til noe som helst slik koden er nå. Man får beskjed om at kriteriet er gått ut og man kan velge å ignorere/trykke vekk denne meldingen. Du kan la kriteriet bli værende, redigere andre kriterier og lagre uten å bli stoppet fordi er kriterie mangler.
  3. Det er mulig å legge de på kategori og geografi, men det er disse som er mest kompleks siden disse har en nestet struktur. Som du sier vil ikke de andre fasettene endre seg med det første, så ser ikke helt hvorfor vi skal vente med de. Vil noen av disse fasettene dukke opp med null stillinger?
  4. Vi kan godt vente til "fasit-indeksene" er på plass, men da har man fortsatt problemet med at brukeren ikke får treff og at fasettene som gjør man ikke får treff er skjult.
  5. Det blir sjekket når et søk lastes inn og når man lagrer et søk.
@oyvindstegard

This comment has been minimized.

Contributor

oyvindstegard commented Nov 21, 2018

Det er mulig det finnes enklere måte å sjekke dette på, og det er litt derfor jeg opprettet denne PR før hele tasken er ferdig. Her er mine tanker rundt det som ble kommentert i forrige kommentar:

1. Per nå vil det være sånn så lenge det er null stillinger på et kriterie ja, men sånn jeg har skjønt det kommer vi til å vise alle tilgjengelige kriterier med det første slik at dette ikke vil være et problem.

Hva definerer alle tilgjengelige kriterier, er det alle fasett-verdier som finnes for tiden i Elastic + (union) alle brukerens lagrede verdier for gitt fasett ? Vet ikke om jeg forstår at dette ikke blir et problem.

2. Det er ingen som blir tvunget til noe som helst slik koden er nå. Man får beskjed om at kriteriet er gått ut og man kan velge å ignorere/trykke vekk denne meldingen. Du kan la kriteriet bli værende, redigere andre kriterier og lagre uten å bli stoppet fordi er kriterie mangler.

Bra :)

3. Det er mulig å legge de på kategori og geografi, men det er disse som er mest kompleks siden disse har en nestet struktur. Som du sier vil ikke de andre fasettene endre seg med det første, så ser ikke helt hvorfor vi skal vente med de. Vil noen av disse fasettene dukke opp med null stillinger?

Tenker bare siden kategori/geografi er viktigst, og at vi kanskje ikke trenger legge på slik håndtering på de andre. Da blir det vel mindre kode ?

Verdier vil ikke dukke opp i det hele tatt fra Elastic hvis det er null stillinger, og det er litt av kjernen i problemet her :). Søkefronten vet ikke at verdi A eksisterer før det finnes minst én aktiv stilling med verdi A i feltet. Om det vil inntreffe i praksis er neppe sannsynlig for heltid/deltid/ansettelsesform/sektor, men ikke utenkelig. F.eks. i et testmiljø med bare få indekserte stillinger, synes det er litt uheldig hvis funksjonaliteten er avhengig av at aktive data i søkeindeksen har et visst nivå av mangfold/rikhet for å virke skikkelig.

4. Vi kan godt vente til "fasit-indeksene" er på plass, men da har man fortsatt problemet med at brukeren ikke får treff og at fasettene som gjør man ikke får treff er skjult.

Kan man ikke bare vise alle filtre som er lagret som kriterier, selv om de ikke kommer fra Elastic ?

5. Det blir sjekket når et søk lastes inn og når man lagrer et søk.

Ok

@oyvindstegard

This comment has been minimized.

Contributor

oyvindstegard commented Nov 21, 2018

Prøve å oppsummere litt av essens, for å ikke spore av: det er umulig å unngå potensielt falske varsler til brukere på utgåtte filter-verdier når man ikke har en ekte fasit tilgjengelig. Jeg synes falske varsler her er uheldig, men kanskje det ikke er så farlig. Da er forslaget å begrense denne funksjonaliteten til de viktigste fasettene der endringer er mest sannsynlig, til å begynne med: geografi og kategori, men ikke noe mer.

@otenav

This comment has been minimized.

Contributor

otenav commented Nov 21, 2018

@oyvindstegard Med dagens måte å hente fasetter, så vil detl bli en del falske varsler, i allefall på kommuner som har ingen stillinger på nåværende tidspunkt. Et alternativ er å droppe den store oransje advarselen, men beholde checkboxene?

@otenav otenav merged commit 07508ec into master Nov 22, 2018

@otenav otenav deleted the PAM-2040-utgåtte-søkekriterier branch Nov 22, 2018

@otenav otenav restored the PAM-2040-utgåtte-søkekriterier branch Nov 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment