Skip to content
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

feat: add functionality to see unit draft preview #1501

Merged
merged 4 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const DECODE_ROUTES = {
'/course/:courseId/:sequenceId/:unitId',
'/course/:courseId/:sequenceId',
'/course/:courseId',
'/preview/course/:courseId/:sequenceId/:unitId',
],
REDIRECT_HOME: 'home/:courseId',
REDIRECT_SURVEY: 'survey/:courseId',
Expand Down
166 changes: 116 additions & 50 deletions src/courseware/CoursewareContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
import withParamsAndNavigation from './utils';

// Look at where this is called in componentDidUpdate for more info about its usage
const checkResumeRedirect = memoize((courseStatus, courseId, sequenceId, firstSequenceId, navigate) => {
const checkResumeRedirect = memoize((courseStatus, courseId, sequenceId, firstSequenceId, navigate, isPreview) => {
if (courseStatus === 'loaded' && !sequenceId) {
// Note that getResumeBlock is just an API call, not a redux thunk.
getResumeBlock(courseId).then((data) => {
// This is a replace because we don't want this change saved in the browser's history.
if (data.sectionId && data.unitId) {
navigate(`/course/${courseId}/${data.sectionId}/${data.unitId}`, { replace: true });
const baseUrl = `/course/${courseId}/${data.sectionId}`;
const sequenceUrl = isPreview ? `/preview${baseUrl}` : baseUrl;
navigate(`${sequenceUrl}/${data.unitId}`, { replace: true });
} else if (firstSequenceId) {
navigate(`/course/${courseId}/${firstSequenceId}`, { replace: true });
}
Expand All @@ -34,9 +36,19 @@
});

// Look at where this is called in componentDidUpdate for more info about its usage
const checkSectionUnitToUnitRedirect = memoize((courseStatus, courseId, sequenceStatus, section, unitId, navigate) => {
const checkSectionUnitToUnitRedirect = memoize((
courseStatus,
courseId,
sequenceStatus,
section,
unitId,
navigate,
isPreview,
) => {
if (courseStatus === 'loaded' && sequenceStatus === 'failed' && section && unitId) {
navigate(`/course/${courseId}/${unitId}`, { replace: true });
const baseUrl = `/course/${courseId}`;

Check warning on line 49 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L49

Added line #L49 was not covered by tests
const courseUrl = isPreview ? `/preview${baseUrl}` : baseUrl;
navigate(`${courseUrl}/${unitId}`, { replace: true });

Check warning on line 51 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L51

Added line #L51 was not covered by tests
}
});

Expand All @@ -54,68 +66,84 @@
});

// Look at where this is called in componentDidUpdate for more info about its usage
const checkUnitToSequenceUnitRedirect = memoize(
(courseStatus, courseId, sequenceStatus, sequenceMightBeUnit, sequenceId, section, routeUnitId, navigate) => {
if (courseStatus === 'loaded' && sequenceStatus === 'failed' && !section && !routeUnitId) {
if (sequenceMightBeUnit) {
// If the sequence failed to load as a sequence, but it is marked as a possible unit, then
// we need to look up the correct parent sequence for it, and redirect there.
const unitId = sequenceId; // just for clarity during the rest of this method
getSequenceForUnitDeprecated(courseId, unitId).then(
parentId => {
if (parentId) {
navigate(`/course/${courseId}/${parentId}/${unitId}`, { replace: true });
} else {
navigate(`/course/${courseId}`, { replace: true });
}
},
() => { // error case
const checkUnitToSequenceUnitRedirect = memoize((
courseStatus,
courseId,
sequenceStatus,
sequenceMightBeUnit,
sequenceId,
section,
routeUnitId,
navigate,
isPreview,
) => {
if (courseStatus === 'loaded' && sequenceStatus === 'failed' && !section && !routeUnitId) {
if (sequenceMightBeUnit) {
// If the sequence failed to load as a sequence, but it is marked as a possible unit, then
// we need to look up the correct parent sequence for it, and redirect there.
const unitId = sequenceId; // just for clarity during the rest of this method
getSequenceForUnitDeprecated(courseId, unitId).then(
parentId => {

Check warning on line 86 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L84-L86

Added lines #L84 - L86 were not covered by tests
if (parentId) {
const baseUrl = `/course/${courseId}/${parentId}`;

Check warning on line 88 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L88

Added line #L88 was not covered by tests
const sequenceUrl = isPreview ? `/preview${baseUrl}` : baseUrl;
navigate(`${sequenceUrl}/${unitId}`, { replace: true });
} else {

Check warning on line 91 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L90-L91

Added lines #L90 - L91 were not covered by tests
navigate(`/course/${courseId}`, { replace: true });
},
);
} else {
// Invalid sequence that isn't a unit either. Redirect up to main course.
navigate(`/course/${courseId}`, { replace: true });
}
}
},
() => { // error case
navigate(`/course/${courseId}`, { replace: true });

Check warning on line 96 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L95-L96

Added lines #L95 - L96 were not covered by tests
},
);
} else {

Check warning on line 99 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L99

Added line #L99 was not covered by tests
// Invalid sequence that isn't a unit either. Redirect up to main course.
navigate(`/course/${courseId}`, { replace: true });

Check warning on line 101 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L101

Added line #L101 was not covered by tests
}
},
);
}
});

// Look at where this is called in componentDidUpdate for more info about its usage
const checkSequenceToSequenceUnitRedirect = memoize((courseId, sequenceStatus, sequence, unitId, navigate) => {
if (sequenceStatus === 'loaded' && sequence.id && !unitId) {
if (sequence.unitIds !== undefined && sequence.unitIds.length > 0) {
const nextUnitId = sequence.unitIds[sequence.activeUnitIndex];
// This is a replace because we don't want this change saved in the browser's history.
navigate(`/course/${courseId}/${sequence.id}/${nextUnitId}`, { replace: true });
const checkSequenceToSequenceUnitRedirect = memoize(
(courseId, sequenceStatus, sequence, unitId, navigate, isPreview) => {
if (sequenceStatus === 'loaded' && sequence.id && !unitId) {
if (sequence.unitIds !== undefined && sequence.unitIds.length > 0) {
const baseUrl = `/course/${courseId}/${sequence.id}`;
const sequenceUrl = isPreview ? `/preview${baseUrl}` : baseUrl;
const nextUnitId = sequence.unitIds[sequence.activeUnitIndex];
// This is a replace because we don't want this change saved in the browser's history.
navigate(`${sequenceUrl}/${nextUnitId}`, { replace: true });
}
}
}
});
},
);

// Look at where this is called in componentDidUpdate for more info about its usage
const checkSequenceUnitMarkerToSequenceUnitRedirect = memoize(
(courseId, sequenceStatus, sequence, unitId, navigate) => {
(courseId, sequenceStatus, sequence, unitId, navigate, isPreview) => {
if (sequenceStatus !== 'loaded' || !sequence.id) {
return;
}

const baseUrl = `/course/${courseId}/${sequence.id}`;
const sequenceUrl = isPreview ? `/preview${baseUrl}` : baseUrl;
const hasUnits = sequence.unitIds?.length > 0;

if (unitId === 'first') {
if (hasUnits) {
const firstUnitId = sequence.unitIds[0];
navigate(`/course/${courseId}/${sequence.id}/${firstUnitId}`, { replace: true });
navigate(`${sequenceUrl}/${firstUnitId}`, { replace: true });
} else {
// No units... go to general sequence page
navigate(`/course/${courseId}/${sequence.id}`, { replace: true });
navigate(baseUrl, { replace: true });

Check warning on line 138 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L138

Added line #L138 was not covered by tests
}
} else if (unitId === 'last') {
if (hasUnits) {
const lastUnitId = sequence.unitIds[sequence.unitIds.length - 1];
navigate(`/course/${courseId}/${sequence.id}/${lastUnitId}`, { replace: true });
navigate(`${sequenceUrl}/${lastUnitId}`, { replace: true });
} else {
// No units... go to general sequence page
navigate(`/course/${courseId}/${sequence.id}`, { replace: true });
navigate(baseUrl, { replace: true });

Check warning on line 146 in src/courseware/CoursewareContainer.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/CoursewareContainer.jsx#L146

Added line #L146 was not covered by tests
}
}
},
Expand Down Expand Up @@ -169,6 +197,7 @@
routeSequenceId,
routeUnitId,
navigate,
isPreview,
} = this.props;

// Load data whenever the course or sequence ID changes.
Expand Down Expand Up @@ -197,7 +226,7 @@
// Check resume redirect:
// /course/:courseId -> /course/:courseId/:sequenceId/:unitId
// based on sequence/unit where user was last active.
checkResumeRedirect(courseStatus, courseId, sequenceId, firstSequenceId, navigate);
checkResumeRedirect(courseStatus, courseId, sequenceId, firstSequenceId, navigate, isPreview);

// Check section-unit to unit redirect:
// /course/:courseId/:sectionId/:unitId -> /course/:courseId/:unitId
Expand All @@ -210,33 +239,69 @@
// otherwise, we could get stuck in a redirect loop, since a sequence that failed to load
// would endlessly redirect to itself through `checkSectionUnitToUnitRedirect`
// and `checkUnitToSequenceUnitRedirect`.
checkSectionUnitToUnitRedirect(courseStatus, courseId, sequenceStatus, sectionViaSequenceId, routeUnitId, navigate);
checkSectionUnitToUnitRedirect(
courseStatus,
courseId,
sequenceStatus,
sectionViaSequenceId,
routeUnitId,
navigate,
isPreview,
);

// Check section to sequence redirect:
// /course/:courseId/:sectionId -> /course/:courseId/:sequenceId
// by redirecting to the first sequence within the section.
checkSectionToSequenceRedirect(courseStatus, courseId, sequenceStatus, sectionViaSequenceId, routeUnitId, navigate);
checkSectionToSequenceRedirect(
courseStatus,
courseId,
sequenceStatus,
sectionViaSequenceId,
routeUnitId,
navigate,
);

// Check unit to sequence-unit redirect:
// /course/:courseId/:unitId -> /course/:courseId/:sequenceId/:unitId
// by filling in the ID of the parent sequence of :unitId.
checkUnitToSequenceUnitRedirect((
courseStatus, courseId, sequenceStatus, sequenceMightBeUnit,
sequenceId, sectionViaSequenceId, routeUnitId, navigate
));
checkUnitToSequenceUnitRedirect(
courseStatus,
courseId,
sequenceStatus,
sequenceMightBeUnit,
sequenceId,
sectionViaSequenceId,
routeUnitId,
navigate,
isPreview,
);

// Check sequence to sequence-unit redirect:
// /course/:courseId/:sequenceId -> /course/:courseId/:sequenceId/:unitId
// by filling in the ID the most-recently-active unit in the sequence, OR
// the ID of the first unit the sequence if none is active.
checkSequenceToSequenceUnitRedirect(courseId, sequenceStatus, sequence, routeUnitId, navigate);
checkSequenceToSequenceUnitRedirect(
courseId,
sequenceStatus,
sequence,
routeUnitId,
navigate,
isPreview,
);

// Check sequence-unit marker to sequence-unit redirect:
// /course/:courseId/:sequenceId/first -> /course/:courseId/:sequenceId/:unitId
// /course/:courseId/:sequenceId/last -> /course/:courseId/:sequenceId/:unitId
// by filling in the ID the first or last unit in the sequence.
// "Sequence unit marker" is an invented term used only in this component.
checkSequenceUnitMarkerToSequenceUnitRedirect(courseId, sequenceStatus, sequence, routeUnitId, navigate);
checkSequenceUnitMarkerToSequenceUnitRedirect(
courseId,
sequenceStatus,
sequence,
routeUnitId,
navigate,
isPreview,
);
}

handleUnitNavigationClick = () => {
Expand Down Expand Up @@ -334,6 +399,7 @@
fetchCourse: PropTypes.func.isRequired,
fetchSequence: PropTypes.func.isRequired,
navigate: PropTypes.func.isRequired,
isPreview: PropTypes.bool.isRequired,
};

CoursewareContainer.defaultProps = {
Expand Down
8 changes: 8 additions & 0 deletions src/courseware/course/Course.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { Helmet } from 'react-helmet';
import { useDispatch, useSelector } from 'react-redux';
import { getConfig } from '@edx/frontend-platform';
import { useLocation, useNavigate } from 'react-router-dom';
import { breakpoints, useWindowSize } from '@openedx/paragon';

import { AlertList } from '@src/generic/user-messages';
Expand Down Expand Up @@ -38,6 +39,13 @@
const section = useModel('sections', sequence ? sequence.sectionId : null);
const { enableNavigationSidebar } = useSelector(getCoursewareOutlineSidebarSettings);
const navigationDisabled = enableNavigationSidebar || (sequence?.navigationDisabled ?? false);
const navigate = useNavigate();
const { pathname } = useLocation();

if (!isStaff && pathname.startsWith('/preview')) {
const courseUrl = pathname.replace('/preview', '');
navigate(courseUrl, { replace: true });

Check warning on line 47 in src/courseware/course/Course.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/Course.jsx#L46-L47

Added lines #L46 - L47 were not covered by tests
}

const pageTitleBreadCrumbs = [
sequence,
Expand Down
1 change: 1 addition & 0 deletions src/courseware/course/sequence/Sequence.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ const Sequence = ({
sequenceId={sequenceId}
unitId={unitId}
unitLoadedHandler={handleUnitLoaded}
isStaff={isStaff}
/>
{unitHasLoaded && renderUnitNavigation(false)}
</div>
Expand Down
10 changes: 6 additions & 4 deletions src/courseware/course/sequence/SequenceContent.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { Suspense, useEffect } from 'react';
import PropTypes from 'prop-types';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { useIntl } from '@edx/frontend-platform/i18n';
import PageLoading from '../../../generic/PageLoading';
import { useModel } from '../../../generic/model-store';

Expand All @@ -11,12 +11,13 @@ const ContentLock = React.lazy(() => import('./content-lock'));

const SequenceContent = ({
gated,
intl,
courseId,
sequenceId,
unitId,
unitLoadedHandler,
isStaff,
}) => {
const intl = useIntl();
const sequence = useModel('sequences', sequenceId);

// Go back to the top of the page whenever the unit or sequence changes.
Expand Down Expand Up @@ -59,6 +60,7 @@ const SequenceContent = ({
key={unitId}
id={unitId}
onLoaded={unitLoadedHandler}
isStaff={isStaff}
/>
);
};
Expand All @@ -69,11 +71,11 @@ SequenceContent.propTypes = {
sequenceId: PropTypes.string.isRequired,
unitId: PropTypes.string,
unitLoadedHandler: PropTypes.func.isRequired,
intl: intlShape.isRequired,
isStaff: PropTypes.bool.isRequired,
};

SequenceContent.defaultProps = {
unitId: null,
};

export default injectIntl(SequenceContent);
export default SequenceContent;
7 changes: 6 additions & 1 deletion src/courseware/course/sequence/Unit/index.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import PropTypes from 'prop-types';
import React from 'react';
import { useSearchParams } from 'react-router-dom';
import { useSearchParams, useLocation } from 'react-router-dom';

import { AppContext } from '@edx/frontend-platform/react';
import { useIntl } from '@edx/frontend-platform/i18n';
Expand All @@ -22,22 +22,26 @@ const Unit = ({
format,
onLoaded,
id,
isStaff,
}) => {
const { formatMessage } = useIntl();
const [searchParams] = useSearchParams();
const { pathname } = useLocation();
const { authenticatedUser } = React.useContext(AppContext);
const examAccess = useExamAccess({ id });
const shouldDisplayHonorCode = useShouldDisplayHonorCode({ courseId, id });
const unit = useModel(modelKeys.units, id);
const isProcessing = unit.bookmarkedUpdateState === 'loading';
const view = authenticatedUser ? views.student : views.public;
const shouldDisplayUnitPreview = pathname.startsWith('/preview') && isStaff;

const getUrl = usePluginsCallback('getIFrameUrl', () => getIFrameUrl({
id,
view,
format,
examAccess,
jumpToId: searchParams.get('jumpToId'),
preview: shouldDisplayUnitPreview ? '1' : '0',
}));

const iframeUrl = getUrl();
Expand Down Expand Up @@ -74,6 +78,7 @@ Unit.propTypes = {
format: PropTypes.string,
id: PropTypes.string.isRequired,
onLoaded: PropTypes.func,
isStaff: PropTypes.bool.isRequired,
};

Unit.defaultProps = {
Expand Down
Loading
Loading