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

fix: corrects the navigation issue if the student does not pass the entrance exam #1429

Merged
merged 2 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ const CertificateStatus = ({ intl }) => {
courseId,
} = useSelector(state => state.courseHome);

const {
entranceExamData,
} = useModel('coursewareMeta', courseId);

const {
isEnrolled,
org,
Expand All @@ -42,13 +46,16 @@ const CertificateStatus = ({ intl }) => {
certificateAvailableDate,
} = certificateData || {};

const entranceExamPassed = entranceExamData?.entranceExamPassed ?? null;

const mode = getCourseExitMode(
certificateData,
hasScheduledContent,
isEnrolled,
userHasPassingGrade,
null, // CourseExitPageIsActive
canViewCertificate,
entranceExamPassed,
);

const eventProperties = {
Expand Down
16 changes: 15 additions & 1 deletion src/courseware/course/course-exit/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const COURSE_EXIT_MODES = {
celebration: 1,
nonPassing: 2,
inProgress: 3,
entranceExamFail: 4,
};

// These are taken from the edx-platform `get_cert_data` function found in lms/courseware/views/views.py
Expand All @@ -32,9 +33,14 @@ function getCourseExitMode(
userHasPassingGrade,
courseExitPageIsActive = null,
canImmediatelyViewCertificate = false,
entranceExamPassed = null,
) {
const authenticatedUser = getAuthenticatedUser();

if (entranceExamPassed === false) {
return COURSE_EXIT_MODES.entranceExamFail;
}

if (courseExitPageIsActive === false || !authenticatedUser || !isEnrolled) {
return COURSE_EXIT_MODES.disabled;
}
Expand Down Expand Up @@ -73,6 +79,7 @@ function GetCourseExitNavigation(courseId, intl) {
isEnrolled,
userHasPassingGrade,
courseExitPageIsActive,
entranceExamData: { entranceExamPassed },
} = useModel('coursewareMeta', courseId);
const { canViewCertificate } = useModel('courseHomeMeta', courseId);
const exitMode = getCourseExitMode(
Expand All @@ -82,8 +89,15 @@ function GetCourseExitNavigation(courseId, intl) {
userHasPassingGrade,
courseExitPageIsActive,
canViewCertificate,
entranceExamPassed,
);
const exitActive = exitMode !== COURSE_EXIT_MODES.disabled;

/** exitActive is used to enable/disable the exit i.e. next buttons.
COURSE_EXIT_MODES denote the current status of the course.
Available COURSE_EXIT_MODES: disabled, celebration, nonPassing, inProgress, entranceExamFail
If the user fails the entrance exam,
access to further course sections should not be allowed i.e. disable the next buttons. */
const exitActive = ((exitMode !== COURSE_EXIT_MODES.disabled) && (exitMode !== COURSE_EXIT_MODES.entranceExamFail));
Copy link
Contributor

@bradenmacdonald bradenmacdonald Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you help me understand this? What exactly does exitActive mean and what are the various exit modes? Just want to make sure this logic is correct, but I'm not clear on what's even going on here.

I'd love if you could add a few JSDoc comments to this file as part of this PR to explain what an "exit mode" is and what each of the COURSE_EXIT_MODES mean. And why exitActive should be false when the exit mode is entranceExamFail ??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exitActive is used to enable/disable the exit mode i.e. the next buttons.
Available COURSE_EXIT_MODES: disabled, celebration, nonPassing, inProgress, entranceExamFail
So if the user has failed the entrance exam, access to further course sections should be disabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added some comments in the code as per your suggestion. Let me know if more explanation is needed in the comments and I will add accordingly.


let exitText;
switch (exitMode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,50 @@ describe('Unit Navigation', () => {
expect(screen.getByRole('button', { name: /next/i })).toBeDisabled();
});

it('has the "Next" button disabled for entrance exam failed', async () => {
const testCourseMetadata = {
...courseMetadata,
certificate_data: { cert_status: 'bogus_status' },
enrollment: { is_active: true },
entrance_exam_data: {
entrance_exam_current_score: 0, entrance_exam_enabled: true, entrance_exam_id: '1', entrance_exam_minimum_score_pct: 0.65, entrance_exam_passed: false,
},
};
const testStore = await initializeTestStore({ courseMetadata: testCourseMetadata, unitBlocks }, false);
// Have to refetch the sequenceId since the new store generates new sequences
const { courseware } = testStore.getState();
const testData = { ...mockData, sequenceId: courseware.sequenceId };

render(
<UnitNavigation {...testData} unitId={unitBlocks[0].id} />,
{ store: testStore, wrapWithRouter: true },
);

expect(screen.getByRole('button', { name: /next/i })).toBeDisabled();
});

it('has the "Next" button enabled for entrance exam pass', async () => {
const testCourseMetadata = {
...courseMetadata,
certificate_data: { cert_status: 'bogus_status' },
enrollment: { is_active: true },
entrance_exam_data: {
entrance_exam_current_score: 1.0, entrance_exam_enabled: true, entrance_exam_id: '1', entrance_exam_minimum_score_pct: 0.65, entrance_exam_passed: true,
},
};
const testStore = await initializeTestStore({ courseMetadata: testCourseMetadata, unitBlocks }, false);
// Have to refetch the sequenceId since the new store generates new sequences
const { courseware } = testStore.getState();
const testData = { ...mockData, sequenceId: courseware.sequenceId };

render(
<UnitNavigation {...testData} unitId={unitBlocks[0].id} />,
{ store: testStore, wrapWithRouter: true },
);

expect(screen.getByRole('link', { name: /next/i })).toBeEnabled();
});

it('displays end of course message instead of the "Next" button as needed', async () => {
const testCourseMetadata = { ...courseMetadata, certificate_data: { cert_status: 'notpassing' }, enrollment: { is_active: true } };
const testStore = await initializeTestStore({ courseMetadata: testCourseMetadata, unitBlocks }, false);
Expand Down
11 changes: 11 additions & 0 deletions src/courseware/course/sequence/sequence-navigation/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export function useSequenceNavigationMetadata(currentSequenceId, currentUnitId)
const sequence = useModel('sequences', currentSequenceId);
const courseId = useSelector(state => state.courseware.courseId);
const courseStatus = useSelector(state => state.courseware.courseStatus);
const { entranceExamData: { entranceExamPassed } } = useModel('coursewareMeta', courseId);
const sequenceStatus = useSelector(state => state.courseware.sequenceStatus);

// If we don't know the sequence and unit yet, then assume no.
Expand All @@ -25,6 +26,16 @@ export function useSequenceNavigationMetadata(currentSequenceId, currentUnitId)
};
}

// if entrance exam is not passed then we should treat this as 1st and last unit
if (entranceExamPassed === false) {
return {
isFirstUnit: true,
isLastUnit: true,
navigationDisabledNextSequence: false,
navigationDisabledPrevSequence: false,
};
}

const sequenceIndex = sequenceIds.indexOf(currentSequenceId);
const unitIndex = sequence.unitIds.indexOf(currentUnitId);

Expand Down
Loading