Refactor and fix types in events redux code #4977

eikhr · 2024-09-22T22:00:47Z

Description

Improves types and simplifies redux code related to events. Fixes most of the typescript errors in events code.

Number of typescript errors: 474 -> 371

Result

No visual changes.
Except that I fixed two small bugs:

When unregistering people from the administrate view, the button loading state is now set correctly
The registration countdown no longer shows 0 seconds left (mistake from Simplify event detail page by moving stuff into components #4974)

Testing

I have thoroughly tested my changes.

The redux code is fairly well tested using unit tests, these still pass of course. I have also manually tested by opening pages and testing some core functionality.

vercel · 2024-09-22T22:00:51Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
lego-bricks-storybook	⬜️ Ignored (Inspect)	Visit Preview		Sep 23, 2024 1:47pm

eikhr · 2024-09-23T09:17:49Z

app/routes/events/components/EventAdministrate/Abacard.tsx

-  usePreparedEffect(
-    'fetchEventAbacard',
-    () => {
-      if (query && typeof query === 'string') {
-        return dispatch(autocomplete(query, ['users.user']));
-      }
-    },
-    [query],
-  );


I don't know what this was supposed to do. typeof query is never a string

you never know with this webapp! 😆

eikhr · 2024-09-23T09:21:27Z

app/routes/events/components/EventDetail/infoLists.tsx

This is all moved out of EventDetail with very minor changes

Also fix issues in createLegoAdapter causing type errors

It looks a bit weird when it shows 0 seconds left

norbye

Whop didn't get to review everything, but I'll submit my one comment

norbye · 2024-09-24T11:29:06Z

app/reducers/registrations.ts

-  initialState: legoAdapter.getInitialState(),
+  initialState: legoAdapter.getInitialState({
+    test: 1,
+  }),


What's the purpose of this test: 1?

well, heh, it was for testing the typescript types...
Fixed in #5002

ivarnakken

Looks much cleaner! 😍 You've done an awesome job here! I love the custom hooks

ivarnakken · 2024-09-25T12:55:26Z

app/routes/events/components/JoinEventForm.tsx

@@ -402,7 +415,7 @@ const JoinEventForm = ({

                        {event.activationTime && registrationOpensIn && (
                          <Button disabled={disabledButton}>
-                            {`Åpner om ${moment(registrationOpensIn.asMilliseconds()).format('mm:ss')}`}
+                            {`Åpner om ${moment(registrationOpensIn.add(1, 'second').asMilliseconds()).format('mm:ss')}`}


Isn't the add supposed to be on the moment object instead of registrationOpensIn?

It's ugly but it works correctly now. We add one second to the displayed time left until it opens, so that the countdown will end after showing 00:01 instead of showing 00:00 for one second. Normally it rounds down the number of seconds left.

I get that, but I was wondering if it was more "right" to do

Suggested change

{`Åpner om ${moment(registrationOpensIn.add(1, 'second').asMilliseconds()).format('mm:ss')}`}

{`Åpner om ${moment(registrationOpensIn).add(1, 'second').asMilliseconds().format('mm:ss')}`}

Oh, right of course. No, I don't think so, this does something weird to get the formatting we want. registrationOpensIn is a duration, we add one second to this duration, and then we change it into a moment which is more of a timestamp (I think?). And then we format this as "mm:ss" to get the countdown.
I'm not sure how it works, but I think it is correct now. The duration (registrationOpensIn) doesn't have .format, so we need to do moment() and I'm not sure if .add would work the same after doing moment()

ivarnakken · 2024-09-25T12:57:00Z

app/routes/events/components/JoinEventForm.tsx

@@ -385,6 +397,7 @@ const JoinEventForm = ({
                      !registrationPending &&
                      !registration &&
                      captchaOpen &&
+                      config.environment !== 'ci' &&


This could probably just be on the Captcha Field itself

ivarnakken · 2024-09-25T13:01:50Z

app/store/models/Event.ts


  unansweredSurveys: EntityId[];
+
+  actionGrant: ActionGrant;


What's up with the spacings

idk, sometimes I've put some spacing to separate admin-stuff from normal stuff f.ex. But this one seems unnecessary

ivarnakken · 2024-09-27T14:28:49Z

app/routes/users/components/UserProfile.tsx

+    selectAllEvents<ListEventWithUserRegistration>(state, {
+      pagination: previousEventsPagination,
+    }),
+  ).filter((e) => e.userReg);


Why is this filter only done on previous events?

I think it filters out any events weher you were on the waiting list. For upcoming events we want to show these too.

ivarnakken · 2024-09-27T14:29:55Z

app/routes/users/components/UserProfile.tsx

+  ).filter((e) => e.userReg);
+
+  const sortedPreviousEvents = previousEvents
+    ?.filter((e) => !!e.userReg?.pool)


I suppose e.userReg will never be undefined since you've already filtered it out above

Agree, but typescript is dumb

ivarnakken · 2024-09-27T14:31:15Z

app/routes/surveys/components/SurveyList/SurveyItem.tsx

              For arrangement{' '}
-              <Link to={`/events/${event.id}`}>{event.title}</Link>
+              <Link to={`/events/${event?.id}`}>{event?.title}</Link>


Kinda useless to only show "For arrangement" if event is undefined

Yeah, I think all surveys have an event. But it will probably look a bit weird while loading:/

ivarnakken · 2024-09-27T14:35:24Z

app/routes/events/components/EventEditor/index.tsx

  const params = useParams<{
    eventIdOrSlug: string;
-  }>();
+  }>() as {
+    eventIdOrSlug: string;
+  };


Is this really how we have to do this now 💀

Argh, yes... I don't know why they changed this to make all params optional, it worked fine in react-router-v5. And the params are required, there is no way to reach this path without specifying an event id...

I have a plan to make a custom hook like useRequiredParams or something, that just typecasts this a bit prettier.

haha oh well 🤷🏼‍♂️

app/routes/events/components/EventAdministrate/Attendees.tsx

ivarnakken · 2024-09-27T14:44:04Z

app/routes/events/components/EventAdministrate/Abacard.tsx

-  usePreparedEffect(
-    'fetchEventAbacard',
-    () => {
-      if (query && typeof query === 'string') {
-        return dispatch(autocomplete(query, ['users.user']));
-      }
-    },
-    [query],
-  );


you never know with this webapp! 😆

github-actions bot added the review-needed Pull requests that need review label Sep 22, 2024

eikhr force-pushed the improve-redux-events branch 2 times, most recently from 95a02a4 to c00ec5c Compare September 23, 2024 09:01

eikhr changed the title ~~Move interested button into a separate component~~ Refactor and fix types in events redux code Sep 23, 2024

eikhr requested review from ivarnakken, norbye, juniwbjerde, falbru and jonasdeluna and removed request for ivarnakken and norbye September 23, 2024 09:05

eikhr marked this pull request as ready for review September 23, 2024 09:06

eikhr commented Sep 23, 2024

View reviewed changes

app/routes/events/components/EventDetail/infoLists.tsx Outdated

Copy link

Member Author

eikhr Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all moved out of EventDetail with very minor changes

eikhr added 8 commits September 23, 2024 15:46

Use default legoAdapter selectors for selecting lists of events

f025d64

Use non-tranformed event in event detail page

db299c9

Use non-tranformed event in event edit page

9f263c8

Clean up pool and registration types

ad98bf9

Also fix issues in createLegoAdapter causing type errors

Simplify registration types

3968f05

Remove last usages of transformed event selector

731b526

Fix types in event reducer cases

0d35a86

Add one second to registration countdown button

a0f7908

It looks a bit weird when it shows 0 seconds left

eikhr force-pushed the improve-redux-events branch from 5d27c22 to a0f7908 Compare September 23, 2024 13:46

eikhr mentioned this pull request Sep 23, 2024

Show waiting list position even if there are multiple pools #4983

Merged

4 tasks

falbru approved these changes Sep 23, 2024

View reviewed changes

eikhr merged commit 8ae9420 into master Sep 25, 2024
6 checks passed

eikhr deleted the improve-redux-events branch September 25, 2024 11:03

norbye reviewed Sep 26, 2024

View reviewed changes

eikhr mentioned this pull request Sep 26, 2024

Hide registration countdown button when already admin-registered #5002

Merged

1 task

ivarnakken reviewed Sep 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor and fix types in events redux code #4977

Refactor and fix types in events redux code #4977

eikhr commented Sep 22, 2024 •

edited

Loading

vercel bot commented Sep 22, 2024 •

edited

Loading

eikhr Sep 23, 2024

ivarnakken Sep 27, 2024

eikhr Sep 23, 2024

norbye left a comment

norbye Sep 24, 2024

eikhr Sep 26, 2024

ivarnakken left a comment •

edited

Loading

ivarnakken Sep 25, 2024

eikhr Sep 30, 2024

ivarnakken Sep 30, 2024

eikhr Sep 30, 2024

ivarnakken Sep 25, 2024

ivarnakken Sep 25, 2024

eikhr Sep 30, 2024

ivarnakken Sep 27, 2024

eikhr Sep 30, 2024

ivarnakken Sep 27, 2024

eikhr Sep 30, 2024

ivarnakken Sep 27, 2024

eikhr Sep 30, 2024

ivarnakken Sep 27, 2024

eikhr Sep 30, 2024

ivarnakken Sep 30, 2024

ivarnakken Sep 27, 2024

	{`Åpner om ${moment(registrationOpensIn.add(1, 'second').asMilliseconds()).format('mm:ss')}`}
	{`Åpner om ${moment(registrationOpensIn).add(1, 'second').asMilliseconds().format('mm:ss')}`}

Refactor and fix types in events redux code #4977

Refactor and fix types in events redux code #4977

Conversation

eikhr commented Sep 22, 2024 • edited Loading

Description

Result

Testing

vercel bot commented Sep 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norbye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivarnakken left a comment • 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

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

eikhr commented Sep 22, 2024 •

edited

Loading

vercel bot commented Sep 22, 2024 •

edited

Loading

ivarnakken left a comment •

edited

Loading