[#9255] Migrate Instructor Course Student Details Edit page to Angular #9301

tanhengyeow · 2019-01-03T18:46:09Z

Part of #9255

Summary of PR:

Migrated InstructorCourseStudentDetailsEditSaveAction to PutCourseStudentDetailsEditAction
- Endpoint: /webapi/courses/students/details/edit
- Requires: 2 non-null query parameters courseid and studentemail to check for current student to edit. Other query parameters are sent through the form and fields are updated accordingly.
- Returns: any error message if the action fails. If the action is successful, user will be routed to the instructor course details page.
- Added PutCourseStudentDetailsEditActionTest to test for the new action.

Added new action GetStudentEditDetailsAction to fetch required data for the student details edit page. In v6, relevant data is generated at the backend and mapped to the template page (via JSP) to be served to the users. In v7, where both front-end and back-end are separate, a new endpoint is needed to fetch the required data for this page.
- Endpoint: /webapi/courses/students/editDetails
- Requires: 2 non-null query parameters courseid and studentemail to check for current student details to fetch.
- Added GetStudentEditDetailsActionTest to test for the new action.

Migrated front-end of this page instructor-course-student-edit-page.*
- Uses reactive form template to generate the form
- Uses updated bootstrap styles that matches the old look of v6
- Included 2 subscriptions that listen for form value changes of the fields Team name and Email
  - Depending on what the user changed, logic for the respective modal boxes is handled in instructor-course-student-edit-page.component.ts

InstructorCourseStudentDetailsEditPageAction will not be migrated into a new action as its' purpose is to show the edit page.

To delete (once PR is reviewed):

Deprecated routes in src/main/java/teammates/ui/controller/ActionFactory.java
InstructorCourseStudentDetailsEdit*.java files (except browsertests and pageobjects related files)
WEB-INF/tags/instructor/courseStudentDetailsEdit/studentInformationTable.tag
webapp-backup/jsp/instructorCourseStudentEdit.jsp
Deprecated JSP paths in Const.java
webapp-backup/dev/js/main/instructorCourseStudentEdit.js

Points to note:

In v6, after successful edit of a student, the user is routed to the instructor course details page and a status message is shown, e.g.:
In v7, this message has to be passed along to the instructor course details page once it is migrated.

wkurniawan07 · 2019-01-04T06:22:35Z

@tanhengyeow see if you can build the front-end on top of instructor course student details page instead (already migrated thanks to @tran-tien-dat's good work).

tanhengyeow · 2019-01-04T12:43:19Z

@wkurniawan07 AFAIK the current instructor course student details page is just a static page showing the student details with no functionality for edits.

The different user flow for instructor course student details edit is as follow:

instructorCoursesPage -> Click “View” button of any table row entry -> instructorCourseDetailsPage -> Click “Edit” -> instructorCourseStudentDetailsEdit (which shows a separate edit page)
instructorStudentListPage -> Expand Course -> Click “Edit” button of any table row entry -> instructorCourseStudentDetailsEdit

Are you suggesting towards the idea of not having a separate edit page but instead allow edits on instructor course student details page? If that's the case, we need to take note to discontinue the "Edit" button for the current user flow I've mentioned above.

wkurniawan07 · 2019-01-09T16:50:50Z

Are you suggesting towards the idea of not having a separate edit page but instead allow edits on instructor course student details page? If that's the case, we need to take note to discontinue the "Edit" button for the current user flow I've mentioned above.

Is it too hard to do? The way I see it, it would in fact make your life a lot easier, but I could be wrong on it.

Or alternatively you could just migrate it as is and we can take a look later on whether to merge it.

tanhengyeow · 2019-01-12T12:49:49Z

@wkurniawan07 Ready for review. PR description is updated with the summary of the changes.

wkurniawan07

Mostly good work! Some changes requested.
Also, you should remove the action classes, tests, and JS/JSP/tag files that have been migrated.

src/main/java/teammates/ui/newcontroller/GetStudentEditDetailsAction.java

wkurniawan07 · 2019-01-12T14:29:04Z

...tructor/instructor-course-student-edit-page/instructor-course-student-edit-page.component.ts

+
+  /**
+   * Initializes the student details edit form with the fields fetched from the backend.
+   * Subscriptions are set up to listen to changes in the 'teamname' fields and 'newstudentemail' fields.


...tructor/instructor-course-student-edit-page/instructor-course-student-edit-page.component.ts

wkurniawan07 · 2019-01-12T14:34:10Z

...tructor/instructor-course-student-edit-page/instructor-course-student-edit-page.component.ts

+  /**
+   * Sets the boolean value of `isSessionSummarySendEmail` to true if
+   * user chooses to resend past session link to the new email.
+   * Submits the form otherwise.


This is not accurate? It submits the form regardless of the value.

@wkurniawan07 This is intended. The default value of isSessionSummarySendEmail is declared false initially.

In the event the user clicks No, just save the changes, the form will still be submitted with the boolean being false. If the user clicks Yes, save changes and resend links, the form will be submitted with the boolean being true.

The point remains that whatever the value is, calling this function will result in the form being submitted?

@wkurniawan07 Yup! That's right. When the user reaches this modal, pressing any of the primary-coloured buttons will lead to the form being submitted, which is what this function is doing.

Btw, ready for latest review :)

So "Submits the form otherwise." is not accurate because it submits the form always.
Not a big deal since it's just a comment and can be addressed later.

…tSave

wkurniawan07 · 2019-01-13T14:12:30Z

@tanhengyeow let's not remove the browser tests yet. We still need them for reference when migrating the E2E tests later on.

tanhengyeow · 2019-01-13T15:00:16Z

@wkurniawan07 Ready for review again 😄

wkurniawan07

Tested, mostly there! Few minor changes requested.

wkurniawan07 · 2019-01-13T15:10:03Z

...tructor/instructor-course-student-edit-page/instructor-course-student-edit-page.component.ts

+          this.router.navigate(['../../', 'details'],
+              { relativeTo: this.route },
+          );
+          // TODO: Show success status message in courses/details page after successful edit


You can use NavigationService to achieve this.

wkurniawan07 · 2019-01-13T15:18:13Z

...tructor/instructor-course-student-edit-page/instructor-course-student-edit-page.component.ts

+  /**
+   * Sets the boolean value of `isSessionSummarySendEmail` to true if
+   * user chooses to resend past session link to the new email.
+   * Submits the form otherwise.


The point remains that whatever the value is, calling this function will result in the form being submitted?

wkurniawan07 · 2019-01-13T15:26:10Z

...uctor/instructor-course-student-edit-page/instructor-course-student-edit-page.component.html

+                (click)="modal.dismiss()">×</button>
+          </div>
+          <div class="modal-body">
+            Do you want to resend past session links of this course to the new email {{ editForm.get('newstudentemail') }}?


editForm.get('newstudentemail') results in [object Object]. Need further processing.

wkurniawan07

Ok, this page is clear to go.

#9301) * Setup new Action that would replace instructorCourseStudentDetailsEditSave * Changed courseId and studentEmail to use getNonNullRequestParamValue * Added method verifyInaccessibleWithoutModifyStudentPrivilege for test * Added test for PutCourseStudentDetailsEditAction * Initial setup of edit form structure * Add new action to fetch student edit page details * Added test for GetStudentEditDetailsAction * Finish up page migration * Added relevant imports for spec file and fix lint errors * Dlt deprecated inst course student edit files except E2E related files * Fix code according to review * Use updated test method * Fix logic bug * Updated success message and use NavigationService

tanhengyeow added the s.Ongoing The PR is being worked on by the author(s) label Jan 3, 2019

tanhengyeow force-pushed the 9255-inst-course-student-details-edit branch from 0ebd603 to 7060a1b Compare January 6, 2019 19:47

wkurniawan07 self-requested a review January 9, 2019 16:51

wkurniawan07 self-assigned this Jan 9, 2019

tanhengyeow force-pushed the 9255-inst-course-student-details-edit branch 3 times, most recently from 8a04183 to ceb9e1d Compare January 12, 2019 11:39

tanhengyeow 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 Jan 12, 2019

wkurniawan07 requested changes Jan 12, 2019

View reviewed changes

wkurniawan07 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 Jan 12, 2019

tanhengyeow added 8 commits January 13, 2019 21:15

Setup new Action that would replace instructorCourseStudentDetailsEdi…

b235ca0

…tSave

Changed courseId and studentEmail to use getNonNullRequestParamValue

3e20dd4

Added method verifyInaccessibleWithoutModifyStudentPrivilege for test

084ab10

Added test for PutCourseStudentDetailsEditAction

a4f078e

Initial setup of edit form structure

470400f

Add new action to fetch student edit page details

cd6523e

Added test for GetStudentEditDetailsAction

3b90552

Finish up page migration

8969bb2

tanhengyeow force-pushed the 9255-inst-course-student-details-edit branch from ceb9e1d to 0fd8d3d Compare January 13, 2019 13:21

Added relevant imports for spec file and fix lint errors

caa0ca2

tanhengyeow force-pushed the 9255-inst-course-student-details-edit branch 2 times, most recently from 4e9519c to 03cc79b Compare January 13, 2019 13:51

Dlt deprecated inst course student edit files except E2E related files

2f81f59

tanhengyeow force-pushed the 9255-inst-course-student-details-edit branch 2 times, most recently from f12f784 to 2885d44 Compare January 13, 2019 14:30

tanhengyeow added 2 commits January 13, 2019 22:47

Fix code according to review

a1915d5

Use updated test method

e9317f7

tanhengyeow force-pushed the 9255-inst-course-student-details-edit branch from 2885d44 to e9317f7 Compare January 13, 2019 14:49

tanhengyeow 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 Jan 13, 2019

wkurniawan07 requested changes Jan 13, 2019

View reviewed changes

wkurniawan07 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 Jan 13, 2019

tanhengyeow added 2 commits January 14, 2019 00:12

Fix logic bug

458ac22

Updated success message and use NavigationService

b1c58f8

tanhengyeow force-pushed the 9255-inst-course-student-details-edit branch from 412f49a to b1c58f8 Compare January 13, 2019 16:12

tanhengyeow 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 Jan 13, 2019

wkurniawan07 approved these changes Jan 13, 2019

View reviewed changes

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 Jan 13, 2019

wkurniawan07 merged commit bf91057 into TEAMMATES:teammatesv7 Jan 13, 2019

wkurniawan07 added this to the V7.0.0-alpha.0 milestone Jan 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#9255] Migrate Instructor Course Student Details Edit page to Angular #9301

[#9255] Migrate Instructor Course Student Details Edit page to Angular #9301

tanhengyeow commented Jan 3, 2019 •

edited

wkurniawan07 commented Jan 4, 2019

tanhengyeow commented Jan 4, 2019

wkurniawan07 commented Jan 9, 2019

tanhengyeow commented Jan 12, 2019

wkurniawan07 left a comment

wkurniawan07 Jan 12, 2019

wkurniawan07 Jan 12, 2019

tanhengyeow Jan 13, 2019

wkurniawan07 Jan 13, 2019

tanhengyeow Jan 13, 2019

wkurniawan07 Jan 13, 2019

wkurniawan07 commented Jan 13, 2019

tanhengyeow commented Jan 13, 2019

wkurniawan07 left a comment

wkurniawan07 Jan 13, 2019

wkurniawan07 Jan 13, 2019

wkurniawan07 Jan 13, 2019

wkurniawan07 left a comment

[#9255] Migrate Instructor Course Student Details Edit page to Angular #9301

[#9255] Migrate Instructor Course Student Details Edit page to Angular #9301

Conversation

tanhengyeow commented Jan 3, 2019 • edited

wkurniawan07 commented Jan 4, 2019

tanhengyeow commented Jan 4, 2019

wkurniawan07 commented Jan 9, 2019

tanhengyeow commented Jan 12, 2019

wkurniawan07 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wkurniawan07 commented Jan 13, 2019

tanhengyeow commented Jan 13, 2019

wkurniawan07 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wkurniawan07 left a comment

Choose a reason for hiding this comment

tanhengyeow commented Jan 3, 2019 •

edited