Skip to content

[#14097] Create Submission Reminders page - #14370

Open
javieryeow wants to merge 23 commits into
TEAMMATES:masterfrom
javieryeow:feat/submission-reminders-page
Open

[#14097] Create Submission Reminders page#14370
javieryeow wants to merge 23 commits into
TEAMMATES:masterfrom
javieryeow:feat/submission-reminders-page

Conversation

@javieryeow

@javieryeow javieryeow commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #14097

Outline of Solution

  • sessions-table still emits the same reminder actions:
    • remind all non-submitters
    • remind selected non-submitters
  • instructor pages still listen to those events through sendRemindersToRespondentsEventHandler
  • shared handler now navigates to /web/instructor/sessions/:feedbackSessionId/send-reminders
  • it passes:
    • preselectNonSubmitters=true for “remind all non-submitters”
    • preselectNonSubmitters=false for “select non-submitters to remind”
    • returnUrl so the page can navigate back after submit/cancel

on the new send-reminders page:

  • preselect non-submitters when requested
  • same selection controls as before
  • submit reminders directly through remindFeedbackSessionSubmissionForRespondents
  • navigate back with the success message preserved

cleanup and test coverage:

  • removed the old send-reminders modal component and model
  • added unit tests for selection behavior on the new page
  • added a dedicated InstructorSessionSendRemindersPageE2ETest and required page object
  • updated existing e2e tests (InstructorHomePage, InstructorFeedbackSession, InstructorFeedbackReport) to use the new send reminders page instead
Screenshot 2026-06-28 at 5 15 05 PM Screenshot 2026-06-28 at 5 15 23 PM

Copilot AI review requested due to automatic review settings June 28, 2026 09:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Introduces a dedicated instructor “Send Reminders” page (replacing the previous modal) to better support large classes, while preserving existing reminder entry points from session-related instructor pages and updating test coverage accordingly.

Changes:

  • Added a new instructor route and standalone page for selecting respondents and sending submission reminders, with optional preselection of non-submitters and return navigation.
  • Refactored existing “send reminders” triggers to navigate to the new page instead of opening a modal / emitting modal results.
  • Removed the old send-reminders modal implementation and updated/added unit + E2E tests and page objects for the new flow.

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/web/app/pages-instructor/instructor.routes.ts Registers the new :feedbackSessionId/send-reminders route.
src/web/app/pages-instructor/instructor-session-send-reminders-page/instructor-session-send-reminders-page.component.ts Implements the new page: data loading, selection logic, reminder submission, safe return URL.
src/web/app/pages-instructor/instructor-session-send-reminders-page/instructor-session-send-reminders-page.component.html New page UI: session details, selection controls, respondent table, actions.
src/web/app/pages-instructor/instructor-session-send-reminders-page/instructor-session-send-reminders-page.component.scss Styles for the new page layout and controls.
src/web/app/pages-instructor/instructor-session-send-reminders-page/instructor-session-send-reminders-page.component.spec.ts Unit tests for selection behavior, toggles, navigation, and error handling.
src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.ts Removes now-obsolete reminder-sending logic previously tied to the modal response.
src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.html Removes event binding previously used to send reminders via the modal workflow.
src/web/app/pages-instructor/instructor-session-result-page/instructor-session-no-response-panel.component.ts Replaces modal opening with navigation to the new send-reminders page.
src/web/app/pages-instructor/instructor-session-result-page/instructor-session-no-response-panel.component.html Updates “Remind All” button to use the new navigation handler.
src/web/app/pages-instructor/instructor-session-result-page/instructor-session-no-response-panel.component.spec.ts Updates/extends tests to assert navigation to the send-reminders page.
src/web/app/pages-instructor/instructor-session-modal-page.component.ts Updates shared reminder handler to navigate to the new page (with preselect + returnUrl).
src/web/app/components/sessions-table/send-reminders-to-respondents-modal/send-reminders-to-respondents-model.ts Deletes old modal response model.
src/web/app/components/sessions-table/send-reminders-to-respondents-modal/send-reminders-to-respondents-modal.component.ts Deletes old modal component.
src/web/app/components/sessions-table/send-reminders-to-respondents-modal/send-reminders-to-respondents-modal.component.html Deletes old modal template.
src/web/app/components/sessions-table/send-reminders-to-respondents-modal/send-reminders-to-respondents-modal.component.scss Deletes old modal styles.
src/web/app/components/sessions-table/send-reminders-to-respondents-modal/send-reminders-to-respondents-modal.component.spec.ts Deletes old modal unit tests.
src/web/app/components/sessions-table/respondent-list-info-table/respondent-list-info-table.component.ts Adds optional table label inputs for reuse on the new page.
src/web/app/components/sessions-table/respondent-list-info-table/respondent-list-info-table.component.html Renders optional student/instructor labels above the tables.
src/main/java/teammates/common/util/Const.java Adds a frontend URI constant for the new send-reminders page.
src/e2e/resources/testng-e2e.xml Includes the new E2E test in the TestNG suite.
src/e2e/resources/data/InstructorSessionSendRemindersPageE2ETest.json Adds data bundle for the new send-reminders page E2E test.
src/e2e/java/teammates/e2e/pageobjects/InstructorSessionSendRemindersPage.java New page object encapsulating interactions/assertions for the send-reminders page.
src/e2e/java/teammates/e2e/pageobjects/InstructorHomePage.java Updates reminder flows to use the new send-reminders page object.
src/e2e/java/teammates/e2e/pageobjects/InstructorFeedbackSessionsPage.java Updates reminder flows to use the new send-reminders page object.
src/e2e/java/teammates/e2e/pageobjects/InstructorFeedbackResultsPage.java Updates reminder flow to use the new send-reminders page object.
src/e2e/java/teammates/e2e/cases/InstructorSessionSendRemindersPageE2ETest.java Adds dedicated E2E test for the new send-reminders page.
src/e2e/java/teammates/e2e/cases/InstructorHomePageE2ETest.java Updates existing E2E assertions to follow the new reminder navigation flow.
src/e2e/java/teammates/e2e/cases/InstructorFeedbackSessionsPageE2ETest.java Updates existing E2E assertions to follow the new reminder navigation flow.
src/e2e/java/teammates/e2e/cases/InstructorFeedbackReportPageE2ETest.java Updates existing E2E assertions to follow the new reminder navigation flow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/e2e/java/teammates/e2e/pageobjects/InstructorSessionSendRemindersPage.java Outdated
@javieryeow

Copy link
Copy Markdown
Contributor Author

@samuelfangjw hi, was investigating the e2e test errors: InstructorHomePageE2ETest: "download results"

BaseE2ETestCase.verifyDownloadedFile is failing as the downloaded file could not be identified, and upon checking the downloads folder, the files are being saved as temporary download files that likely indicate the download is not being finalised (im using chrome so they appear as .com.google.Chrome.xxx). could i check if this is an existing issue or perhaps something to do with my test environment? im testing without my changes and the error seems to be happening as well.

@samuelfangjw

Copy link
Copy Markdown
Member

BaseE2ETestCase.verifyDownloadedFile is failing as the downloaded file could not be identified, and upon checking the downloads folder, the files are being saved as temporary download files that likely indicate the download is not being finalised (im using chrome so they appear as .com.google.Chrome.xxx). could i check if this is an existing issue or perhaps something to do with my test environment? im testing without my changes and the error seems to be happening as well.

Hi @javieryeow, it's a known issue for chrome. Firefox is the only stable one at the moment. As long as it passes on CI it's ok.

Please also help to remove the added InstructorSessionSendRemindersPageE2ETest, updating the existing tests will do.

@javieryeow
javieryeow force-pushed the feat/submission-reminders-page branch from dc7f20e to b4df6bd Compare June 29, 2026 16:23
@javieryeow

Copy link
Copy Markdown
Contributor Author

@samuelfangjw @TobyCyan hi sorry forgot to update: comments have been addressed previously!


.top-padded-sm {
padding-top: 10px;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use bootstrap utility classes where applicable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

bootstrap utility classes have been added, and these sections have been removed from the styles file

WebElement courseTab = getCourseTab(courseTabIndex);
click(courseTab.findElement(By.className("btn-results-" + sessionIndex)));
click(waitForElementPresence(By.className("btn-download-" + sessionIndex)));
click(getVisibleDropdownAction("btn-download-" + sessionIndex));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please revert this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changes reverted

sendRemindersPage.verifyStatusMessage("Reminder e-mails have been sent out to those students"
+ " and instructors. Please allow up to 1 hour for all the notification emails to be sent out.");
homePage = sendRemindersPage.changePageType(InstructorHomePage.class);
homePage.sortCoursesByName();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is a sort needed here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

after we complete the send reminder email action, we navigate back to InstructorHomePage which is a fresh instance, hence it loses its sorted order above. after we navigate back to home page, sortCoursesByName is called so that downstream tests that use courseIndex can still target the correct courses and sessions

/>
<label for="sendCopyToIns" class="form-check-label">Send me a copy</label>
</span>
<button type="button" class="btn btn-light" (click)="cancel()">Cancel</button>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't require a cancel button for a new page.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed cancel button for send reminders page


@Input({ required: true }) feedbackSessionId!: string;
@Input() preselectNonSubmitters = 'false';
@Input() returnUrl = '';

@TobyCyan TobyCyan Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't need returnUrl

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed returnUrl

@javieryeow
javieryeow requested a review from TobyCyan July 21, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create Submission Reminders page

4 participants