-
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 all commits
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 |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import languages from "../i18n/languages"; | |
| import opencastLogo from "../img/opencast-white.svg?url"; | ||
| import { setSpecificServiceFilter } from "../slices/tableFilterSlice"; | ||
| import { getErrorCount, getHealthStatus } from "../selectors/healthSelectors"; | ||
| import { getRegistration, getIsRegistering, getAgreedLatestToU } from "../selectors/registrationSelectors"; | ||
| import { | ||
| getOrgProperties, | ||
| getUserInformation, | ||
|
|
@@ -19,6 +20,11 @@ import HotKeyCheatSheet from "./shared/HotKeyCheatSheet"; | |
| import { useHotkeys } from "react-hotkeys-hook"; | ||
| import { useAppDispatch, useAppSelector } from "../store"; | ||
| import { HealthStatus, fetchHealthStatus } from "../slices/healthSlice"; | ||
| import { | ||
| fetchRegistration, | ||
| fetchLatestToU, | ||
| fetchIsUpToDate, | ||
| } from "../slices/registrationSlice"; | ||
| import { UserInfoState } from "../slices/userInfoSlice"; | ||
| import { Tooltip } from "./shared/Tooltip"; | ||
| import { HiOutlineTranslate } from "react-icons/hi"; | ||
|
|
@@ -51,13 +57,29 @@ const Header = () => { | |
| const healthStatus = useAppSelector(state => getHealthStatus(state)); | ||
| const errorCounter = useAppSelector(state => getErrorCount(state)); | ||
| const user = useAppSelector(state => getUserInformation(state)); | ||
| const registration = useAppSelector(state => getRegistration(state)); | ||
| const _isRegistering = useAppSelector(state => getIsRegistering(state)); | ||
| const _agreedLatestToU = useAppSelector(state => getAgreedLatestToU(state)); | ||
| const orgProperties = useAppSelector(state => getOrgProperties(state)); | ||
| const displayTerms = (orgProperties["org.opencastproject.admin.display_terms"] || "false").toLowerCase() === "true"; | ||
|
|
||
| const loadHealthStatus = async () => { | ||
| await dispatch(fetchHealthStatus()); | ||
| }; | ||
|
|
||
| if (registration == null) { | ||
| dispatch(fetchRegistration()); | ||
| } | ||
| // dispatch(fetchLatestToU()); | ||
| // dispatch(fetchIsUpToDate()); | ||
|
|
||
| const _getLatestToU = async () => { | ||
| await dispatch(fetchLatestToU()); | ||
| }; | ||
| const _getIsUpToDate = async () => { | ||
| await dispatch(fetchIsUpToDate()); | ||
| }; | ||
|
Comment on lines
+73
to
+81
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. This is going unused and should be removed. |
||
|
|
||
| const hideMenuHelp = () => { | ||
| setMenuHelp(false); | ||
| }; | ||
|
|
@@ -134,18 +156,18 @@ const Header = () => { | |
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| if (!user) { return; } | ||
|
|
||
| const isAdmin = user.isAdmin || user.isOrgAdmin; | ||
| const isLocalhost = window.location.hostname === "localhost"; | ||
| const lastDismissed = localStorage.getItem("adopterModalDismissed"); | ||
| const THIRTY_DAYS = 30 * 24 * 60 * 60 * 1000; | ||
| const dismissedLongEnough = !lastDismissed || Date.now() - parseInt(lastDismissed) > THIRTY_DAYS; | ||
|
|
||
| if (isAdmin && !isLocalhost && dismissedLongEnough) { | ||
| showRegistrationModal(); | ||
| } | ||
| }, [user]); | ||
| if (!user) { return; } | ||
|
|
||
| const isAdmin = user.isAdmin || user.isOrgAdmin; | ||
| const isLocalhost = window.location.hostname === "localhost"; | ||
| const lastDismissed = localStorage.getItem("adopterModalDismissed"); | ||
| const THIRTY_DAYS = 30 * 24 * 60 * 60 * 1000; | ||
| const dismissedLongEnough = !lastDismissed || Date.now() - parseInt(lastDismissed) > THIRTY_DAYS; | ||
|
|
||
| if (isAdmin && !isLocalhost && dismissedLongEnough && registration == null) { | ||
| showRegistrationModal(); | ||
| } | ||
| }, [user, registration]); | ||
| return ( | ||
| <> | ||
| <header className="primary-header"> | ||
|
|
||
| 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? |
||
| 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.