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

coach routing - handle lesson creation and deletion #11283

Conversation

thanksameeelian
Copy link
Contributor

@thanksameeelian thanksameeelian commented Sep 21, 2023

Summary

this PR addresses:

  • lesson creation bug caused by an over-application of making classId optional - it should only be optional in the base Coach routes but other URLs were also initially (& unintentionally) effected
  • redirect bug after lesson deletion, which surfaced after the above bug was addressed
  • appearance of perpetual loading bar in Coach > Plan > Quiz > [resource selection] > Quiz Preview (final step before a new quiz is saved)

References


Quiz preview before (no throttling)
note loading bar running perpetually while on quiz preview page, but very quickly resolved on other pages

loading-quiz-preview.mov

Quiz preview after (throttled to Slow 3G)
note loading bar runs during page transitions (including when transitioning from Quiz preview to the Quiz page upon saving)

loading-bar-and-spinner.mov

Reviewer guidance

📌 the existence of more than one class in a facility (or more than one facility assigned to the user) means that URLs for page links & buttons are constructed a bit differently behind the scenes and user-facing behavior differs.

behind the scenes, the facility_id and classId potentially needed for a URL are determined and applied differently in the different cases. UX-wise, a single facility user with one class will go directly to that class's details when clicking on a sidenav Coach subtopic, while a multi-facility user will first land on the "All Facilities" page and, if they select a facility with multiple classes, will next land on that facility's "All Classes" page, and information about the initially-selected subtopic should persist and ultimately be automatically navigated to only upon making that class selection.

with these differences in mind, please consider the 4 potential cases when testing: multi-facility user in facility with 1 class, multi-facility user in facility with 2+ classes, single facility user with 1 class, and single facility user with 2+ classes. 📌

Lesson creation & deletion

  • check for successful navigation through Coach > Plan > New Lesson > [enter details] > Save changes to the new lesson's Lesson Summary page
  • check for successful lesson deletion from a lesson's Lesson Summary page, Options > Delete and automatic navigation to the containing class's Plan > Lessons page

Quiz Preview loading state

  • following Coach > Plan > Quizzes > New Quiz > Create Quiz > [select resources] > Continue, check "Preview Quiz" page top bar to confirm it is no longer constant loading
  • if you're throttling the connection or have a slow connection already, loading indicator should be present within the nested questions component during initial loading and/or if you choose to randomize the questions via the on-screen button.

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

@thanksameeelian thanksameeelian added the work-in-progress Not ready for review label Sep 21, 2023
@github-actions github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) SIZE: small labels Sep 21, 2023
@thanksameeelian thanksameeelian force-pushed the fix-broken-lesson-creation branch from a4d6c5b to 0abd546 Compare September 22, 2023 16:18
@thanksameeelian thanksameeelian added TODO: needs review Waiting for review and removed work-in-progress Not ready for review labels Sep 22, 2023
@thanksameeelian thanksameeelian marked this pull request as ready for review September 22, 2023 17:15
@thanksameeelian thanksameeelian changed the title [WIP] coach routing - handle lesson creation and deletion coach routing - handle lesson creation and deletion Sep 22, 2023
Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

These look like the correct code changes to me. Will merge when manual QA confirms the fix.

@radinamatic
Copy link
Member

Tested all possible permutations of the scenarios for both superadmin-as-coach (1 and 2 classes in a single and multi-facility), and and a native coach user (1 and 2 classes in a single facility): navigation for lesson creation & deletion works as expected, ending in Lesson Summary and class's Plan > Lessons page, respectively 👏🏽

When creating a quiz, slow 3G throttling properly displays the loader within the question component on the Preview quiz page, no loader present below the top bar 👍🏽

Tested in Chrome/Chromium and Firefox on Ubuntu 20.04

Copy link
Member

@radinamatic radinamatic left a comment

Choose a reason for hiding this comment

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

Manual QA passes, good to go! 💯 🎉 :shipit:

@rtibbles rtibbles merged commit 7b61c25 into learningequality:release-v0.16.x Sep 25, 2023
34 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.) SIZE: small TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken lesson creation
3 participants