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

[#9382] Migrate remaining of instructor home page/feedback sessions page remind and resend functionals #9422

Merged
merged 36 commits into from Feb 15, 2019

Conversation

jacoblipech
Copy link
Contributor

@jacoblipech jacoblipech commented Feb 10, 2019

Part of #9382 and fixes #8530

remind resend

  • Remind particular students modal to generate students correctly
  • Bind the buttons together for select all and select all yet submitted
  • Fixes logic for remind students function
  • Implement resend students list modal and back end logic
  • Re-work on the back end tests
  • Remove old code

@jacoblipech jacoblipech changed the title Migrate remaining of instructor home page/feedback sessions page remind and resend functionals [#9382] Migrate remaining of instructor home page/feedback sessions page remind and resend functionals Feb 11, 2019
@TEAMMATES TEAMMATES deleted a comment from teammates-bot Feb 11, 2019
@TEAMMATES TEAMMATES deleted a comment from teammates-bot Feb 11, 2019
@xpdavid xpdavid added the s.Ongoing The PR is being worked on by the author(s) label Feb 11, 2019
@jacoblipech jacoblipech force-pushed the instructor-home-page-remind branch 2 times, most recently from 4196b92 to 8cf66a4 Compare February 11, 2019 17:09
@jacoblipech
Copy link
Contributor Author

@wkurniawan07 Ready for review. I am not sure how to pass the E2E test due to the protected variables in the base class? The UI works as shown

@jacoblipech jacoblipech 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 Feb 11, 2019
@wkurniawan07 wkurniawan07 added this to the V7.0.0-beta.0 milestone Feb 13, 2019
@jacoblipech
Copy link
Contributor Author

@wkurniawan07 Made the necessary changes. Ready for Review

@wkurniawan07 wkurniawan07 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 Feb 14, 2019
@wkurniawan07 wkurniawan07 self-requested a review February 14, 2019 03:23
Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

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

Just one minor stuff, ok otherwise.

/**
* The save request of a list of students from feedback session.
*/
public class FeedbackSessionStudentSaveRequest extends BasicRequest {
Copy link
Member

Choose a reason for hiding this comment

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

But this is not a "save" request?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is it better to use remindRequest or submitRequest then?

Copy link
Member

Choose a reason for hiding this comment

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

Which one do you think is better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would go with remindRequest.

The submit buttons for both modals states remind. Given that saveRequest is used for the save button and createRequest too. I think remindRequest will be more adequate.

@jacoblipech
Copy link
Contributor Author

jacoblipech commented Feb 14, 2019

@wkurniawan07 Ready for review. Added a text (previously not in v6) to remind and resend links to view results modal to show that there are no students. UI shown below:
screenshot 2019-02-14 at 9 43 06 pm

@jacoblipech jacoblipech force-pushed the instructor-home-page-remind branch 2 times, most recently from 722667b to 5431d6e Compare February 14, 2019 17:34
@wkurniawan07 wkurniawan07 merged commit 7745139 into TEAMMATES:master Feb 15, 2019
Front-end and RESTful back-end Migration automation moved this from Ongoing to Done Feb 15, 2019
@wkurniawan07 wkurniawan07 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 Feb 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Instructor: remind students: collapse the two options into one
3 participants