-
Notifications
You must be signed in to change notification settings - Fork 41
Show registration modal only when unregistered #1600
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
base: r/19.x
Are you sure you want to change the base?
Changes from 1 commit
48ae5a7
c12f608
45f695b
92f70b3
094d207
6895fad
75e3a11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { RootState } from "../store"; | ||
|
|
||
| /** | ||
| * This file contains selectors regarding information about the registration status | ||
| */ | ||
| // Are we registered at all | ||
| export const getRegistration = (state: RootState) => state.registration.registration; | ||
| // Are we able to talk to register.opencast.org | ||
| export const getIsRegistering = (state: RootState) => state.registration.isRegistering; | ||
| // Does our registration match the latest ToU on the core | ||
| export const getAgreedLatestToU = (state: RootState) => state.registration.agreedToToU; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| import { PayloadAction, createSlice } from "@reduxjs/toolkit"; | ||
| import axios from "axios"; | ||
| import { WritableDraft } from "immer"; | ||
| import { createAppAsyncThunk } from "../createAsyncThunkWithTypes"; | ||
|
|
||
| export type Registration = { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since all this PR really does is checking if ´/admin-ng/adopter/registration` returns null or not, creating a whole redux state is complete overkill. If you'll need this for some other PR, would it not be better to add it then?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair. The goal here is to be able to set warnings in the notification bell in a few cases. I'll rework this and come back when it's ready.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this more what you're looking for @Arnei? Or are you wanting me to completely remove the slice and selectors? There's an
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking of completely removing the slice and selectors and stuff, and do something like in About.tsx, aka just do the state thing locally in the component. If you plan to make use of redux state in a later PR, it might make sense to use redux right now already, so you don't have to do work twice. But as a reviewer, I can't know that :D |
||
| adopterKey: string, | ||
| statisticsKey: string, | ||
| organisationName: string, | ||
| departmentName: string, | ||
| firstName: string, | ||
| lastName: string, | ||
| email: string, | ||
| country: string, | ||
| postalCode: string, | ||
| street: string, | ||
| streetNo: string, | ||
| contactMe: boolean, | ||
| systemType: string, | ||
| allowsStatistics: boolean, | ||
| allowsErrorReports: boolean, | ||
| dateCreated: string, | ||
| dateUpdated: string, | ||
| agreedToPolicy: boolean, | ||
| registered: boolean, | ||
| termsVersionAgreed: string, | ||
| deleteMe: boolean, | ||
| } | ||
|
|
||
| export type Summary = { | ||
| general: Registration, | ||
| statistics: { | ||
| statistics_key: string, | ||
| adopter_key: string, | ||
| job_count: number, | ||
| event_count: number, | ||
| series_count: number, | ||
| user_count: number, | ||
| ca_count: number, | ||
| total_minutes: number, | ||
| tenant_count: number, | ||
| hosts: { | ||
| cores: number, | ||
| max_load: number, | ||
| memory: number, | ||
| hostname: string, | ||
| disk_space: number, | ||
| services: string, | ||
| }[], | ||
| version: string | ||
| } | ||
| } | ||
|
|
||
| export type RegistrationState = { | ||
| registration: Registration | null, | ||
| summary: Summary | null, | ||
| latestToU: string, | ||
| isRegistering: boolean, | ||
| agreedToToU: boolean, | ||
| error: boolean | ||
| }; | ||
|
|
||
| type Temp = { | ||
| registration: Registration | null, | ||
| latestToU: string, | ||
| }; | ||
|
|
||
| // Initial state of health status in redux store | ||
| const initialState: RegistrationState = { | ||
| registration: null, | ||
| summary: null, | ||
| latestToU: "uninitialized", | ||
| isRegistering: false, | ||
| agreedToToU: false, | ||
| error: false, | ||
| }; | ||
|
|
||
| // This is the registration itself | ||
| export const fetchRegistration = createAppAsyncThunk("registration/fetchRegistration", async () => { | ||
| const res = await axios.get<Registration>("/admin-ng/adopter/registration"); | ||
| return res.data; | ||
| }); | ||
|
|
||
| // This is the summary | ||
| export const fetchSummary = createAppAsyncThunk("registration/fetchSummary", async () => { | ||
| const res = await axios.get<Summary>("/admin-ng/adopter/summary"); | ||
| return res.data; | ||
| }); | ||
|
|
||
| // This is the latest ToU ID. It's a string like APRIL_2020. | ||
| export const fetchLatestToU = createAppAsyncThunk("registration/fetchLatestToU", async () => { | ||
| const res = await axios.get<string>("/admin-ng/adopter/latestToU"); | ||
| return res.data; | ||
| }); | ||
|
|
||
| // This is whether the core can talk to register.opencast.org. | ||
| export const fetchIsUpToDate = createAppAsyncThunk("registration/isUpToDate", async () => { | ||
| const res = await axios.get<boolean>("/admin-ng/adopter/isUpToDate"); | ||
| return res.data; | ||
| }); | ||
|
|
||
| const registrationSlice = createSlice({ | ||
| name: "registration", | ||
| initialState, | ||
| reducers: { | ||
| setError(state, action: PayloadAction<{ | ||
| error: RegistrationState["error"], | ||
| }>) { | ||
| state.error = action.payload.error; | ||
| }, | ||
| }, | ||
| // These are used for thunks | ||
| extraReducers: builder => { | ||
| builder | ||
| /* .addCase(fetchRegistration.pending, state => { | ||
| state.statusHealth = "loading"; | ||
| }) */ | ||
| .addCase(fetchRegistration.fulfilled, (state, action: PayloadAction< | ||
| Registration | ||
| >) => { | ||
| state.registration = action.payload; | ||
| const updatedState = { | ||
| registration: state.registration, | ||
| latestToU: state.latestToU, | ||
| }; | ||
| state.agreedToToU = agreedLatestTerms(state, updatedState); | ||
| }) | ||
| .addCase(fetchLatestToU.fulfilled, (state, action: PayloadAction< | ||
| string | ||
| >) => { | ||
| state.latestToU = action.payload; | ||
| const updatedState = { | ||
| registration: state.registration, | ||
| latestToU: state.latestToU, | ||
| }; | ||
| state.agreedToToU = agreedLatestTerms(state, updatedState); | ||
| }) | ||
| .addCase(fetchSummary.fulfilled, (state, action: PayloadAction< | ||
| Summary | ||
| >) => { | ||
| state.summary = action.payload; | ||
| }) | ||
| .addCase(fetchIsUpToDate.fulfilled, (state, action: PayloadAction< | ||
| boolean | ||
| >) => { | ||
| // This is true if the core can talk to https://register.opencast.org/, false otherwise | ||
| state.isRegistering = action.payload; | ||
| }) | ||
| /* .addCase(fetchHealthStatus.rejected, (state, action) => { | ||
| state.error = true; | ||
| }) */; | ||
| }, | ||
| }); | ||
|
|
||
| const agreedLatestTerms = (_state: WritableDraft<RegistrationState>, updatedState: Temp) => { | ||
| if (null != updatedState.registration && "uninitialized" != updatedState.latestToU) { | ||
| return updatedState.registration.termsVersionAgreed === updatedState.latestToU; | ||
| } | ||
| return false; | ||
| }; | ||
|
|
||
| export const { setError } = registrationSlice.actions; | ||
|
|
||
| // Export the slice reducer as the default export | ||
| export default registrationSlice.reducer; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will dispatch a redux action (and thus a fetch request) every time the component rerenders and registration is still null. This could potentially cause multiple fetch requests. To avoid this, we usually use a useEffect that only runs once on component mount for fetch requests. (see the useEffect in e.g. SeriesDetails.tsx).
If you need to monitor the state of the action, you would usually add a variable to the redux state that tracks the action state (LOADING, SUCCEEDED, FAILED what have you). Then track the action state in your component via a selector and use a useEffect to respond properly.
And if you don't actually need all that redux boilerplate, you could also do the fetch requesting and state tracking here with useEffect and useState.