Skip to content

Commit

Permalink
Get rid of AuthenticatedDataContext by using relay store
Browse files Browse the repository at this point in the history
So, quick recap of what was done before this commit:
- Initial GQL query can include credentials to immediately unlock
  `authorizedData`, but lets just consider the route where it doesn't.
- `useAuthenticatedDataQuery` was called which sent a request based on
  whether the `authorizedData` was already present and on whether
  credentials did exist (by cleverly using `fetchPolicy`). In our case
  it first did nothing as no credentials are there.
- The `authenticatedData` as stored inside a React context, and not in
  `event`.
- The password form, when submitted would send a GQL request. On success
  the `setAuthData` setter from the context was called, meaning that
  all components using the contexts were rerendered.

Generally nothing terrible going on here, but: `authorizedData` is
really a part of the event and having a separate store, instead of just
saving and retrieving it from the Relay store feels weird. There was
also a bug where only one context was used per realm, which lead to
all video blocks showing the same video if one was unlocked.

This commit changes this. The context is completely removed and the
data is stored in the relay store, making this more idiomatic. But oh
boy, it was not easy to get here. Relay docs are just terrible for me
to dig through. So one thing seemed clear fairly quickly: the idiomatic
way is `useRefetchableFragment` for this. With that, one can only
refetch a small part of the initial query, but with potentially
different variables. Perfect! Problem is that the `refetch` function's
callback function does not get the fetched data. So it is hard to
check if the credentials were correct (and to distinguish other network
errors). So my solution at the end is to keep the manual `fetchQuery`
as that allows us to check the result in a callback. Then, on success,
the `refetch` is called with "store-only" to just rerender the
components. One of the big time sinks for me was to understand that the
manual query we use with `fetchQuery` needs to be exactly the same as
what `refetch` will execute. Specifically: use `node()` and not
`eventById`.

Refetching only works with fragments, so another layer of fragments
needed to be introduced. I added a helper function to combine the event
with its `authorizedData` for that.

This commit is best viewed with whitespace-diff disabled.
  • Loading branch information
LukasKalbertodt committed Nov 19, 2024
1 parent 571c25b commit 4059430
Show file tree
Hide file tree
Showing 6 changed files with 232 additions and 260 deletions.
1 change: 1 addition & 0 deletions frontend/src/i18n/locales/en.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ video:
password:
heading: Protected Video
sub-heading: Access to this video is restricted.
no-preview-permission: $t(api-remote-errors.view.event)
body: >
Please enter the username and the password you have received to access this video;
please note that these are not your login credentials.
Expand Down
32 changes: 10 additions & 22 deletions frontend/src/routes/Embed.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ReactNode, Suspense, useState } from "react";
import { ReactNode, Suspense } from "react";
import { LuFrown, LuAlertTriangle } from "react-icons/lu";
import { Translation, useTranslation } from "react-i18next";
import {
Expand All @@ -7,7 +7,7 @@ import {
} from "react-relay";
import { unreachable } from "@opencast/appkit";

import { eventId, getCredentials, isSynced, keyOfId, useAuthenticatedDataQuery } from "../util";
import { eventId, getCredentials, isSynced, keyOfId } from "../util";
import { GlobalErrorBoundary } from "../util/err";
import { loadQuery } from "../relay";
import { makeRoute, MatchedRoute } from "../rauta";
Expand All @@ -19,7 +19,7 @@ import { EmbedQuery } from "./__generated__/EmbedQuery.graphql";
import { EmbedDirectOpencastQuery } from "./__generated__/EmbedDirectOpencastQuery.graphql";
import { EmbedEventData$key } from "./__generated__/EmbedEventData.graphql";
import { PlayerContextProvider } from "../ui/player/PlayerContext";
import { AuthenticatedDataContext, AuthorizedData, PreviewPlaceholder } from "./Video";
import { PreviewPlaceholder, useEventWithAuthData } from "./Video";

export const EmbedVideoRoute = makeRoute({
url: ({ videoId }: { videoId: string }) => `/~embed/!v/${keyOfId(videoId)}`,
Expand Down Expand Up @@ -120,12 +120,8 @@ const embedEventFragment = graphql`
endTime
duration
}
authorizedData(user: $eventUser, password: $eventPassword) {
thumbnail
tracks { uri flavor mimetype resolution isMaster }
captions { uri lang }
segments { uri startTime }
}
... VideoPageAuthorizedData
@arguments(eventUser: $eventUser, eventPassword: $eventPassword)
}
}
`;
Expand All @@ -138,12 +134,12 @@ type EmbedProps = {

const Embed: React.FC<EmbedProps> = ({ query, queryRef }) => {
const fragmentRef = usePreloadedQuery(query, queryRef);
const event = useFragment<EmbedEventData$key>(
const protoEvent = useFragment<EmbedEventData$key>(
embedEventFragment,
fragmentRef.event,
);
const [event, refetch] = useEventWithAuthData(protoEvent);
const { t } = useTranslation();
const [authenticatedData, setAuthenticatedData] = useState<AuthorizedData | null>(null);

if (!event) {
return <PlayerPlaceholder>
Expand All @@ -170,17 +166,9 @@ const Embed: React.FC<EmbedProps> = ({ query, queryRef }) => {
</PlayerPlaceholder>;
}

const authorizedData = useAuthenticatedDataQuery(
event.id,
event.series?.id,
{ authorizedData: event.authorizedData },
);

return authorizedData
? <Player event={{ ...event, authorizedData }} />
: <AuthenticatedDataContext.Provider value={{ authenticatedData, setAuthenticatedData }}>
<PreviewPlaceholder embedded {...{ event }}/>;
</AuthenticatedDataContext.Provider>;
return event.authorizedData
? <Player event={{ ...event, authorizedData: event.authorizedData }} />
: <PreviewPlaceholder embedded {...{ event, refetch }}/>;
};

export const BlockEmbedRoute = makeRoute({
Expand Down
10 changes: 3 additions & 7 deletions frontend/src/routes/Realm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { COLORS } from "../color";
import { useMenu } from "../layout/MenuState";
import { ManageNav } from "./manage";
import { BREAKPOINT as NAV_BREAKPOINT } from "../layout/Navigation";
import { AuthorizedData, AuthenticatedDataContext } from "./Video";


// eslint-disable-next-line @typescript-eslint/quotes
Expand Down Expand Up @@ -146,7 +145,6 @@ const RealmPage: React.FC<Props> = ({ realm }) => {
const { t } = useTranslation();
const siteTitle = useTranslatedConfig(CONFIG.siteTitle);
const breadcrumbs = realmBreadcrumbs(t, realm.ancestors);
const [authenticatedData, setAuthenticatedData] = useState<AuthorizedData | null>(null);

const title = realm.isMainRoot ? siteTitle : realm.name;
useTitle(title, realm.isMainRoot);
Expand All @@ -168,11 +166,9 @@ const RealmPage: React.FC<Props> = ({ realm }) => {
{realm.isUserRealm && <UserRealmNote realm={realm} />}
</div>
)}
<AuthenticatedDataContext.Provider value={{ authenticatedData, setAuthenticatedData }}>
{realm.blocks.length === 0 && realm.isMainRoot
? <WelcomeMessage />
: <Blocks realm={realm} />}
</AuthenticatedDataContext.Provider>
{realm.blocks.length === 0 && realm.isMainRoot
? <WelcomeMessage />
: <Blocks realm={realm} />}
</>;
};

Expand Down
Loading

0 comments on commit 4059430

Please sign in to comment.