Skip to content

fix: useExamAccess hook does not make call to fetchExamAccessToken after isExam changes.#1644

Merged
michaelroytman merged 1 commit intomasterfrom
michaelroytman/COSMO-683-staff-unable-to-view-exams
Mar 20, 2025
Merged

fix: useExamAccess hook does not make call to fetchExamAccessToken after isExam changes.#1644
michaelroytman merged 1 commit intomasterfrom
michaelroytman/COSMO-683-staff-unable-to-view-exams

Conversation

@michaelroytman
Copy link
Copy Markdown
Contributor

@michaelroytman michaelroytman commented Mar 20, 2025

Description

This commit fixes a bug that prevents the fetchExamAccessToken hook in the useExamAccess hook from running. This occurs when the value of isExam returned by the useIsExam hook changes from false to true. Because the dependency array of the useEffect hook within the useExamAccess hook was ['id'], the useEffect hook would not rerun on changes to isExam. The fix is to add isExam to the dependency array.

Jira: COSMO-673 (private)

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.08%. Comparing base (f39a50e) to head (f098b74).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1644   +/-   ##
=======================================
  Coverage   90.08%   90.08%           
=======================================
  Files         338      338           
  Lines        5782     5782           
  Branches     1410     1410           
=======================================
  Hits         5209     5209           
  Misses        556      556           
  Partials       17       17           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@varshamenon4 varshamenon4 left a comment

Choose a reason for hiding this comment

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

Nice! Just one nit/typo!

});
}
}, [id]);
}, [id, isExam]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice find!

const [isExam, setIsExam] = useState(initialState);

// This setTimeout block is intended to replicate the case where a unit is an exam, but
// the call to fetch exam metada has not yet completed. That is the value of isExam starts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this supposed to be metadata?

Suggested change
// the call to fetch exam metada has not yet completed. That is the value of isExam starts
// the call to fetch exam metadata has not yet completed. That is the value of isExam starts

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that! I'll push up a fix.

expect(blockAccess).toBe(true);
expect(mockFetchExamAccessToken).toHaveBeenCalled();

// This is to get rid of the act(...) warning.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

…ter isExam changes

This commit fixes a bug that prevents the fetchExamAccessToken hook in the useExamAccess hook from running. This occurs when the value of isExam returned by the useIsExam hook changes from false to true. Because the dependency array of the useEffect hook within the useExamAccess hook was ['id'], the useEffect hook would not rerun on changes to isExam. The fix is to add isExam to the dependency array.
@michaelroytman michaelroytman force-pushed the michaelroytman/COSMO-683-staff-unable-to-view-exams branch from 4a16065 to f098b74 Compare March 20, 2025 18:56
@michaelroytman michaelroytman changed the title useExamAccess hook does not make call to fetchExamAccessToken after isExam changes. fix: useExamAccess hook does not make call to fetchExamAccessToken after isExam changes. Mar 20, 2025
@michaelroytman michaelroytman merged commit e5f04d9 into master Mar 20, 2025
7 checks passed
@michaelroytman michaelroytman deleted the michaelroytman/COSMO-683-staff-unable-to-view-exams branch March 20, 2025 19:01
KristinAoki pushed a commit that referenced this pull request Mar 25, 2025
…ter isExam changes (#1644)

This commit fixes a bug that prevents the fetchExamAccessToken hook in the useExamAccess hook from running. This occurs when the value of isExam returned by the useIsExam hook changes from false to true. Because the dependency array of the useEffect hook within the useExamAccess hook was ['id'], the useEffect hook would not rerun on changes to isExam. The fix is to add isExam to the dependency array.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants