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

[#12125] Instructor results page stats tables: differentiate students with same team and name #12249

Merged

Conversation

undermyumbrella1
Copy link
Contributor

@undermyumbrella1 undermyumbrella1 commented Mar 25, 2023

Fixes #12125
My attempt at adding email to statistics table. Not sure if I am supposed to modify the statistics-calculation files and subsequently the response.json test fiiles.
image
image
image
image

The emails was already included in the csv files before this feature
image

@zhaojj2209
Copy link
Contributor

Do fix the failing E2E tests and update your branch with the changes from the master branch. Approach looks alright for now, we will do a more thorough review once the branch has been updated.

In the meantime, it would be good if you can add screenshots showing your fix, so as to help speed up the review process. Do also check if the same changes are present in the downloaded CSV data as well.

@zhaojj2209 zhaojj2209 added the s.ToReview The PR is waiting for review(s) label Mar 25, 2023
@samuelfangjw samuelfangjw added s.Ongoing The PR is being worked on by the author(s) and removed s.ToReview The PR is waiting for review(s) labels Mar 30, 2023
@undermyumbrella1
Copy link
Contributor Author

Hi I have updated the pr to pass the e2e tests, merge main into this branch, and added screenshots in the pull request. May I request for some help reviewing the pr? Thank you!

Copy link
Contributor

@cedricongjh cedricongjh left a comment

Choose a reason for hiding this comment

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

Just a minor nit from my end!

@cedricongjh cedricongjh requested a review from weiquu April 6, 2023 17:42
@samuelfangjw samuelfangjw added s.ToReview The PR is waiting for review(s) and removed s.Ongoing The PR is being worked on by the author(s) labels Apr 6, 2023
Copy link
Contributor

@weiquu weiquu left a comment

Choose a reason for hiding this comment

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

LGTM

@weiquu weiquu requested a review from cedricongjh April 7, 2023 14:27
Copy link
Contributor

@cedricongjh cedricongjh left a comment

Choose a reason for hiding this comment

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

LGTM! thank you for your contribution to TEAMMATES

@samuelfangjw samuelfangjw added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels Apr 7, 2023
@samuelfangjw samuelfangjw added this to the V8.27.0 milestone Apr 7, 2023
@samuelfangjw samuelfangjw merged commit 8990935 into TEAMMATES:master Apr 7, 2023
@samuelfangjw samuelfangjw added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Apr 7, 2023
@undermyumbrella1 undermyumbrella1 deleted the 12125/update-statistic-tables branch May 4, 2023 15:13
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.

Instructor results page stats tables: differentiate students with same team and name
5 participants