Skip to content

Commit

Permalink
(HP-2023) Move some logic of next phase resolving to actions
Browse files Browse the repository at this point in the history 
Actions should know are they resumable, not the hook using them.

Added new option: "resumable". Easier to check can queue be resumed from that action.

Also added action.data.requiredPath which is compared to current path.

Action is only resumable on that path.

RedirectionCatcher also has data.isRedirectionCatcher.
Its type cannot be compared because it can be dynamic.

Cleaned up code too
  • Loading branch information
@NikoHelle
NikoHelle committed Oct 16, 2023
1 parent 9a06612 commit 7e61f15
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 128 deletions.
5 changes: 5 additions & 0 deletions src/common/actionQueue/actionQueue.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export type ActionOptions = {
idleWhenActive?: boolean;
noStorage?: boolean;
syncronousCompletion?: boolean;
resumable?: boolean;
data?: JSONStringifyableResult;
};

Expand Down Expand Up @@ -147,6 +148,10 @@ export function getData(
: data;
}

export function isResumable(action: Action): boolean {
return !!getOption(action, 'resumable');
}

export function verifyQueuesMatch(
primaryQueue: ActionSource[],
secondaryQueue: ActionSource[]
Expand Down
65 changes: 22 additions & 43 deletions src/gdprApi/__tests__/useAuthCodeQueues.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import {
} from '../actions/__mocks__/mock.util';
import {
getScenarioForScopes,
getScenarioForTunnistamoAuth,
getScenarioWhereEveryActionCanBeManuallyCompletetedSuccessfully,
getScenarioWhereKeycloakAuthCodeNotInUrl,
getScenarioWhereNextPhaseIsResumeCallback,
Expand All @@ -68,6 +67,7 @@ import {
import { downloadAsFileAction } from '../actions/downloadAsFile';
import { actionLogTypes } from '../../common/actionQueue/actionQueueRunner';
import { getQueue } from '../actions/queues';
import { getMockCallArgs } from '../../common/test/jestMockHelper';

type HookFunctionResults = {
hasError: boolean;
Expand Down Expand Up @@ -448,7 +448,7 @@ describe('useAuthCodeQueues', () => {
beforeEach(() => {
mockedWindowControls.setPath(config.gdprCallbackPath);
});
it('When codes are fetched, next phase is "resume-callback". User is redirected to the download page', async () => {
it('When codes are fetched, next action will redirect back to start page.', async () => {
mockedWindowControls.setSearch(
`state=${tunnistamoState}&code=${tunnistamoCode}`
);
Expand All @@ -474,17 +474,19 @@ describe('useAuthCodeQueues', () => {
expect(isActionCompleted(loadKeycloakConfigAction.type)).toBeTruthy();
});
await waitFor(() => {
expect(mockHistoryPushTracker).toHaveBeenCalledTimes(1);
expect(mockHistoryPushTracker).toHaveBeenLastCalledWith(
'/?next=redirectionCatcher'
);
expect(getState()).toMatchObject({
nextPhase: nextPhases.redirectBackToStartPage,
nextPhase: nextPhases.waitForInternalRedirect,
});
});
expect(getFunctionResults()).toMatchObject({
...hookFunctionResultsAsFalse,
isLoading: true,
});
});
it('When an action fails, an error is logged and redirection to download page is done', async () => {
it('When an action fails, an error is logged and redirection to start page is done', async () => {
mockedWindowControls.setSearch(`state=${keycloakState}&code=`);
initQueue(getScenarioWhereKeycloakAuthCodeNotInUrl());
const { resume, getState, getFunctionResults } = renderTestComponent();
Expand Down Expand Up @@ -520,46 +522,20 @@ describe('useAuthCodeQueues', () => {
});
await waitFor(async () => {
expect(mockHistoryPushTracker).toHaveBeenCalledTimes(1);
});
const lastCall = getMockCallArgs(
mockHistoryPushTracker,
0
)[0] as string;

await waitFor(async () => {
expect(getState()).toMatchObject({
currentPhase: currentPhases.error,
nextPhase: nextPhases.redirectBackToStartPage,
});
});
expect(getFunctionResults()).toMatchObject({
...hookFunctionResultsAsFalse,
hasError: true,
expect(
lastCall.includes(`/?error=${keycloakAuthCodeParserAction.type}`)
).toBeTruthy();
});
});
it('Download page redirection is always done when gdpr-callback page has an error', async () => {
initQueue([
...getScenarioForScopes({ store: true }),
...getScenarioForTunnistamoAuth({
store: true,
overrides: [
{
type: tunnistamoAuthCodeCallbackUrlAction.type,
rejectValue: rejectionError,
resolveValue: undefined,
store: false,
autoTrigger: true,
},
{
type: tunnistamoAuthCodeParserAction.type,
store: false,
},
],
}),
]);
const { resume, getState, getFunctionResults } = renderTestComponent();
resume();

await waitFor(async () => {
expect(mockHistoryPushTracker).toHaveBeenCalledTimes(1);
expect(getState()).toMatchObject({
currentPhase: currentPhases.error,
nextPhase: nextPhases.redirectBackToStartPage,
nextPhase: nextPhases.waitForInternalRedirect,
});
});
expect(getFunctionResults()).toMatchObject({
Expand Down Expand Up @@ -871,19 +847,22 @@ describe('useAuthCodeQueues', () => {

expect(getState()).toMatchObject({
currentPhase: currentPhases.running,
nextPhase: nextPhases.waitForAction,
nextPhase: nextPhases.waitForInternalRedirect,
});

await checkCurrentActionAndManuallyCompleteIt(
defaultRedirectorActionType,
defaultRedirectionCatcherActionType
);

expect(getState()).toMatchObject({
currentPhase: currentPhases.running,
nextPhase: nextPhases.redirectBackToStartPage,
nextPhase: nextPhases.waitForInternalRedirect,
});

expect(mockHistoryPushTracker).toHaveBeenLastCalledWith(
'/?next=redirectionCatcher'
);

expect(getFunctionResults()).toMatchObject({
...hookFunctionResultsAsFalse,
isLoading: true,
Expand Down
2 changes: 1 addition & 1 deletion src/gdprApi/actions/__mocks__/queueScenarios.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ export function getScenarioWhereEveryActionCanBeManuallyCompletetedSuccessfully(
overrides: [
{
type: defaultRedirectorActionType,
runOriginal: false,
runOriginal: true,
resolveValue: true,
},
{
Expand Down
34 changes: 22 additions & 12 deletions src/gdprApi/actions/authCodeCallbackUrlDetector.ts
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import {
Action,
ActionExecutor,
ActionOptions,
ActionProps,
QueueController,
isResumable,
} from '../../common/actionQueue/actionQueue';
import { RunnerFunctions } from '../../common/actionQueue/actionQueueRunner';
import config from '../../config';
import { getAuthCodeRedirectionInitializationResult } from './authCodeRedirectionInitialization';
import {
isAuthCodeActionNeeded,
isGdprCallbackUrl,
isOnActionRequiredPath,
parseAuthorizationCallbackUrl,
rejectExecutorWithStartPageRedirection,
thirtySecondsInMs,
Expand Down Expand Up @@ -50,17 +54,6 @@ export const isQueueWaitingForAuthCodeCallback = (
controller: QueueController
) => !!getNextAuthCodeCallbackDetector(controller);

export const shouldResumeWithAuthCodeCallback = (
runner: RunnerFunctions
): boolean => {
const action = getNextAuthCodeCallbackDetector(runner);
if (!action) {
return false;
}
const status = runner.getActionStatus(action);
return status === 'next' || status === 'pending';
};

export const resumeQueueFromNextCallbackDetector = (
runner: RunnerFunctions
) => {
Expand All @@ -74,6 +67,19 @@ export const resumeQueueFromNextCallbackDetector = (
return undefined;
};

export const isResumableGdprCallback = (action: Action) => {
if (
action.type !== tunnistamoAuthCodeCallbackUrlDetectorType &&
action.type !== keycloakAuthCodeCallbackUrlDetectorType
) {
return false;
}
if (!isResumable(action)) {
return false;
}
return isOnActionRequiredPath(action);
};

const authCodeCallbackUrlDetectorExecutor: ActionExecutor = async (
action,
controller
Expand All @@ -96,7 +102,6 @@ const authCodeCallbackUrlDetectorExecutor: ActionExecutor = async (
);
return state && storedUrlProps && storedUrlProps.state !== state;
};

if (isWrongPathOrState()) {
return rejectExecutorWithStartPageRedirection(
controller,
Expand All @@ -111,6 +116,11 @@ const authCodeCallbackUrlDetectorExecutor: ActionExecutor = async (
const options: ActionOptions = {
noStorage: true,
idleWhenActive: true,
resumable: true,
data: {
requiredPath: config.gdprCallbackPath,
redirectsOnError: true,
},
};

export const tunnistamoAuthCodeCallbackUrlAction: ActionProps = {
Expand Down
13 changes: 6 additions & 7 deletions src/gdprApi/actions/authCodeParser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,23 @@ const authCodeParserExecutor: ActionExecutor = async (action, controller) => {
);
}

const rejector = (message: string) =>
rejectExecutorWithStartPageRedirection(controller, action, message);

const { code, state } = parseAuthorizationCallbackUrl();
if (!code || !state) {
return Promise.reject('No code or state found in callback url');
return rejector('No code or state found in callback url');
}

const storedUrlProps = getAuthCodeRedirectionInitializationResult(
action,
controller
);
if (!storedUrlProps) {
return Promise.reject('Stored state not found');
return rejector('Stored state not found');
}
if (!state || !storedUrlProps.state || storedUrlProps.state !== state) {
return rejectExecutorWithStartPageRedirection(
controller,
action,
'State is not for this action'
);
return rejector('State is not for this action');
}
return Promise.resolve(code);
};
Expand Down
27 changes: 22 additions & 5 deletions src/gdprApi/actions/redirectionHandlers.ts
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import {
Action,
ActionExecutor,
ActionProps,
ActionType,
QueueController,
getData,
isResumable,
} from '../../common/actionQueue/actionQueue';
import {
RunnerFunctions,
Expand All @@ -13,6 +15,7 @@ import {
import matchUrls from '../../common/helpers/matchUrls';
import {
createNextActionParams,
isOnActionRequiredPath,
rejectExecutorWithRedirection,
resolveExecutorWithRedirection,
thirtySecondsInMs,
Expand All @@ -21,16 +24,24 @@ import {
export const defaultRedirectorActionType = 'redirector';
export const defaultRedirectionCatcherActionType = 'redirectionCatcher';

export const canResumeWithRedirectionCatcher = (
controller: RunnerFunctions,
catcherActionType = defaultRedirectionCatcherActionType
): boolean => canQueueContinueFrom(controller, catcherActionType, true);

export const resumeQueueFromRedirectionCatcher = (
runner: RunnerFunctions,
catcherActionType = defaultRedirectionCatcherActionType
) => resumeQueueFromAction(runner, catcherActionType);

export const isResumableRedirectionCatcher = (action: Action) => {
if (
!isResumable(action) ||
getData(action, 'isRedirectionCatcher') !== true
) {
return false;
}
return isOnActionRequiredPath(action);
};

export const isWaitingForRedirectionCatcher = (action: Action) =>
action.active && !isOnActionRequiredPath(action);

export const getStartPagePathFromQueue = (
controller: QueueController,
redirectorActionType = defaultRedirectorActionType
Expand Down Expand Up @@ -81,6 +92,11 @@ export const createRedirectorAndCatcherActionProps = (
options: {
noStorage: true,
idleWhenActive: true,
resumable: true,
data: {
isRedirectionCatcher: true,
requiredPath: targetPath,
},
},
};

Expand All @@ -91,6 +107,7 @@ export const createRedirectorAndCatcherActionProps = (
noStorage: true,
data: {
startPagePath: targetPath,
redirectsInternally: true,
},
},
};
Expand Down
9 changes: 9 additions & 0 deletions src/gdprApi/actions/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
ActionType,
JSONStringifyableResult,
QueueController,
getData,
} from '../../common/actionQueue/actionQueue';
import config from '../../config';
import {
Expand Down Expand Up @@ -269,6 +270,14 @@ export function resolveExecutorWithRedirection(
return Promise.resolve(result);
}

export function isOnActionRequiredPath(action: Action): boolean {
const requiredPath = getData(action, 'requiredPath') as string;
if (!requiredPath) {
return true;
}
return matchUrls(requiredPath);
}

// There cannot be more than one redirection active - in the last completed action.
// This hook allows one redirection per initialization.
// The hook is cleared when a component using it is unmounted, so it is reset on each mount.
Expand Down
Loading

0 comments on commit 7e61f15

Please sign in to comment.