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

Move handleReplacement logic from ReplaceQuestions.vue to useQuizCreation.js #12099

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

KshitijThareja
Copy link
Contributor

Summary

This PR aims to move the handleReplacements logic from the file ReplaceQuestions.vue to useQuizCreation.js as it was identified as a tech-debt and is better placed in useQuizCreation. The changes are introduced keeping in mind the requirements of the issue and the discussion in PR #11937.

References

Closes #12012

Reviewer guidance

To verify the working of the changes introduced:

  1. Login as an admin/coach.
  2. Create a new quiz.
  3. After importing the questions, try to replace them using the replace option.

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend SIZE: small labels Apr 22, 2024
@nucleogenesis nucleogenesis self-assigned this Apr 22, 2024
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 @KshitijThareja - I have one blocking and relatively minor question. I'll spin this up later on for a test locally as well

Comment on lines +212 to +213
function submitReplacement() {
handleReplacement();
Copy link
Member

Choose a reason for hiding this comment

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

Is there any particular reason to call this from within a function rather than returning handleReplacement from the setup function?

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 believe we needed the router logic to be placed inside the ReplaceQuestions file itself. Returning handleReplacement from the setup function and directly calling it from @submit handler would require the routing logic to be placed in the handleReplacement function that is now moved to useQuizCreation.
That was the main reason for me to call it from within a function as that provides separation of concerns.

Though I'd like to know if I'm missing something here and if there's a better workaround for this.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh thanks for that - I misread the diff 😅 - I thought the only thing happening in submitReplacement was calling handleReplacement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh 😅. No problem :)

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.

Thanks for this!

Comment on lines +212 to +213
function submitReplacement() {
handleReplacement();
Copy link
Member

Choose a reason for hiding this comment

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

Ahh thanks for that - I misread the diff 😅 - I thought the only thing happening in submitReplacement was calling handleReplacement.

@nucleogenesis nucleogenesis merged commit 85d7288 into learningequality:develop Apr 23, 2024
31 checks passed
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 SIZE: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quiz Creation: Move handleReplacements logic from ReplaceQuestions to useQuizCreation
2 participants