diff --git a/client/app/api/course/Assessment/Submission/Answer/Answer.ts b/client/app/api/course/Assessment/Submission/Answer/Answer.ts index fe3747c6dd2..fb0f53ae16c 100644 --- a/client/app/api/course/Assessment/Submission/Answer/Answer.ts +++ b/client/app/api/course/Assessment/Submission/Answer/Answer.ts @@ -4,10 +4,19 @@ import { JobSubmitted } from 'types/jobs'; import { APIResponse } from 'api/types'; import BaseAPI from '../../Base'; +import { getActivePreview } from '../../previewAttemptContext'; import SubmissionsAPI from '../../Submissions'; export default class AnswersAPI extends BaseAPI { get #urlPrefix(): string { + // See Submissions.js#urlPrefix. A marketplace preview URL has no /assessments/:aid + // segment, so per-answer save/submit must route to the shallow attempt endpoint; the + // attempt id (this.submissionId, resolved from the preview URL) stands in as the + // submission id. The `assessmentId === null` disjunct survives a stray poller firing + // after the singleton was cleared, so it never emits /assessments/null/... . + if (getActivePreview() !== null || this.assessmentId === null) { + return `/courses/${this.courseId}/marketplace/attempt/${this.submissionId}/answers`; + } return `/courses/${this.courseId}/assessments/${this.assessmentId}/submissions/${this.submissionId}/answers`; } diff --git a/client/app/api/course/Assessment/Submission/Answer/Scribing.js b/client/app/api/course/Assessment/Submission/Answer/Scribing.js index 7f96493a603..bd402acc84f 100644 --- a/client/app/api/course/Assessment/Submission/Answer/Scribing.js +++ b/client/app/api/course/Assessment/Submission/Answer/Scribing.js @@ -1,4 +1,5 @@ import BaseAssessmentAPI from '../../Base'; +import { getActivePreview } from '../../previewAttemptContext'; export default class ScribingsAPI extends BaseAssessmentAPI { /** @@ -12,6 +13,12 @@ export default class ScribingsAPI extends BaseAssessmentAPI { } get #urlPrefix() { + // See Submissions.js#urlPrefix. In preview the path is keyed on the attempt id + // (the submission base-record id), which the browser path does not expose, so it + // comes from the singleton instead of `this.submissionId` (null on preview URLs). + if (getActivePreview() !== null || this.assessmentId === null) { + return `/courses/${this.courseId}/marketplace/attempt/${getActivePreview()}/answers`; + } return `/courses/${this.courseId}/assessments/${this.assessmentId}/submissions/${this.submissionId}/answers`; } } diff --git a/client/app/api/course/Assessment/Submissions.js b/client/app/api/course/Assessment/Submissions.js index 20c2bc70332..369e6b4bf2b 100644 --- a/client/app/api/course/Assessment/Submissions.js +++ b/client/app/api/course/Assessment/Submissions.js @@ -1,4 +1,5 @@ import BaseAssessmentAPI from './Base'; +import { getActivePreview } from './previewAttemptContext'; export default class SubmissionsAPI extends BaseAssessmentAPI { index() { @@ -122,6 +123,10 @@ export default class SubmissionsAPI extends BaseAssessmentAPI { return this.client.post(`${this.#urlPrefix}/${submissionId}/auto_grade`); } + reset(submissionId) { + return this.client.post(`${this.#urlPrefix}/${submissionId}/reset`); + } + reevaluateAnswer(submissionId, params) { return this.client.post( `${this.#urlPrefix}/${submissionId}/reevaluate_answer`, @@ -198,6 +203,14 @@ export default class SubmissionsAPI extends BaseAssessmentAPI { } get #urlPrefix() { + // Preview routing triggers on EITHER the explicit context OR the absence of an + // assessment id in the URL. The only reuse of this API on a non-/assessments/:id + // URL is the marketplace preview page, so `this.assessmentId === null` is a robust + // fallback that survives a stray poller firing after the singleton was cleared: + // it routes to /marketplace/attempt/... rather than ever emitting /assessments/null/... + if (getActivePreview() !== null || this.assessmentId === null) { + return `/courses/${this.courseId}/marketplace/attempt`; + } return `/courses/${this.courseId}/assessments/${this.assessmentId}/submissions`; } diff --git a/client/app/api/course/Assessment/__test__/previewAttemptContext.test.ts b/client/app/api/course/Assessment/__test__/previewAttemptContext.test.ts new file mode 100644 index 00000000000..3648d4f6e65 --- /dev/null +++ b/client/app/api/course/Assessment/__test__/previewAttemptContext.test.ts @@ -0,0 +1,24 @@ +import { + clearActivePreview, + getActivePreview, + setActivePreview, +} from '../previewAttemptContext'; + +describe('previewAttemptContext', () => { + afterEach(() => clearActivePreview()); + + it('is null by default', () => { + expect(getActivePreview()).toBeNull(); + }); + + it('returns the id after setActivePreview', () => { + setActivePreview(42); + expect(getActivePreview()).toBe(42); + }); + + it('returns null after clearActivePreview', () => { + setActivePreview(42); + clearActivePreview(); + expect(getActivePreview()).toBeNull(); + }); +}); diff --git a/client/app/api/course/Assessment/__test__/previewRouting.test.ts b/client/app/api/course/Assessment/__test__/previewRouting.test.ts new file mode 100644 index 00000000000..101f0afa218 --- /dev/null +++ b/client/app/api/course/Assessment/__test__/previewRouting.test.ts @@ -0,0 +1,203 @@ +import { createMockAdapter } from 'mocks/axiosMock'; + +import CourseAPI from 'api/course'; + +import { clearActivePreview, setActivePreview } from '../previewAttemptContext'; + +// The submission API reads the assessment id from window.location. Preview URLs have +// none, so the seam must fall back to preview routing even without the singleton. +const setPath = (path: string): void => { + window.history.pushState({}, '', path); +}; + +const submissionsMock = createMockAdapter( + CourseAPI.assessment.submissions.client, +); +const scribingMock = createMockAdapter( + CourseAPI.assessment.answer.scribing.client, +); +const answerMock = createMockAdapter(CourseAPI.assessment.answer.answer.client); +const marketplaceMock = createMockAdapter(CourseAPI.marketplace.client); + +beforeEach(() => { + submissionsMock.reset(); + scribingMock.reset(); + answerMock.reset(); + marketplaceMock.reset(); + clearActivePreview(); +}); +afterEach(() => clearActivePreview()); + +describe('submission API preview routing', () => { + it('routes edit to /assessments/:aid/submissions when NOT in preview', async () => { + setPath(`/courses/${global.courseId}/assessments/9/submissions/5/edit`); + submissionsMock + .onGet(`/courses/${global.courseId}/assessments/9/submissions/5/edit`) + .reply(200, {}); + + await CourseAPI.assessment.submissions.edit(5); + + expect(submissionsMock.history.get[0].url).toBe( + `/courses/${global.courseId}/assessments/9/submissions/5/edit`, + ); + }); + + it('routes edit to /marketplace/attempt when the singleton is set', async () => { + setPath(`/courses/${global.courseId}/marketplace/attempt/5/edit`); + setActivePreview(5); + submissionsMock + .onGet(`/courses/${global.courseId}/marketplace/attempt/5/edit`) + .reply(200, {}); + + await CourseAPI.assessment.submissions.edit(5); + + expect(submissionsMock.history.get[0].url).toBe( + `/courses/${global.courseId}/marketplace/attempt/5/edit`, + ); + }); + + it('routes to /marketplace/attempt when assessment id is absent even if the singleton was cleared (stray poller)', async () => { + setPath(`/courses/${global.courseId}/marketplace/attempt/5/edit`); + clearActivePreview(); + submissionsMock + .onGet( + `/courses/${global.courseId}/marketplace/attempt/fetch_live_feedback_status`, + ) + .reply(200, {}); + + await CourseAPI.assessment.submissions.fetchLiveFeedbackStatus('t-1'); + + // The critical assertion: it must NOT be /assessments/null/... + expect(submissionsMock.history.get[0].url).toBe( + `/courses/${global.courseId}/marketplace/attempt/fetch_live_feedback_status`, + ); + expect(submissionsMock.history.get[0].url).not.toContain( + 'assessments/null', + ); + }); + + it('routes reset to /marketplace/attempt/:id/reset', async () => { + setPath(`/courses/${global.courseId}/marketplace/attempt/5/edit`); + setActivePreview(5); + submissionsMock + .onPost(`/courses/${global.courseId}/marketplace/attempt/5/reset`) + .reply(200, {}); + + await CourseAPI.assessment.submissions.reset(5); + + expect(submissionsMock.history.post[0].url).toBe( + `/courses/${global.courseId}/marketplace/attempt/5/reset`, + ); + }); +}); + +describe('answer API preview routing', () => { + it('routes saveDraft to /assessments/:aid/submissions/:sid/answers when NOT in preview', async () => { + setPath(`/courses/${global.courseId}/assessments/9/submissions/5/edit`); + answerMock + .onPatch( + `/courses/${global.courseId}/assessments/9/submissions/5/answers/7`, + ) + .reply(200, {}); + + await CourseAPI.assessment.answer.answer.saveDraft(7, {}); + + expect(answerMock.history.patch[0].url).toBe( + `/courses/${global.courseId}/assessments/9/submissions/5/answers/7`, + ); + }); + + it('routes saveDraft to /marketplace/attempt/:attemptId/answers/:answerId in preview', async () => { + setPath(`/courses/${global.courseId}/marketplace/attempt/5/edit`); + setActivePreview(5); + answerMock + .onPatch(`/courses/${global.courseId}/marketplace/attempt/5/answers/7`) + .reply(200, {}); + + await CourseAPI.assessment.answer.answer.saveDraft(7, {}); + + expect(answerMock.history.patch[0].url).toBe( + `/courses/${global.courseId}/marketplace/attempt/5/answers/7`, + ); + expect(answerMock.history.patch[0].url).not.toContain('assessments/null'); + }); + + it('routes saveDraft to /marketplace/attempt even after the singleton was cleared (stray poller)', async () => { + setPath(`/courses/${global.courseId}/marketplace/attempt/5/edit`); + clearActivePreview(); + answerMock + .onPatch(`/courses/${global.courseId}/marketplace/attempt/5/answers/7`) + .reply(200, {}); + + await CourseAPI.assessment.answer.answer.saveDraft(7, {}); + + expect(answerMock.history.patch[0].url).toBe( + `/courses/${global.courseId}/marketplace/attempt/5/answers/7`, + ); + expect(answerMock.history.patch[0].url).not.toContain('assessments/null'); + }); + + it('routes submitAnswer to /marketplace/attempt/:attemptId/answers/:answerId/submit_answer in preview', async () => { + setPath(`/courses/${global.courseId}/marketplace/attempt/5/edit`); + setActivePreview(5); + answerMock + .onPatch( + `/courses/${global.courseId}/marketplace/attempt/5/answers/7/submit_answer`, + ) + .reply(200, {}); + + await CourseAPI.assessment.answer.answer.submitAnswer(7, {}); + + expect(answerMock.history.patch[0].url).toBe( + `/courses/${global.courseId}/marketplace/attempt/5/answers/7/submit_answer`, + ); + }); +}); + +describe('scribing API preview routing', () => { + it('routes scribble update to /assessments/:aid/submissions/:sid/answers when NOT in preview', async () => { + setPath(`/courses/${global.courseId}/assessments/9/submissions/5/edit`); + scribingMock + .onPost( + `/courses/${global.courseId}/assessments/9/submissions/5/answers/7/scribing/scribbles`, + ) + .reply(200, {}); + + await CourseAPI.assessment.answer.scribing.update(7, {}); + + expect(scribingMock.history.post[0].url).toBe( + `/courses/${global.courseId}/assessments/9/submissions/5/answers/7/scribing/scribbles`, + ); + }); + + it('routes scribble update to /marketplace/attempt/:attemptId/answers when in preview', async () => { + setPath(`/courses/${global.courseId}/marketplace/attempt/5/edit`); + setActivePreview(5); + scribingMock + .onPost( + `/courses/${global.courseId}/marketplace/attempt/5/answers/7/scribing/scribbles`, + ) + .reply(200, {}); + + await CourseAPI.assessment.answer.scribing.update(7, {}); + + expect(scribingMock.history.post[0].url).toBe( + `/courses/${global.courseId}/marketplace/attempt/5/answers/7/scribing/scribbles`, + ); + }); +}); + +describe('marketplace createAttempt', () => { + it('posts to listings/:listingId/attempt and returns id + assessmentId', async () => { + marketplaceMock + .onPost(`/courses/${global.courseId}/marketplace/listings/7/attempt`) + .reply(200, { id: 55, assessmentId: 9 }); + + const response = await CourseAPI.marketplace.createAttempt(7); + + expect(marketplaceMock.history.post[0].url).toBe( + `/courses/${global.courseId}/marketplace/listings/7/attempt`, + ); + expect(response.data).toEqual({ id: 55, assessmentId: 9 }); + }); +}); diff --git a/client/app/api/course/Assessment/previewAttemptContext.ts b/client/app/api/course/Assessment/previewAttemptContext.ts new file mode 100644 index 00000000000..5b8fa194c68 --- /dev/null +++ b/client/app/api/course/Assessment/previewAttemptContext.ts @@ -0,0 +1,22 @@ +/** + * Module-level marker for "the current page is a marketplace preview attempt". + * + * The reused submission API (`SubmissionsAPI`, `ScribingsAPI`) builds its URLs from + * the browser path. A preview URL (`/courses/:c/marketplace/attempt/:id`) has no + * `/assessments/:aid` segment, so the API must be told to route to the shallow + * `/marketplace/attempt/...` endpoints instead. This singleton is that signal. + * + * It is a plain module variable (not React context) because the API layer runs + * outside React and cannot read context. + */ +let activePreviewAttemptId: number | null = null; + +export const setActivePreview = (attemptId: number): void => { + activePreviewAttemptId = attemptId; +}; + +export const clearActivePreview = (): void => { + activePreviewAttemptId = null; +}; + +export const getActivePreview = (): number | null => activePreviewAttemptId; diff --git a/client/app/api/course/Marketplace.ts b/client/app/api/course/Marketplace.ts index 8f1f4bf7ea8..74d09d6fdc9 100644 --- a/client/app/api/course/Marketplace.ts +++ b/client/app/api/course/Marketplace.ts @@ -50,4 +50,10 @@ export default class MarketplaceAPI extends BaseCourseAPI { `${this.#urlPrefix}/listings/${listingId}/questions/${questionId}`, ); } + + createAttempt( + listingId: number, + ): Promise> { + return this.client.post(`${this.#urlPrefix}/listings/${listingId}/attempt`); + } } diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/PreviewAttemptBanner.tsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/PreviewAttemptBanner.tsx new file mode 100644 index 00000000000..b2b48211b65 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/PreviewAttemptBanner.tsx @@ -0,0 +1,69 @@ +import { useContext } from 'react'; +import { useNavigate } from 'react-router-dom'; +import { RestartAlt, Visibility } from '@mui/icons-material'; +import { Alert, Button } from '@mui/material'; + +import CourseAPI from 'api/course'; +import { useCourseContext } from 'course/container/CourseLoader'; +import { setNotification } from 'lib/actions'; +import { useAppDispatch } from 'lib/hooks/store'; +import useTranslation from 'lib/hooks/useTranslation'; + +import { fetchSubmission } from '../../actions'; +import translations from '../../translations'; + +import PreviewAttemptContext from './PreviewAttemptContext'; + +const PreviewAttemptBanner = (): JSX.Element | null => { + const { isPreview, attemptId, listingId } = useContext(PreviewAttemptContext); + const dispatch = useAppDispatch(); + const navigate = useNavigate(); + // This banner mounts on the shared submission edit page too. Read the course context + // defensively (optional chaining, not destructuring) so it never throws there — the + // component early-returns for real submissions before courseUrl is ever used. + const courseContext = useCourseContext(); + const { t } = useTranslation(); + + if (!isPreview || attemptId === undefined) return null; + + const courseUrl = courseContext?.courseUrl; + const backTo = listingId + ? `${courseUrl}/marketplace/listings/${listingId}` + : `${courseUrl}/marketplace`; + + const handleReset = async (): Promise => { + await CourseAPI.assessment.submissions.reset(attemptId); + dispatch(setNotification(translations.resetPreviewSuccess)); + dispatch(fetchSubmission(attemptId)); + }; + + return ( + + + + + } + icon={} + severity="info" + > + {t(translations.previewBanner)} + + ); +}; + +export default PreviewAttemptBanner; diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/PreviewAttemptContext.tsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/PreviewAttemptContext.tsx new file mode 100644 index 00000000000..7ddd5f2102b --- /dev/null +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/PreviewAttemptContext.tsx @@ -0,0 +1,18 @@ +import { createContext } from 'react'; + +export interface PreviewAttemptContextValue { + isPreview: boolean; + attemptId?: number; + listingId?: number; +} + +/** + * Tells the reused submission edit page it is rendering a marketplace preview attempt. + * Default `isPreview: false` so the shared page renders normally for real submissions + * (no provider on the real submission route). + */ +const PreviewAttemptContext = createContext({ + isPreview: false, +}); + +export default PreviewAttemptContext; diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/__test__/PreviewAttemptBanner.test.tsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/__test__/PreviewAttemptBanner.test.tsx new file mode 100644 index 00000000000..5f7d4aa3ca4 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/__test__/PreviewAttemptBanner.test.tsx @@ -0,0 +1,69 @@ +import { render, screen, waitFor } from 'test-utils'; + +import CourseAPI from 'api/course'; + +import PreviewAttemptBanner from '../PreviewAttemptBanner'; +import PreviewAttemptContext from '../PreviewAttemptContext'; + +const mockDispatch = jest.fn(); +jest.mock('lib/hooks/store', () => ({ + ...jest.requireActual('lib/hooks/store'), + useAppDispatch: () => mockDispatch, +})); + +jest.mock('course/container/CourseLoader', () => ({ + useCourseContext: () => ({ courseUrl: `/courses/${global.courseId}` }), +})); + +beforeEach(() => mockDispatch.mockClear()); + +// TestApp always mounts a Toastify container + an I18n LoadingIndicator (removed once +// messages load), so the render container is never truly empty. The meaningful check +// is that the banner emits no Alert in the default (non-preview) context. Wait for the +// loading indicator to clear first, so this isn't vacuously true during async load. +it('renders no banner under the default (non-preview) context', async () => { + render(); + await waitFor(() => + expect(screen.queryByTestId('CircularProgress')).not.toBeInTheDocument(), + ); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); +}); + +it('renders the preview banner with Reset and Exit when isPreview', async () => { + render( + + + , + ); + + expect( + await screen.findByText(/you are previewing this assessment/i), + ).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /reset/i })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /exit/i })).toBeInTheDocument(); +}); + +it('Reset calls the reset API and re-fetches the attempt', async () => { + const resetSpy = jest + .spyOn(CourseAPI.assessment.submissions, 'reset') + .mockResolvedValue({ data: {} } as never); + + render( + + + , + ); + + (await screen.findByRole('button', { name: /reset/i })).click(); + + await waitFor(() => expect(resetSpy).toHaveBeenCalledWith(5)); + // fetchSubmission is a thunk (a function); assert a function was dispatched after reset. + await waitFor(() => + expect(mockDispatch).toHaveBeenCalledWith(expect.any(Function)), + ); + resetSpy.mockRestore(); +}); diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx index 1ec74134964..8fea0652e17 100644 --- a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx @@ -40,6 +40,7 @@ import { import translations from '../../translations'; import BlockedSubmission from './BlockedSubmission'; +import PreviewAttemptBanner from './PreviewAttemptBanner'; import SubmissionEmptyForm from './SubmissionEmptyForm'; import SubmissionForm from './SubmissionForm'; import TimeLimitBanner from './TimeLimitBanner'; @@ -173,6 +174,7 @@ class VisibleSubmissionEditIndex extends Component { return ( + {this.renderTimeLimitBanner()} {this.renderAssessment()} {isBlockedInStudentView ? ( diff --git a/client/app/bundles/course/assessment/submission/reducers/__test__/recorder.test.js b/client/app/bundles/course/assessment/submission/reducers/__test__/recorder.test.js new file mode 100644 index 00000000000..01b1ffef3e9 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/reducers/__test__/recorder.test.js @@ -0,0 +1,41 @@ +import recorderHelper from '../../../utils/recorderHelper'; +import actionTypes from '../../constants'; +import recorderReducer from '../recorder'; + +jest.mock('../../../utils/recorderHelper', () => ({ + stopRecord: jest.fn(() => Promise.resolve()), + isRecording: jest.fn(() => false), +})); + +describe('recorder reducer', () => { + beforeEach(() => jest.clearAllMocks()); + + it('does not stop the recorder on final unmount when it is not recording', () => { + // Unconditionally calling stopRecord() here returns a rejected promise + // ('Recorder has already stopped') that nothing handles -> the dev error overlay. + recorderHelper.isRecording.mockReturnValue(false); + const state = { recorderComponentsCount: 1, recording: false }; + + recorderReducer(state, { type: actionTypes.RECORDER_COMPONENT_UNMOUNT }); + + expect(recorderHelper.stopRecord).not.toHaveBeenCalled(); + }); + + it('stops the recorder on final unmount when it is still recording', () => { + recorderHelper.isRecording.mockReturnValue(true); + const state = { recorderComponentsCount: 1, recording: true }; + + recorderReducer(state, { type: actionTypes.RECORDER_COMPONENT_UNMOUNT }); + + expect(recorderHelper.stopRecord).toHaveBeenCalledTimes(1); + }); + + it('does not stop the recorder while other recorder components remain mounted', () => { + recorderHelper.isRecording.mockReturnValue(true); + const state = { recorderComponentsCount: 2, recording: true }; + + recorderReducer(state, { type: actionTypes.RECORDER_COMPONENT_UNMOUNT }); + + expect(recorderHelper.stopRecord).not.toHaveBeenCalled(); + }); +}); diff --git a/client/app/bundles/course/assessment/submission/reducers/recorder.js b/client/app/bundles/course/assessment/submission/reducers/recorder.js index 59f8d1d2f8c..0d5ed4d0c3b 100644 --- a/client/app/bundles/course/assessment/submission/reducers/recorder.js +++ b/client/app/bundles/course/assessment/submission/reducers/recorder.js @@ -39,9 +39,13 @@ export default function (state = initialState, action) { /** * When the user navigate to other path without stopping the recorder - * We need to help the user to stop + * We need to help the user to stop. + * + * Guard on isRecording(): stopRecord() rejects with 'Recorder has already + * stopped' when nothing is being recorded, and that rejection is unhandled + * here (surfacing as an error overlay). Only stop what is actually recording. */ - if (recorderComponentsCount === 0) { + if (recorderComponentsCount === 0 && recorderHelper.isRecording()) { recorderHelper.stopRecord(); } return { diff --git a/client/app/bundles/course/assessment/submission/translations.ts b/client/app/bundles/course/assessment/submission/translations.ts index fd82e5b2afa..3e3ff564cbc 100644 --- a/client/app/bundles/course/assessment/submission/translations.ts +++ b/client/app/bundles/course/assessment/submission/translations.ts @@ -10,6 +10,23 @@ const translations = defineMessages({ id: 'course.assessment.submission.submissionsHeader', defaultMessage: 'Submissions: {assessment}', }, + previewBanner: { + id: 'course.assessment.submission.previewBanner', + defaultMessage: + 'You are previewing this assessment. This is a throwaway attempt — nothing here is recorded, graded for credit, or visible to students.', + }, + resetPreview: { + id: 'course.assessment.submission.resetPreview', + defaultMessage: 'Reset', + }, + exitPreview: { + id: 'course.assessment.submission.exitPreview', + defaultMessage: 'Exit preview', + }, + resetPreviewSuccess: { + id: 'course.assessment.submission.resetPreviewSuccess', + defaultMessage: 'Preview attempt reset.', + }, studentView: { id: 'course.assessment.submission.studentView', defaultMessage: 'Student View', diff --git a/client/app/bundles/course/marketplace/operations.ts b/client/app/bundles/course/marketplace/operations.ts index ff0303ce4a8..e8293bb3cac 100644 --- a/client/app/bundles/course/marketplace/operations.ts +++ b/client/app/bundles/course/marketplace/operations.ts @@ -50,3 +50,10 @@ export const fetchQuestion = async ( ); return response.data as QuestionPreviewData; }; + +export const createAttempt = async ( + listingId: number, +): Promise<{ id: number; assessmentId: number }> => { + const response = await CourseAPI.marketplace.createAttempt(listingId); + return response.data; +}; diff --git a/client/app/bundles/course/marketplace/pages/ListingPreview/__test__/index.test.tsx b/client/app/bundles/course/marketplace/pages/ListingPreview/__test__/index.test.tsx index c687b47d634..40adccb0475 100644 --- a/client/app/bundles/course/marketplace/pages/ListingPreview/__test__/index.test.tsx +++ b/client/app/bundles/course/marketplace/pages/ListingPreview/__test__/index.test.tsx @@ -208,3 +208,63 @@ it('marks the page title as a preview', async () => { // for the real assessment it mirrors. expect(screen.getByText('Preview')).toBeVisible(); }); + +it('creates a preview attempt and navigates to the preview edit page', async () => { + const url = `/courses/${global.courseId}/marketplace/listings/7`; + mock.onGet(url).reply(200, { + id: 70, + title: LISTING_TITLE, + destinationTabs: [], + description: '', + typeCounts: {}, + questions: [], + }); + mock + .onPost(`/courses/${global.courseId}/marketplace/listings/7/attempt`) + .reply(200, { id: 55, assessmentId: 9 }); + + render(, { at: [url] }); + await waitFor(() => expect(screen.getByText(LISTING_TITLE)).toBeVisible()); + + fireEvent.click(screen.getByRole('button', { name: 'Attempt' })); + + await waitFor(() => + expect(mockNavigate).toHaveBeenCalledWith( + `/courses/${global.courseId}/marketplace/attempt/55/edit?fromListing=7`, + ), + ); +}); + +it('shows a notification and does not navigate when the attempt create conflicts (409)', async () => { + const url = `/courses/${global.courseId}/marketplace/listings/7`; + mock.onGet(url).reply(200, { + id: 70, + title: LISTING_TITLE, + destinationTabs: [], + description: '', + typeCounts: {}, + questions: [], + }); + mock + .onPost(`/courses/${global.courseId}/marketplace/listings/7/attempt`) + .reply(409, { + errors: ['You already have a submission for this assessment.'], + }); + + render(, { at: [url] }); + await waitFor(() => expect(screen.getByText(LISTING_TITLE)).toBeVisible()); + + fireEvent.click(screen.getByRole('button', { name: 'Attempt' })); + + // Wait for the create POST to actually round-trip (and 409), THEN assert no + // navigation followed — otherwise "not navigated" is trivially true before the + // request even fires. + await waitFor(() => + expect( + mock.history.post.filter((r) => + r.url?.endsWith('/marketplace/listings/7/attempt'), + ), + ).toHaveLength(1), + ); + expect(mockNavigate).not.toHaveBeenCalled(); +}); diff --git a/client/app/bundles/course/marketplace/pages/ListingPreview/index.tsx b/client/app/bundles/course/marketplace/pages/ListingPreview/index.tsx index 5e6c9a04abd..ec2cb3605c5 100644 --- a/client/app/bundles/course/marketplace/pages/ListingPreview/index.tsx +++ b/client/app/bundles/course/marketplace/pages/ListingPreview/index.tsx @@ -1,6 +1,6 @@ import { useState } from 'react'; import { useParams, useSearchParams } from 'react-router-dom'; -import { ContentCopy } from '@mui/icons-material'; +import { ContentCopy, PlayArrow } from '@mui/icons-material'; import { Button, Chip, Paper } from '@mui/material'; // Reuse the assessment show page's "Questions" heading so wording + locales stay identical. @@ -17,6 +17,7 @@ import { withFromTab } from '../../fromTab'; import { fetchListing } from '../../operations'; import translations from '../../translations'; import { ListingPreviewData } from '../../types'; +import useStartPreviewAttempt from '../../useStartPreviewAttempt'; import PreviewAssessmentDetails from './PreviewAssessmentDetails'; import PreviewQuestionCard from './PreviewQuestionCard'; @@ -32,6 +33,9 @@ const ListingPreview = (): JSX.Element => { const destinationTabId = parseInt(fromTab ?? '', 10) || null; const [duplicating, setDuplicating] = useState(false); + const { starting: attempting, start: startAttempt } = + useStartPreviewAttempt(); + return ( } @@ -40,14 +44,25 @@ const ListingPreview = (): JSX.Element => { {(listing): JSX.Element => ( setDuplicating(true)} - startIcon={} - variant="contained" - > - {t(translations.duplicateAssessment)} - + <> + + + } backTo={withFromTab(`${courseUrl}/marketplace`, fromTab)} className="space-y-5" diff --git a/client/app/bundles/course/marketplace/pages/MarketplaceIndex/MarketplaceTable.tsx b/client/app/bundles/course/marketplace/pages/MarketplaceIndex/MarketplaceTable.tsx index 416bd9b2865..89d0fb0a325 100644 --- a/client/app/bundles/course/marketplace/pages/MarketplaceIndex/MarketplaceTable.tsx +++ b/client/app/bundles/course/marketplace/pages/MarketplaceIndex/MarketplaceTable.tsx @@ -2,6 +2,7 @@ import { useMemo, useState } from 'react'; import { useIntl } from 'react-intl'; import { ContentCopy, + PlayArrow, StorefrontOutlined, VisibilityOutlined, } from '@mui/icons-material'; @@ -27,12 +28,14 @@ type SortMode = 'adoptions' | 'newest'; interface Props { fromTab?: string | null; listings: MarketplaceListing[]; + onAttempt?: (listing: MarketplaceListing) => void; onDuplicate: (rows: MarketplaceListing[]) => void; } const MarketplaceTable = ({ fromTab = null, listings, + onAttempt = (): void => {}, onDuplicate, }: Props): JSX.Element => { const { formatMessage: t } = useIntl(); @@ -87,6 +90,15 @@ const MarketplaceTable = ({ + + onAttempt(l)} + size="small" + > + + + { + const onAttempt = jest.fn(); + const page = render( + , + ); + + // Default sort = adoptions desc → Graph Theory (12) is the first row; [0] is its cell. + const preview = (await page.findAllByLabelText('Preview'))[0]; + const attempt = (await page.findAllByLabelText('Attempt'))[0]; + const duplicate = (await page.findAllByLabelText('Duplicate'))[0]; + + // Order within the row must be preview → attempt → duplicate. + /* eslint-disable no-bitwise */ + expect( + preview.compareDocumentPosition(attempt) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + expect( + attempt.compareDocumentPosition(duplicate) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + /* eslint-enable no-bitwise */ + + fireEvent.click(attempt); + expect(onAttempt).toHaveBeenCalledWith( + expect.objectContaining({ title: GRAPH_THEORY }), + ); +}); + it('carries from_tab into the preview links when set', async () => { const page = render( { const fromTab = params.get('from_tab'); const destinationTabId = parseInt(fromTab ?? '', 10) || null; const [pending, setPending] = useState([]); + const { start: startAttempt } = useStartPreviewAttempt(); return ( } while={fetchListings}> @@ -28,6 +30,7 @@ const MarketplaceIndex = (): JSX.Element => { => startAttempt(listing.id)} onDuplicate={setPending} />
preview id: {String(getActivePreview())}
; + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useParams: () => ({ submissionId: '5' }), + useSearchParams: () => [new URLSearchParams('fromListing=7'), jest.fn()], + // Stand in for CourseContainer's outlet context so we can assert the wrapper + // forwards it (useCourseContext() delegates to useOutletContext()). + useOutletContext: () => ({ courseUrl: '/courses/1' }), + // Render both the probe (for the singleton timing check) and the context the + // wrapper forwarded via (for the forwarding check). + Outlet: ({ context }: { context?: { courseUrl?: string } }): JSX.Element => ( + <> + +
forwarded: {context?.courseUrl ?? 'none'}
+ + ), +})); + +// TestApp's I18nProvider loads locale messages via a dynamic import(), so the tree +// mounts asynchronously — the first assertion must be an async findBy/waitFor. +it('sets the preview singleton during render, before its child renders', async () => { + render(); + // The Probe reads getActivePreview() during ITS OWN render; seeing '5' proves the + // wrapper set the singleton before the child rendered (i.e. in render, not an effect). + expect(await screen.findByText('preview id: 5')).toBeInTheDocument(); +}); + +it('forwards the course outlet context down to the child route', async () => { + render(); + // A bare would null the context; this proves CourseLayoutData is forwarded + // so the reused SubmissionEditIndex banner can read courseUrl on the preview page. + expect(await screen.findByText('forwarded: /courses/1')).toBeInTheDocument(); +}); + +it('clears the singleton on unmount', async () => { + const { unmount } = render(); + await screen.findByText('preview id: 5'); + expect(getActivePreview()).toBe(5); + unmount(); + expect(getActivePreview()).toBeNull(); +}); diff --git a/client/app/bundles/course/marketplace/pages/PreviewAttempt/index.tsx b/client/app/bundles/course/marketplace/pages/PreviewAttempt/index.tsx new file mode 100644 index 00000000000..c33f97c110a --- /dev/null +++ b/client/app/bundles/course/marketplace/pages/PreviewAttempt/index.tsx @@ -0,0 +1,45 @@ +import { useEffect, useMemo } from 'react'; +import { Outlet, useParams, useSearchParams } from 'react-router-dom'; + +import { + clearActivePreview, + setActivePreview, +} from 'api/course/Assessment/previewAttemptContext'; +import PreviewAttemptContext from 'course/assessment/submission/pages/SubmissionEditIndex/PreviewAttemptContext'; +import { useCourseContext } from 'course/container/CourseLoader'; + +const PreviewAttemptScope = (): JSX.Element => { + const { submissionId } = useParams(); + const attemptId = Number(submissionId); + const [params] = useSearchParams(); + const listingParam = params.get('fromListing'); + const listingId = listingParam ? Number(listingParam) : undefined; + + // Forward CourseContainer's outlet context (CourseLayoutData, incl. courseUrl) down + // to the reused SubmissionEditIndex. A bare would set the outlet context to + // undefined, so the child's useCourseContext() — used by the preview banner — would + // otherwise read undefined on the preview page. + const courseContext = useCourseContext(); + + // Set the singleton SYNCHRONOUSLY in the render body — not in a useEffect. The child + // SubmissionEditIndex dispatches fetchSubmission in componentDidMount, which fires + // BEFORE this parent's effects run. A parent renders before its children, so setting + // it here guarantees the API seam sees the preview context when the child builds its + // first request URL. Idempotent, so re-running on every render is harmless. + setActivePreview(attemptId); + + useEffect(() => () => clearActivePreview(), []); + + const previewContext = useMemo( + () => ({ isPreview: true, attemptId, listingId }), + [attemptId, listingId], + ); + + return ( + + + + ); +}; + +export default PreviewAttemptScope; diff --git a/client/app/bundles/course/marketplace/translations.ts b/client/app/bundles/course/marketplace/translations.ts index 3818d2785cd..77490f84577 100644 --- a/client/app/bundles/course/marketplace/translations.ts +++ b/client/app/bundles/course/marketplace/translations.ts @@ -73,6 +73,14 @@ export default defineMessages({ id: 'course.marketplace.duplicateAssessment', defaultMessage: 'Duplicate Assessment', }, + attemptAssessment: { + id: 'course.marketplace.attemptAssessment', + defaultMessage: 'Attempt', + }, + attemptFailed: { + id: 'course.marketplace.attemptFailed', + defaultMessage: 'Could not start a preview attempt.', + }, viewDetails: { id: 'course.marketplace.viewDetails', defaultMessage: 'View question details', diff --git a/client/app/bundles/course/marketplace/useStartPreviewAttempt.ts b/client/app/bundles/course/marketplace/useStartPreviewAttempt.ts new file mode 100644 index 00000000000..d680fcbf808 --- /dev/null +++ b/client/app/bundles/course/marketplace/useStartPreviewAttempt.ts @@ -0,0 +1,45 @@ +import { useState } from 'react'; +import { useNavigate } from 'react-router-dom'; + +import { useCourseContext } from 'course/container/CourseLoader'; +import { setNotification } from 'lib/actions'; +import { useAppDispatch } from 'lib/hooks/store'; + +import { createAttempt } from './operations'; +import translations from './translations'; + +interface StartPreviewAttempt { + starting: boolean; + start: (listingId: number) => Promise; +} + +/** + * Shared entrance into a marketplace preview attempt: create the attempt, then navigate + * to the reused submission editor (carrying `fromListing` so the preview banner's Exit can + * link back to the listing). On a 409 conflict (the previewer already has a real submission + * for the assessment) it notifies and stays put. Used by both the listing preview page and + * the marketplace index row action. + */ +const useStartPreviewAttempt = (): StartPreviewAttempt => { + const navigate = useNavigate(); + const dispatch = useAppDispatch(); + const { courseUrl } = useCourseContext(); + const [starting, setStarting] = useState(false); + + const start = async (listingId: number): Promise => { + setStarting(true); + try { + const { id } = await createAttempt(listingId); + navigate( + `${courseUrl}/marketplace/attempt/${id}/edit?fromListing=${listingId}`, + ); + } catch { + dispatch(setNotification(translations.attemptFailed)); + setStarting(false); + } + }; + + return { starting, start }; +}; + +export default useStartPreviewAttempt; diff --git a/client/app/lib/helpers/__test__/url-helpers.test.ts b/client/app/lib/helpers/__test__/url-helpers.test.ts new file mode 100644 index 00000000000..764f51729b6 --- /dev/null +++ b/client/app/lib/helpers/__test__/url-helpers.test.ts @@ -0,0 +1,26 @@ +import { getSubmissionId } from '../url-helpers'; + +const setPath = (path: string): void => { + window.history.pushState({}, '', path); +}; + +describe('getSubmissionId', () => { + it('reads the submission id from a submission edit URL', () => { + setPath('/courses/3/assessments/9/submissions/5/edit'); + expect(getSubmissionId()).toBe('5'); + }); + + it('reads the attempt id from a marketplace preview attempt URL', () => { + // The preview page reuses the submission edit UI on a shallow attempt URL that has + // no /assessments/:aid/submissions segment. The attempt id plays the submission id, + // so pollers/buttons deriving the id from the URL must not fall through to null and + // emit /marketplace/attempt/null/... + setPath('/courses/3/marketplace/attempt/5/edit'); + expect(getSubmissionId()).toBe('5'); + }); + + it('returns null when the path is neither a submission nor a preview attempt', () => { + setPath('/courses/3/assessments/9'); + expect(getSubmissionId()).toBeNull(); + }); +}); diff --git a/client/app/lib/helpers/url-helpers.ts b/client/app/lib/helpers/url-helpers.ts index c55b137b774..3b8a5c5c258 100644 --- a/client/app/lib/helpers/url-helpers.ts +++ b/client/app/lib/helpers/url-helpers.ts @@ -80,7 +80,17 @@ function getSubmissionId(): string | null { const match = window.location.pathname.match( /^\/courses\/\d+\/assessments\/\d+\/submissions\/(\d+)/, ); - return match?.[1] ?? null; + if (match) return match[1]; + + // The marketplace preview reuses the submission edit UI on a shallow attempt URL + // (`/courses/:c/marketplace/attempt/:id`) that has no /assessments/:aid/submissions + // segment. The attempt id is the submission base-record id, so pollers/buttons that + // derive the id from the URL must resolve it here rather than fall through to null + // and emit /marketplace/attempt/null/... requests. + const previewMatch = window.location.pathname.match( + /^\/courses\/\d+\/marketplace\/attempt\/(\d+)/, + ); + return previewMatch?.[1] ?? null; } /** diff --git a/client/app/routers/course/marketplace.tsx b/client/app/routers/course/marketplace.tsx index a3c54571dc8..acb27fb6711 100644 --- a/client/app/routers/course/marketplace.tsx +++ b/client/app/routers/course/marketplace.tsx @@ -48,6 +48,31 @@ const marketplaceRouter: Translated = () => ({ }, ], }, + { + path: 'attempt/:submissionId', + lazy: async () => ({ + Component: (await import('course/marketplace/pages/PreviewAttempt')) + .default, + }), + children: [ + { + path: 'edit', + lazy: async (): Promise> => { + const SubmissionEditIndex = ( + await import( + /* webpackChunkName: 'SubmissionEditIndex' */ + 'course/assessment/submission/pages/SubmissionEditIndex' + ) + ).default; + + return { + Component: SubmissionEditIndex, + handle: SubmissionEditIndex.handle, + }; + }, + }, + ], + }, ], });