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

EQM: Notify not enough resources to replace questions #12219

Conversation

AlexVelezLl
Copy link
Member

Summary

Adds a modal to notify the user when there are no enough resources to replace the current selected questions

Compartir.pantalla.-.2024-05-28.16_43_28.mp4

References

Closes #12094

Reviewer guidance

  • Fill a section with almost all, or/and all available questions in the pool.
  • Select them all and try to replace them. A modal should appear and inform you that there are no enough questions, and redirect you to add more resources

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

@AlexVelezLl AlexVelezLl added the TODO: needs review Waiting for review label May 28, 2024
@github-actions github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend SIZE: medium labels May 28, 2024
@AlexVelezLl AlexVelezLl force-pushed the notify-not-enough-replacements branch from d9ff9d8 to 7bb7603 Compare May 29, 2024 16:37
@@ -117,19 +117,17 @@ export default function useQuizCreation() {
},
[]
);
if (removedResourceQuestionIds.length === 0) {
// If no resources were removed, we don't need to update the questions
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

This return here has making the whole function to return, and the updates werent applied when we had resource_pool updates.

@nucleogenesis nucleogenesis self-assigned this May 30, 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.

Only one blocking issue noted, otherwise, things look good to me and worked well in testing!

v-if="showNoEnoughResources"
:selectedQuestions="selectedActiveQuestions"
:availableResources="replacementQuestionPool"
@close="showNoEnoughResources = false"
Copy link
Member

Choose a reason for hiding this comment

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

This should also "go back" in the router and close the side panel.

@AlexVelezLl
Copy link
Member Author

Thank you @nucleogenesis! I have updated the close handler!

@AlexVelezLl AlexVelezLl requested a review from nucleogenesis May 30, 2024 22:52
@pcenov
Copy link
Member

pcenov commented May 31, 2024

Hi @AlexVelezLl I confirm that this is implemented as specified in #12094 but while testing I noticed the following:

  1. The 'add' word is missing here:

2024-05-31_11-30-38

  1. Each time the modal pops up I can see the 'Replace questions' sidebar as well, even when there are no questions in it. Wondering whether that could that be avoided as it seems unnecessary?
2024-05-31_11-34-39.mp4

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.

I fixed @pcenov 's comment about the missing word and I think that the other comment is a great point well suited for a follow-up issue - #12229

I'll approve and merge this for now so that I can rebase my open PR on it.

Comment on lines +125 to +131
<NotEnoughResourcesModal
v-if="showNoEnoughResources"
:selectedQuestions="selectedActiveQuestions"
:availableResources="replacementQuestionPool"
@close="closeNoEnoughResourcesModal"
@addResources="redirectToSelectResources"
/>
Copy link
Member

Choose a reason for hiding this comment

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

To @pcenov 's comment about the side panel showing under the modal, I think maybe moving this to the CreateQuizSection component would help. Show the modal before navigating there and should be good to go

@nucleogenesis nucleogenesis merged commit 4fadf48 into learningequality:develop Jun 3, 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: medium TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quiz management: Notify user of replacements vs selection mismatch
3 participants