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

[#11030] Batch download for session-table component #11066

Conversation

teikjun
Copy link
Member

@teikjun teikjun commented Mar 26, 2021

Fixes #11030

Outline of Solution
In instructor-session-base-page.component.ts:

  1. Fetch feedbackQuestionId for the relevant session on demand (i.e. when user clicks the "Download Results" button of the corresponding row in the session-table component)
  2. Break down the download in batches partitioned by feedbackQuestionId.
  3. Display download progress using the loading modal

The changes to the UI can be viewed in an instructor's "Home" tab and "Session" tab - these are the only remaining pages that contain the "Download Results" button. The "Download Results" button on these two pages now have the same behavior as that of instructor-session-result-page.

108107680-86128f00-70ca-11eb-8f96-ffba0a988bb8

@teikjun teikjun added the s.ToReview The PR is waiting for review(s) label Mar 26, 2021
Copy link
Contributor

@daongochieu2810 daongochieu2810 left a comment

Choose a reason for hiding this comment

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

The code overall looks good to me. Just some minor comments

@teikjun
Copy link
Member Author

teikjun commented Mar 27, 2021

Thanks for reviewing! I have made the suggested changes except for the ones that are still marked as unresolved.

Copy link
Contributor

@daongochieu2810 daongochieu2810 left a comment

Choose a reason for hiding this comment

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

LGTM

@teikjun teikjun added s.FinalReview The PR is ready for final review and removed s.ToReview The PR is waiting for review(s) labels Mar 28, 2021
Copy link
Contributor

@hcwong hcwong left a comment

Choose a reason for hiding this comment

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

Sflr! The changes look fine :)

@teikjun teikjun added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.FinalReview The PR is ready for final review labels Apr 6, 2021
@madanalogy madanalogy added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Apr 12, 2021
@madanalogy madanalogy added this to the V7.15.0 milestone Apr 12, 2021
@madanalogy madanalogy merged commit 837f1d1 into TEAMMATES:master Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Feature User-facing feature; can be new feature or enhancement to existing feature s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modification to "Download Results" button in session table component
5 participants