Skip to content

Commit

Permalink
refactor: add 'key-utils' to consolidate opaque key logic
Browse files Browse the repository at this point in the history
  • Loading branch information
bradenmacdonald committed Sep 18, 2024
1 parent 27b3d49 commit d7e185b
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 77 deletions.
3 changes: 2 additions & 1 deletion src/editors/data/redux/app/selectors.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { createSelector } from 'reselect';
import { blockTypes } from '../../constants/app';
import { isLibraryV1Key } from '../../../../generic/key-utils';
import * as urls from '../../services/cms/urls';
// This 'module' self-import hack enables mocking during tests.
// See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested
Expand Down Expand Up @@ -46,7 +47,7 @@ export const isLibrary = createSelector(
module.simpleSelectors.blockId,
],
(learningContextId, blockId) => {
if (learningContextId && learningContextId.startsWith('library-v1')) {
if (isLibraryV1Key(learningContextId)) {
return true;
}
if (blockId && blockId.startsWith('lb:')) {
Expand Down
3 changes: 2 additions & 1 deletion src/editors/data/redux/thunkActions/app.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { StrictDict, camelizeKeys } from '../../../utils';
import { isLibraryKey } from '../../../../generic/key-utils';
import * as requests from './requests';
// This 'module' self-import hack enables mocking during tests.
// See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested
Expand Down Expand Up @@ -102,7 +103,7 @@ export const initialize = (data) => (dispatch) => {
dispatch(module.fetchCourseDetails());
break;
case 'html':
if (data.learningContextId?.startsWith('lib:')) {
if (isLibraryKey(data.learningContextId)) {
// eslint-disable-next-line no-console
console.log('Not fetching image assets - not implemented yet for content libraries.');
} else {
Expand Down
3 changes: 2 additions & 1 deletion src/editors/data/redux/thunkActions/problem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { actions as problemActions } from '../problem';
import { actions as requestActions } from '../requests';
import { selectors as appSelectors } from '../app';
import * as requests from './requests';
import { isLibraryKey } from '../../../../generic/key-utils';
import { OLXParser } from '../../../containers/ProblemEditor/data/OLXParser';
import { parseSettings } from '../../../containers/ProblemEditor/data/SettingsParser';
import { ProblemTypeKeys } from '../../constants/problem';
Expand Down Expand Up @@ -86,7 +87,7 @@ export const initializeProblem = (blockValue) => (dispatch, getState) => {
const rawOLX = _.get(blockValue, 'data.data', {});
const rawSettings = _.get(blockValue, 'data.metadata', {});
const learningContextId = selectors.app.learningContextId(getState());
if (learningContextId?.startsWith('lib:')) {
if (isLibraryKey(learningContextId)) {
// Content libraries don't yet support defaults for fields like max_attempts, showanswer, etc.
// So proceed with loading the problem.
// Though first we need to fake the request or else the problem type selection UI won't display:
Expand Down
3 changes: 2 additions & 1 deletion src/editors/data/services/cms/api.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { AxiosRequestConfig } from 'axios';
import { camelizeKeys } from '../../../utils';
import { isLibraryKey } from '../../../../generic/key-utils';
import * as urls from './urls';
import { get, post, deleteObject } from './utils';
import { durationStringFromValue } from '../../../containers/VideoEditor/components/VideoSettingsModal/components/DurationWidget/hooks';
Expand Down Expand Up @@ -111,7 +112,7 @@ export const apiMethods = {
studioEndpointUrl,
pageNumber,
}): Promise<{ data: AssetResponse & Pagination }> => {
if (learningContextId.startsWith('lib:')) {
if (isLibraryKey(learningContextId)) {
// V2 content libraries don't support static assets yet:
return Promise.resolve({
data: {
Expand Down
8 changes: 6 additions & 2 deletions src/editors/data/services/cms/urls.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { isLibraryKey, isLibraryV1Key } from '../../../../generic/key-utils';

/**
* A little helper so we can write the types of these functions more compactly
* The main purpose of this is to indicate the params are all strings.
Expand All @@ -15,11 +17,13 @@ export const unit = ({ studioEndpointUrl, unitUrl, blockId }) => (
export const returnUrl = ({
studioEndpointUrl, unitUrl, learningContextId, blockId,
}): string => {
if (learningContextId && learningContextId.startsWith('library-v1')) {
// Is this a v1 library?
if (isLibraryV1Key(learningContextId)) {
// when the learning context is a v1 library, return to the library page
return libraryV1({ studioEndpointUrl, learningContextId });
}
if (learningContextId && learningContextId.startsWith('lib:')) {
// Is this a v2 library?
if (isLibraryKey(learningContextId)) {
// when it's a v2 library, there will be no return url (instead a closed popup)
// (temporary) don't throw error, just return empty url. it will fail it's network connection but otherwise
// the app will run
Expand Down
4 changes: 2 additions & 2 deletions src/editors/sharedComponents/ErrorBoundary/ErrorPage.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import React from 'react';
import { connect } from 'react-redux';
import PropTypes from 'prop-types';
import {
Expand All @@ -7,6 +6,7 @@ import {

import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import messages from './messages';
import { isLibraryV1Key } from '../../../generic/key-utils';
import { navigateTo } from '../../hooks';
import { selectors } from '../../data/redux';

Expand All @@ -23,7 +23,7 @@ const ErrorPage = ({
// injected
intl,
}) => {
const outlineType = learningContextId?.startsWith('library-v1') ? 'library' : 'course';
const outlineType = isLibraryV1Key(learningContextId) ? 'library' : 'course';
const outlineUrl = `${studioEndpointUrl}/${outlineType}/${learningContextId}`;
const unitUrl = unitData?.data ? `${studioEndpointUrl}/container/${unitData?.data.ancestors[0].id}` : null;

Expand Down
78 changes: 78 additions & 0 deletions src/generic/key-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import {
getBlockType,
getLibraryId,
isLibraryKey,
isLibraryV1Key,
} from './key-utils';

describe('component utils', () => {
describe('getBlockType', () => {
for (const [input, expected] of [
['lb:org:lib:html:id', 'html'],
['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'html'],
['lb:Axim:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'problem'],
]) {
it(`returns '${expected}' for usage key '${input}'`, () => {
expect(getBlockType(input)).toStrictEqual(expected);
});
}

for (const input of ['', undefined, null, 'not a key', 'lb:foo']) {
it(`throws an exception for usage key '${input}'`, () => {
expect(() => getBlockType(input as any)).toThrow(`Invalid usageKey: ${input}`);
});
}
});

describe('getLibraryId', () => {
for (const [input, expected] of [
['lb:org:lib:html:id', 'lib:org:lib'],
['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'lib:OpenCraftX:ALPHA'],
['lb:Axim:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'lib:Axim:beta'],
]) {
it(`returns '${expected}' for usage key '${input}'`, () => {
expect(getLibraryId(input)).toStrictEqual(expected);
});
}

for (const input of ['', undefined, null, 'not a key', 'lb:foo']) {
it(`throws an exception for usage key '${input}'`, () => {
expect(() => getLibraryId(input as any)).toThrow(`Invalid usageKey: ${input}`);
});
}
});

describe('isLibraryKey', () => {
for (const [input, expected] of [
['lib:org:lib', true],
['lib:OpenCraftX:ALPHA', true],
['lb:org:lib:html:id', false],
['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', false],
['library-v1:AximX+L1', false],
['course-v1:AximX+TS100+23', false],
['', false],
[undefined, false],
] as const) {
it(`returns '${expected}' for learning context key '${input}'`, () => {
expect(isLibraryKey(input)).toStrictEqual(expected);
});
}
});

describe('isLibraryV1Key', () => {
for (const [input, expected] of [
['library-v1:AximX+L1', true],
['lib:org:lib', false],
['lib:OpenCraftX:ALPHA', false],
['lb:org:lib:html:id', false],
['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', false],
['course-v1:AximX+TS100+23', false],
['', false],
[undefined, false],
] as const) {
it(`returns '${expected}' for learning context key '${input}'`, () => {
expect(isLibraryV1Key(input)).toStrictEqual(expected);
});
}
});
});
40 changes: 40 additions & 0 deletions src/generic/key-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* Given a usage key like `lb:org:lib:html:id`, get the type (e.g. `html`)
* @param usageKey e.g. `lb:org:lib:html:id`
* @returns The block type as a string
*/
export function getBlockType(usageKey: string): string {
if (usageKey && usageKey.startsWith('lb:')) {
const blockType = usageKey.split(':')[3];
if (blockType) {
return blockType;
}
}
throw new Error(`Invalid usageKey: ${usageKey}`);
}

/**
* Given a usage key like `lb:org:lib:html:id`, get the library key
* @param usageKey e.g. `lb:org:lib:html:id`
* @returns The library key, e.g. `lib:org:lib`
*/
export function getLibraryId(usageKey: string): string {
if (usageKey && usageKey.startsWith('lb:')) {
const org = usageKey.split(':')[1];
const lib = usageKey.split(':')[2];
if (org && lib) {
return `lib:${org}:${lib}`;
}
}
throw new Error(`Invalid usageKey: ${usageKey}`);
}

/** Check if this is a V2 library key. */
export function isLibraryKey(learningContextKey: string | undefined): learningContextKey is string {
return typeof learningContextKey === 'string' && learningContextKey.startsWith('lib:');
}

/** Check if this is a V1 library key. */
export function isLibraryV1Key(learningContextKey: string | undefined): learningContextKey is string {
return typeof learningContextKey === 'string' && learningContextKey.startsWith('library-v1:');
}
38 changes: 1 addition & 37 deletions src/library-authoring/components/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,42 +1,6 @@
import { getBlockType, getEditUrl, getLibraryId } from './utils';
import { getEditUrl } from './utils';

describe('component utils', () => {
describe('getBlockType', () => {
for (const [input, expected] of [
['lb:org:lib:html:id', 'html'],
['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'html'],
['lb:Axim:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'problem'],
]) {
it(`returns '${expected}' for usage key '${input}'`, () => {
expect(getBlockType(input)).toStrictEqual(expected);
});
}

for (const input of ['', undefined, null, 'not a key', 'lb:foo']) {
it(`throws an exception for usage key '${input}'`, () => {
expect(() => getBlockType(input as any)).toThrow(`Invalid usageKey: ${input}`);
});
}
});

describe('getLibraryId', () => {
for (const [input, expected] of [
['lb:org:lib:html:id', 'lib:org:lib'],
['lb:OpenCraftX:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'lib:OpenCraftX:ALPHA'],
['lb:Axim:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd', 'lib:Axim:beta'],
]) {
it(`returns '${expected}' for usage key '${input}'`, () => {
expect(getLibraryId(input)).toStrictEqual(expected);
});
}

for (const input of ['', undefined, null, 'not a key', 'lb:foo']) {
it(`throws an exception for usage key '${input}'`, () => {
expect(() => getLibraryId(input as any)).toThrow(`Invalid usageKey: ${input}`);
});
}
});

describe('getEditUrl', () => {
it('returns the right URL for an HTML (Text) block', () => {
const usageKey = 'lb:org:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd';
Expand Down
32 changes: 2 additions & 30 deletions src/library-authoring/components/utils.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,6 @@
/**
* Given a usage key like `lb:org:lib:html:id`, get the type (e.g. `html`)
* @param usageKey e.g. `lb:org:lib:html:id`
* @returns The block type as a string
*/
export function getBlockType(usageKey: string): string {
if (usageKey && usageKey.startsWith('lb:')) {
const blockType = usageKey.split(':')[3];
if (blockType) {
return blockType;
}
}
throw new Error(`Invalid usageKey: ${usageKey}`);
}

/**
* Given a usage key like `lb:org:lib:html:id`, get the library key
* @param usageKey e.g. `lb:org:lib:html:id`
* @returns The library key, e.g. `lib:org:lib`
*/
export function getLibraryId(usageKey: string): string {
if (usageKey && usageKey.startsWith('lb:')) {
const org = usageKey.split(':')[1];
const lib = usageKey.split(':')[2];
if (org && lib) {
return `lib:${org}:${lib}`;
}
}
throw new Error(`Invalid usageKey: ${usageKey}`);
}
import { getBlockType, getLibraryId } from '../../generic/key-utils';

/* eslint-disable import/prefer-default-export */
export function getEditUrl(usageKey: string): string | undefined {
let blockType: string;
let libraryId: string;
Expand Down
2 changes: 1 addition & 1 deletion src/library-authoring/data/apiHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
type QueryClient,
} from '@tanstack/react-query';

import { getLibraryId } from '../components/utils';
import { getLibraryId } from '../../generic/key-utils';
import {
type GetLibrariesV2CustomParams,
type ContentLibrary,
Expand Down
3 changes: 2 additions & 1 deletion src/search-modal/SearchResult.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { useSelector } from 'react-redux';
import { useNavigate } from 'react-router-dom';

import { getItemIcon } from '../generic/block-type-utils';
import { isLibraryKey } from '../generic/key-utils';
import { useSearchContext, type ContentHit, Highlight } from '../search-manager';
import { getStudioHomeData } from '../studio-home/data/selectors';
import { constructLibraryAuthoringURL } from '../utils';
Expand Down Expand Up @@ -116,7 +117,7 @@ const SearchResult: React.FC<{ hit: ContentHit }> = ({ hit }) => {
return `/${urlSuffix}`;
}

if (contextKey.startsWith('lib:')) {
if (isLibraryKey(contextKey)) {
const urlSuffix = getLibraryComponentUrlSuffix(hit);
if (redirectToLibraryAuthoringMfe && libraryAuthoringMfeUrl) {
return constructLibraryAuthoringURL(libraryAuthoringMfeUrl, urlSuffix);
Expand Down

0 comments on commit d7e185b

Please sign in to comment.