Skip to content
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

Modernize redux: eventDetailsSlice #269

Merged
merged 14 commits into from
Mar 4, 2024

Conversation

Arnei
Copy link
Member

@Arnei Arnei commented Jan 12, 2024

Helps with #213.

Switching to redux toolkit for getting event details for the event details modal.

This breaks statistics for the event details! To be fixed after #264 is merged.

(Feels like the event details alone send more different web requests then the rest of the ui combined, sheesh)

Arnei added 4 commits January 11, 2024 11:52
While storing dates as date objects in redux does not
necessarily break anything, it can interfere with some
features (persisting, debugging) and lead to bad practices
(for an encompassing discussion see reduxjs/redux-toolkit#456).

This changes the scheduling info fetching method
to store dates as strings
Turns out even more of the parameters for the addNotification
function in the notificationThunks are perfectly optional.
Fixes a "can't access lexical declaration 'X' before initialization" error,
by resolving a kind of circular dependency.
Switching to redux toolkit for getting event details for the event details modal.
@Arnei Arnei added the type:code-enhancement Internal improvements to the codebase label Jan 12, 2024
Adds types to the redux state for event details. Also
fixes small errors regarding fetching workflow details
introduced in the previous commit.
Copy link
Contributor

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

Copy link
Contributor

@owi92 owi92 left a comment

Choose a reason for hiding this comment

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

Event details modal is working. (See #242 (review) for more info on these approvals)

Copy link
Contributor

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

Copy link
Contributor

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

Copy link
Contributor

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

Statistics were commented out for statistics in the
events details tab. This comments the functionality
back in and fixes it by making use of the new
functionality offeered by statisticsSlice.ts.

Same as for the "Modernize Statistics" PR, I have not
tested this with actual statistics, because setting that
up is very hard for some reason.
Copy link
Contributor

github-actions bot commented Mar 4, 2024

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@Arnei Arnei merged commit 46c95ec into opencast:admin-ui-picard Mar 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:code-enhancement Internal improvements to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants