IS-1710: Add trengerOppfolgingFrist to frist column #383

andersrognstad · 2023-12-01T13:37:08Z

Hva har blitt lagt til✨🌈

Viser trengerOppfolging-frist med eget ikon i fristkolonnen.

Screenshots 📸✨

andersrognstad · 2023-12-01T13:38:46Z

src/components/FristColumn.tsx

-          <Tooltip content="Avventer" arrow={false}>
-            <HourglassTopFilledIcon aria-hidden fontSize="1.5rem" />
+    <>
+      {frister.sort(byFristAsc).map(({ date, icon, tooltip }, index) => (


Viser tidligste frist først hvis det er flere frister i kolonnen for en person.

andersrognstad · 2023-12-01T13:43:18Z

src/utils/hendelseFilteringUtils.tsx

@@ -312,8 +316,14 @@ const sortEventsOnFrist = (
 ): PersonregisterState => {
  const sorted = Object.entries(personregister).sort(
    ([, persondataA], [, persondataB]) => {
-      const fristDateA = persondataA.aktivitetskravVurderingFrist;
-      const fristDateB = persondataB.aktivitetskravVurderingFrist;
+      const fristDateA =


Sammenligner tidligste frist per person dersom det er stigende sortering og ellers seneste frist. Litt usikker på om det er en grei måte å gjøre det på, så setter pris på om flere kan teste lokalt for å se om det gir mening.

Den synes jeg var vanskelig! Ascending synes jeg gir mening, siden det da er den øversten datoen på hver rad som sammenlignes. Men på descending er det den nederste. Men når det kun er én dato, tenker jeg på den som "øverst" 🤔 Det er jo heller ikke helt intuitivt hvordan sorteringen fungerer med de ulike symbolene 🤷‍♂️

Jeg tror det kan bli rart uansett hvis man vil, men i praksis vil vel kanskje de to datoene være ganske like så lenge begge er satt? Og da vil jo sorteringen være "ca riktig" uansett hvilken man ser på?

Mulig man kunne bytte plass på datoene så den nederste kom øverst når det sorteres descending. Evt måtte man bare alltid sortert på den tidligste datoen. Men det blir nok litt rart uansett ja, så lenge man sorterer på en kolonne hvor man kan ha to rader (datoer) "inni" én rad 🥴
Like greit å teste ut dette så får vi se om de synes det er intuitivt :)

eirikdahlen

Det som blir ekstra vanskelig her er vel når de velger et filter, feks trenger oppfølging, og så kommer avventer-datoen øverst 🤔 Når de velger dette filteret, så bryr de seg kanskje ikke om avventer-datoen.

Akkurat i denne casen så flytter ikke radene på seg når jeg prøver å bytte mellom DESC og ASC 😔 Er noe rart, for det samme gjelder for fødselsnummer?

Screen.Recording.2023-12-01.at.15.48.23.mov

test/components/FristColumnTest.tsx

andersrognstad · 2023-12-04T07:11:16Z

Det som blir ekstra vanskelig her er vel når de velger et filter, feks trenger oppfølging, og så kommer avventer-datoen øverst 🤔 Når de velger dette filteret, så bryr de seg kanskje ikke om avventer-datoen.

Akkurat i denne casen så flytter ikke radene på seg når jeg prøver å bytte mellom DESC og ASC 😔 Er noe rart, for det samme gjelder for fødselsnummer?

Screen.Recording.2023-12-01.at.15.48.23.mov

Hmja, det blir kanskje litt rart ja, men vi har vel ikke noe tilpasning av andre kolonner basert på filtrering? Tenker i så fall det får bli videre forbedring.

andersrognstad · 2023-12-04T07:27:15Z

Det som blir ekstra vanskelig her er vel når de velger et filter, feks trenger oppfølging, og så kommer avventer-datoen øverst 🤔 Når de velger dette filteret, så bryr de seg kanskje ikke om avventer-datoen.
Akkurat i denne casen så flytter ikke radene på seg når jeg prøver å bytte mellom DESC og ASC 😔 Er noe rart, for det samme gjelder for fødselsnummer?
Screen.Recording.2023-12-01.at.15.48.23.mov

Hmja, det blir kanskje litt rart ja, men vi har vel ikke noe tilpasning av andre kolonner basert på filtrering? Tenker i så fall det får bli videre forbedring.

Det som blir ekstra vanskelig her er vel når de velger et filter, feks trenger oppfølging, og så kommer avventer-datoen øverst 🤔 Når de velger dette filteret, så bryr de seg kanskje ikke om avventer-datoen.

Akkurat i denne casen så flytter ikke radene på seg når jeg prøver å bytte mellom DESC og ASC 😔 Er noe rart, for det samme gjelder for fødselsnummer?

Screen.Recording.2023-12-01.at.15.48.23.mov

Jeg klarer ikke helt å gjenskape buggen her. Har filtrert på Vurdert for oppfølging og fødselsnummer-radene flytter ikke på seg, men frist funker bra 🤔

eirikdahlen · 2023-12-04T08:01:31Z

Det som blir ekstra vanskelig her er vel når de velger et filter, feks trenger oppfølging, og så kommer avventer-datoen øverst 🤔 Når de velger dette filteret, så bryr de seg kanskje ikke om avventer-datoen.
Akkurat i denne casen så flytter ikke radene på seg når jeg prøver å bytte mellom DESC og ASC 😔 Er noe rart, for det samme gjelder for fødselsnummer?
Screen.Recording.2023-12-01.at.15.48.23.mov

Hmja, det blir kanskje litt rart ja, men vi har vel ikke noe tilpasning av andre kolonner basert på filtrering? Tenker i så fall det får bli videre forbedring.

Det som blir ekstra vanskelig her er vel når de velger et filter, feks trenger oppfølging, og så kommer avventer-datoen øverst 🤔 Når de velger dette filteret, så bryr de seg kanskje ikke om avventer-datoen.
Akkurat i denne casen så flytter ikke radene på seg når jeg prøver å bytte mellom DESC og ASC 😔 Er noe rart, for det samme gjelder for fødselsnummer?
Screen.Recording.2023-12-01.at.15.48.23.mov

Jeg klarer ikke helt å gjenskape buggen her. Har filtrert på Vurdert for oppfølging og fødselsnummer-radene flytter ikke på seg, men frist funker bra 🤔

Ja, litt rart... Burde nok gå over til Aksel-Table snart 😬

JMLindseth · 2023-12-04T14:23:43Z

src/utils/personDataUtil.ts

+  return aktivitetskravVurderingFrist
+    ? aktivitetskravVurderingFrist
+    : trengerOppfolgingFrist;


Denne synes jeg ikke var så lett å lese 🤔 Det er ikke helt åpenbart at man i praksis returnerer aktivitetskravVurderingFrist eller (trengerOppfolgingFrist eller null).
Jeg leste det som at man antar at hvis aktivitetskravVurderingFrist er null, er trengerOppfolgingFrist ikke null, men det stemmer jo ikke, siden man kan får null fra denne metoden 🤷

Hm nei.. hadde det vært bedre med
return aktivitetskravVurderingFrist || trengerOppfolgingFrist;
på linje 57 der? Eller har du annet forslag? 🙂

Kanskje return aktivitetskravVurderingFrist || trengerOppfolgingFrist || null? Det blir jo egentlig overflødig, men i hvert fall mer eksplisitt 🤷‍♂️

Eller så kan det sikkert være som det er, jeg skjønte det jo etter hvert 👍

JMLindseth · 2023-12-04T14:26:15Z

test/components/FristColumnTest.tsx

+    const personAvventerMedFrist: PersonData = { ...defaultPersonData };
+    render(<FristColumn personData={personAvventerMedFrist} />);
+
+    expect(screen.queryAllByText(/2023/)).to.be.empty;


Hvis noen endrer testdata til å være 2024 neste år, vil denne testen bli grønn uansett? 🤔

Mm, pushet en forbedring av testene nå 👍

JMLindseth · 2023-12-04T14:28:35Z

test/components/FristColumnTest.tsx

+  it('viser ingen frister når person har hverken aktivitetskrav AVVENT med frist eller trenger oppfolging med frist', () => {
+    const personAvventerMedFrist: PersonData = { ...defaultPersonData };


Testen tester at man ikke har avvent med frist, men variabelen sier at man har avvent med frist?

JMLindseth · 2023-12-04T14:29:54Z

test/components/FristColumnTest.tsx

+  it('viser frist for person når aktivitetskrav AVVENT med frist', () => {
+    const personAvventerMedFrist: PersonData = {
+      ...defaultPersonData,
+      aktivitetskrav: AktivitetskravStatus.AVVENT,
+      aktivitetskravVurderingFrist: new Date('2023-12-18'),
+    };
+    render(<FristColumn personData={personAvventerMedFrist} />);
+
+    expect(screen.getByText('18.12.2023')).to.exist;
+  });


Trenger vi å teste at personen har akt.kravfrist, men ikke status avvent? 🤔

JMLindseth · 2023-12-04T14:33:48Z

test/utils/hendelseFilteringUtilsTest.ts

Gjerne legg inn en tom linje mellom iter, og evt. del opp testene i given/when/then, så blir det litt enklere å lese 👍

andersrognstad requested a review from a team as a code owner December 1, 2023 13:37

andersrognstad commented Dec 1, 2023

View reviewed changes

eirikdahlen reviewed Dec 1, 2023

View reviewed changes

test/components/FristColumnTest.tsx Outdated Show resolved Hide resolved

andersrognstad force-pushed the IS-1710-oppfolging-frist branch from 2b8c1d7 to 448d3a7 Compare December 4, 2023 07:07

eirikdahlen approved these changes Dec 4, 2023

View reviewed changes

JMLindseth reviewed Dec 4, 2023

View reviewed changes

IS-1710: Add trengerOppfolgingFrist to frist column

3ae3705

andersrognstad force-pushed the IS-1710-oppfolging-frist branch from 448d3a7 to 3ae3705 Compare December 4, 2023 19:09

andersrognstad added 2 commits December 5, 2023 09:01

Fix PR feedback

5ac5ce4

Fix pr feedback

68dd847

JMLindseth approved these changes Dec 5, 2023

View reviewed changes

andersrognstad merged commit 92709c7 into master Dec 5, 2023
3 checks passed

andersrognstad deleted the IS-1710-oppfolging-frist branch December 5, 2023 11:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IS-1710: Add trengerOppfolgingFrist to frist column #383

IS-1710: Add trengerOppfolgingFrist to frist column #383

andersrognstad commented Dec 1, 2023

andersrognstad Dec 1, 2023

andersrognstad Dec 1, 2023

JMLindseth Dec 4, 2023

andersrognstad Dec 4, 2023

eirikdahlen left a comment •

edited

Loading

andersrognstad commented Dec 4, 2023

andersrognstad commented Dec 4, 2023

eirikdahlen commented Dec 4, 2023

JMLindseth Dec 4, 2023 •

edited

Loading

andersrognstad Dec 5, 2023

JMLindseth Dec 5, 2023

JMLindseth Dec 5, 2023

JMLindseth Dec 4, 2023

andersrognstad Dec 5, 2023

JMLindseth Dec 4, 2023

JMLindseth Dec 4, 2023

JMLindseth Dec 4, 2023

		it('viser ingen frister når person har hverken aktivitetskrav AVVENT med frist eller trenger oppfolging med frist', () => {
		const personAvventerMedFrist: PersonData = { ...defaultPersonData };

IS-1710: Add trengerOppfolgingFrist to frist column #383

IS-1710: Add trengerOppfolgingFrist to frist column #383

Conversation

andersrognstad commented Dec 1, 2023

Hva har blitt lagt til✨🌈

Screenshots 📸✨

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eirikdahlen left a comment • edited Loading

Choose a reason for hiding this comment

andersrognstad commented Dec 4, 2023

andersrognstad commented Dec 4, 2023

eirikdahlen commented Dec 4, 2023

JMLindseth Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eirikdahlen left a comment •

edited

Loading

JMLindseth Dec 4, 2023 •

edited

Loading