`Core`: Move phase mailing into settings for application and interview by rappm · Pull Request #1920 · prompt-edu/prompt

rappm · 2026-07-13T19:04:49Z

📝 Pull Request Template

✨ What is the change?

Unifies phase settings by folding the standalone Mailing page into the phase Settings page,
as a section (not a tab) matching the other settings sections. The separate Mailing sidebar item
and its route are removed for the two phases that had them.

Application (core): the mailing settings now render as a Mailing section on the Application
Settings page. The /mailing route and sidebar item are removed; the "missing mailing config"
warning on the landing page now links to /settings.
Interview (component): a new SettingsPage composes the interview question configuration and a
Mailing section. The /mailing route, sidebar item, and the now-orphaned MailingPage are
removed. The question configuration was adjusted to flow as a section (no viewport-fixed height /
sticky header) so both read consistently.

The "rename Config → Settings" part of the issue was already satisfied — every phase sidebar already
uses "Settings" (no phase item was named "Config").

📌 Reason for the change / Link to issue

Phase settings were inconsistent: most phases had a single "Settings" entry, while Application and
Interview split "Mailing" into its own sidebar entry. This consolidates them.

Closes #894

🧪 How to Test

Application: open an Application phase → Settings. Confirm the Mailing section appears
below the other settings, there is no separate Mailing sidebar item, and editing/saving mailing
templates still works. On the landing page, the "no mailing settings" warning links to Settings.
Interview: open an Interview phase → Settings. Confirm both the interview questions and a
Mailing section appear on one page, and there is no separate Mailing sidebar item.

🖼️ Screenshots (if UI changes are included)

Before	After

✅ PR Checklist

Tested locally or on the dev environment
Code is clean, readable, and documented
Tests added or updated (if needed)
Screenshots attached for UI changes (if any)
Documentation updated (if relevant)

coderabbitai · 2026-07-13T19:04:57Z

Warning

Review limit reached

@rappm, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dd914c8a-57e5-4452-a156-44a3ce59c519

📥 Commits

Reviewing files that changed from the base of the PR and between a4c7721 and 2bd031a.

📒 Files selected for processing (11)

clients/core/src/managementConsole/PhaseMapping/ExternalRoutes/ApplicationRoutes.tsx
clients/core/src/managementConsole/PhaseMapping/ExternalSidebars/ApplicationSidebar.tsx
clients/core/src/managementConsole/applicationAdministration/pages/ApplicationLandingPage/hooks/useMissingConfig.ts
clients/core/src/managementConsole/applicationAdministration/pages/ApplicationSettingsPage/ApplicationSettings.tsx
clients/core/src/managementConsole/applicationAdministration/pages/Mailing/ApplicationMailingSettings.tsx
clients/interview_component/routes/index.tsx
clients/interview_component/sidebar/index.tsx
clients/interview_component/src/interview/pages/Mailing/MailingPage.tsx
clients/interview_component/src/interview/pages/Settings/QuestionConfiguration.tsx
clients/interview_component/src/interview/pages/Settings/SettingsPage.tsx
e2e/src/data/permissionMatrix.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch 894-unify-phase-settings-mailing

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

mathildeshagl

Solid, focused consolidation and the route/sidebar cleanup is complete. A few things to address before merge — mainly making the mailing section genuinely match the sibling settings sections and keeping the e2e coverage meaningful.

Card + heading consistency (main one): the neighboring Application sections are <Card>-wrapped with an <h3 text-lg font-semibold> header; the new Mailing section is a bare div with an <h2 text-2xl font-bold>, so it won't visually match despite the PR description's claim.
e2e mailing surface now only asserts the generic settings-page heading, so it no longer proves the mailing section rendered.
Two minor Interview UX changes from the reflow (non-sticky add input, nested inner scroll) — inline.

Non-blocking: the pre-existing duplicate <h2> ("Mailing" vs "Mailing Templates " — note the trailing space) is more visible now; consider demoting the templates heading to h3.

mathildeshagl · 2026-07-15T14:54:20Z

  return (
    <div className='space-y-6'>
-      <ManagementPageHeader>Application Mailing Settings</ManagementPageHeader>
+      <h2 className='text-2xl font-bold'>Mailing</h2>


The sibling sections on this page (ApplicationGeneralSettings, ApplicationSettingsCustomScores) are each wrapped in a <Card>/<CardContent> with an <h3 className="text-lg font-semibold"> header. This section is a bare div with <h2 text-2xl font-bold>, so it reads as a larger, un-carded block rather than "matching the other settings sections." Suggest wrapping in a Card and using the same h3 text-lg font-semibold header for real consistency. (Doing so also fixes the non-sequential h1→h3→h3→h2 order on the page.)

mathildeshagl · 2026-07-15T14:54:20Z

+      <div className='pb-4'>
        <header className='mb-4'>
-          <ManagementPageHeader>Interview Question Configuration</ManagementPageHeader>
+          <h2 className='text-2xl font-bold'>Interview Questions</h2>


Dropping the sticky wrapper means the "Add question" input now scrolls away with the page instead of staying pinned. Fine as a section, but confirm that's intended — it's a UX downgrade from the standalone page.

mathildeshagl · 2026-07-15T14:54:20Z


      {/* Scrollable content */}
-      <div className='grow overflow-auto h-[calc(100vh-300px)] p-4'>
+      <div className='grow overflow-auto max-h-[60vh]'>


max-h-[60vh] overflow-auto creates a nested scroll region inside the also-scrolling SettingsPage. Works, but nested scrollbars can feel awkward with many questions — worth a quick look on a small viewport.

mathildeshagl · 2026-07-15T14:54:20Z

-      heading: 'Application Mailing Settings',
+      // Mailing config moved into the Application settings page.
+      path: (courseId) => `/management/course/${courseId}/${MAILING_PHASE_ID}/settings`,
+      heading: 'Application Settings',


heading: 'Application Settings' is the generic page title, so this row no longer proves the mailing section actually rendered — it now largely overlaps a plain settings-page load. Consider asserting on something mailing-specific (e.g. the Mailing section header or a templates control) so the mailing surface still exercises what it's named for. The API half is unchanged and still meaningful.

Core: Move phase mailing into settings for application and interview

16a757e

github-actions Bot assigned rappm Jul 13, 2026

rappm had a problem deploying to prompt-dev-vm July 13, 2026 19:26 — with GitHub Actions Error

Merge branch 'main' into 894-unify-phase-settings-mailing

461fcdc

rappm had a problem deploying to prompt-dev-vm July 15, 2026 08:08 — with GitHub Actions Error

Testing: Point mailing permission surface at Application settings page

2bd031a

rappm had a problem deploying to prompt-dev-vm July 15, 2026 10:34 — with GitHub Actions Failure

rappm marked this pull request as ready for review July 15, 2026 14:16

rappm requested a review from a team July 15, 2026 14:16

rappm added the schau mi o Translation: Ready to review label Jul 15, 2026

mathildeshagl approved these changes Jul 15, 2026

View reviewed changes

mathildeshagl requested changes Jul 15, 2026

View reviewed changes

rappm requested a deployment to prompt-dev-vm July 15, 2026 16:08 — with GitHub Actions Waiting

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`Core`: Move phase mailing into settings for application and interview#1920

`Core`: Move phase mailing into settings for application and interview#1920
rappm wants to merge 3 commits into
mainfrom
894-unify-phase-settings-mailing

rappm commented Jul 13, 2026

Uh oh!

coderabbitai Bot commented Jul 13, 2026 •

edited

Loading

Review limit reached

Uh oh!

mathildeshagl left a comment

Uh oh!

mathildeshagl Jul 15, 2026

Uh oh!

mathildeshagl Jul 15, 2026

Uh oh!

mathildeshagl Jul 15, 2026

Uh oh!

mathildeshagl Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

rappm commented Jul 13, 2026

📝 Pull Request Template

✨ What is the change?

📌 Reason for the change / Link to issue

🧪 How to Test

🖼️ Screenshots (if UI changes are included)

✅ PR Checklist

Uh oh!

coderabbitai Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

mathildeshagl left a comment

Choose a reason for hiding this comment

Uh oh!

mathildeshagl Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

mathildeshagl Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

mathildeshagl Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

mathildeshagl Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jul 13, 2026 •

edited

Loading