-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[#11911] Instructor copying course: Progress bar does not load when no feedback sessions are copied #11914
[#11911] Instructor copying course: Progress bar does not load when no feedback sessions are copied #11914
Conversation
3e7b9ad
to
2031462
Compare
src/web/app/pages-instructor/instructor-courses-page/instructor-courses-page.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for responding so quickly to the issue! Could you also add a video of the fix to your description? Here's also some small suggestions
src/web/app/pages-instructor/instructor-courses-page/instructor-courses-page.component.ts
Outdated
Show resolved
Hide resolved
src/web/app/pages-instructor/instructor-courses-page/instructor-courses-page.component.ts
Outdated
Show resolved
Hide resolved
src/web/app/pages-instructor/instructor-courses-page/instructor-courses-page.component.ts
Outdated
Show resolved
Hide resolved
Hello @FergusMok! Thanks so much for reviewing this PR! I have added replies to your comments, and will wait for your reply before pushing out the changes. As for the video, may I ask if there is an example of how the video should be filmed? Is it just how it looks like after fixing or there has to be some explanation/demo? Thank you again! 😃 |
- Update empty copy feedback session list to check against 0 instead of lesser than 1 - Remove progress bar handler
@FergusMok I see! I understand. Have updated based on your suggestions and made the necessary changes for your review please. Thank you again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@domlimm LGTM! Thanks for fixing this issue! :)
Fixes #11911.
Outline of Solution
Previously, when an instructor creates a course by copying another course without any feedback session selected, user gets stuck on the copying and no progress is shown on the progress bar.
The fix is to check if there is any feedback session to copy, else we just need to resolve the
Promise
and this would update the progress bar as well.Additionally, shifted out the
ProgressBarService
update lines as there are code duplications in the above fix and the current implementation (5e50c39).demo.mp4