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

Add tests for useQuizResources and update functionality for annotation and loading states #13080

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

GautamBytes
Copy link
Contributor

Summary

This PR adds unit tests for the new useQuizResources module. The module extends the already-tested useFetchTree module, so in these tests the useFetchTree functionality is mocked to return a suitable data structure. The tests verify that:

  • The helper function annotateTopicsWithDescendantCounts correctly annotates topics with their descendant assessment counts (or filters them out if none are present).
  • Both fetchQuizResources and fetchMoreQuizResources functions pass their returned data through the annotation function before updating the resources.
  • The loading state is correctly updated during the fetch operations.

Manual testing was performed by running the full test suite locally, and all tests passed.

References

Addresses #12033

Reviewer guidance

  • Verify that the tests for annotateTopicsWithDescendantCounts cover both successful annotation and error handling.
  • Check that the tests for fetchQuizResources and fetchMoreQuizResources ensure the final resources state matches the expected annotated output.
  • Confirm that the loading states are correctly updated during fetch operations.
  • Note that the useFetchTree module is mocked to isolate the behavior of useQuizResources.

@github-actions github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels Feb 14, 2025
@MisRob MisRob requested a review from nucleogenesis February 17, 2025 04:30
@MisRob MisRob added the TODO: needs review Waiting for review label Feb 17, 2025
Copy link
Member

@nucleogenesis nucleogenesis left a comment

Choose a reason for hiding this comment

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

Thank you for the test suite - the tests look okay to me on my first read.

I wonder if the tests would pass without the changes made to useQuizResources - I'm not seeing the purpose for the changes to the implementation there so clarification there would be appreciated.

Why did you remove all of the JSDoc definitions from useQuizResources? To get this ready for merge, please undo the diff on useQuizResources altogether and see if the tests pass - if they don't, then please revert all changes to comments in the file and only commit the necessary implementation changes to pass the test suite.

Then we can look for some manual testing of the underlying components.

Comment on lines 82 to 92
const api = {
setResources,
resources: computed(() => get(_resources)),
loading: computed(() => get(_loading) || get(treeLoading)),
loadingMore: computed(() => get(_loadingMore)),
hasMore,
topic,
annotateTopicsWithDescendantCounts,
fetchQuizResources: undefined,
fetchMoreQuizResources: undefined,
};
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of defining the api in a separate object here?

@GautamBytes
Copy link
Contributor Author

Hi @nucleogenesis , did the suggested changes . You can have a look and confirm if it meets all requirements or needs further improvements. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants