Skip to content

feat: implement core modules for homework, timetable, payments, and staff routing with new frontend dashboard pages and initial database migrations - #10

Merged
Kuzuri247 merged 2 commits into
mainfrom
test
Aug 9, 2026
Merged

feat: implement core modules for homework, timetable, payments, and staff routing with new frontend dashboard pages and initial database migrations#10
Kuzuri247 merged 2 commits into
mainfrom
test

Conversation

@Kuzuri247

@Kuzuri247 Kuzuri247 commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Added comprehensive timetable management for teachers and administrators, including class schedules, personal teaching loads, editing, deletion, conflict detection, and access controls.
    • Students can view their class timetables with improved session and schedule details.
    • Added subject and teacher selection options for timetable management.
    • Added password visibility toggle on the login page.
  • Bug Fixes

    • Prevented unauthorized homework deletions.
    • Strengthened payment verification permissions.
    • Improved timetable validation, authorization, and error handling.
  • Style

    • Refreshed landing page navigation, hero layout, spacing, and branding presentation.

…taff routing with new frontend dashboard pages and initial database migrations
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
thoms-school-erp Ready Ready Preview Aug 9, 2026 2:32pm

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

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

Next review available in: 44 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

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 Plus

Run ID: fe85508a-b6aa-43a8-8d7a-9fe192157079

📥 Commits

Reviewing files that changed from the base of the PR and between 0e1031f and 03e724c.

📒 Files selected for processing (10)
  • backend/migrations/010_timetable_breaks.sql
  • backend/modules/homework/homework.routes.js
  • backend/modules/homework/homework.service.js
  • backend/modules/payments/payments.route.js
  • backend/modules/payments/payments.service.js
  • backend/modules/timetable/timetable.routes.js
  • backend/scratch/verify_timetable.js
  • frontend/src/pages/Login.jsx
  • frontend/src/pages/SchoolLandingPage.jsx
  • frontend/src/pages/TeacherDashboard.jsx
📝 Walkthrough

Walkthrough

The pull request expands timetable routing and management across the backend and frontend, adds timetable verification coverage, tightens selected authorization checks, removes the authentication router, and updates login, landing-page, and application formatting.

Changes

Timetable management

Layer / File(s) Summary
Routing and timetable retrieval
backend/index.js, backend/migrations/010_timetable_breaks.sql, backend/modules/timetable/timetable.routes.js, frontend/src/pages/StudentDashboard.jsx
The timetable router is mounted at both API prefixes. Timetable retrieval now supports students, sections, assigned classes, teacher schedules, subjects, and teachers. subject_id now allows NULL values for breaks.
Timetable mutation and validation
backend/modules/timetable/timetable.routes.js
Timetable upsert and deletion now normalize day and session values, handle breaks, enforce class-teacher permissions, and detect teacher conflicts.
Teacher dashboard and verification
backend/modules/timetable/timetable.routes.js, backend/scratch/verify_timetable.js, frontend/src/pages/TeacherDashboard.jsx
The teacher dashboard supports class schedules, personal teaching loads, permission-aware editing, standard periods, filtering, upsert, and deletion. The verification script exercises timetable access and mutation paths.

Authorization and route changes

Layer / File(s) Summary
Authorization checks
backend/modules/homework/homework.routes.js, backend/modules/payments/payments.route.js, backend/modules/staff/teacherAssignment.routes.js, backend/routes/authRoutes.js
Homework deletion validates teacher ownership. Payment verification requires canCreateOrder. The teacher-assignment route removes a duplicate authorization argument. The authentication router and its listed routes were removed.

Frontend presentation

Layer / File(s) Summary
Login and landing page updates
frontend/src/pages/Login.jsx, frontend/src/pages/SchoolLandingPage.jsx
Login adds password visibility toggling. The landing page updates navigation, hero layout, action icons, spacing, and footer markup.
Application formatting
frontend/src/App.jsx
Imports, access-denied markup, and protected route declarations were reformatted without behavioral changes.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested reviewers: singhnavneet470

Sequence Diagram(s)

sequenceDiagram
  participant Teacher
  participant TeacherDashboard
  participant TimetableRoutes
  participant Database
  Teacher->>TeacherDashboard: Select class and edit timetable
  TeacherDashboard->>TimetableRoutes: POST /upsert
  TimetableRoutes->>Database: Validate permissions and teacher conflicts
  TimetableRoutes->>Database: Insert or update timetable slot
  Database-->>TimetableRoutes: Persisted timetable data
  TimetableRoutes-->>TeacherDashboard: Return updated schedule
  TeacherDashboard-->>Teacher: Refresh timetable view
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the pull request’s main backend modules, frontend dashboard work, and database migrations, although it is broad.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 19

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (5)
frontend/src/pages/Login.jsx-168-178 (1)

168-178: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Give the password toggle an accessible name.

The button contains only an icon. Add a state-dependent aria-label and aria-pressed value so assistive technology can identify the action and its current state. (w3.org)

Proposed fix
                 <button
                   type="button"
+                  aria-label={showPassword ? "Hide password" : "Show password"}
+                  aria-pressed={showPassword}
                   onClick={() => setShowPassword(!showPassword)}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/pages/Login.jsx` around lines 168 - 178, Update the password
toggle button in Login.jsx to include a state-dependent aria-label describing
whether it will show or hide the password, and set aria-pressed from
showPassword so assistive technology exposes the current toggle state.

Source: MCP tools

frontend/src/pages/SchoolLandingPage.jsx-97-101 (1)

97-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add group to the sign-in Link.

LogIn uses group-hover:translate-x-0.5, so the parent link needs the group class for the hover effect to apply.

Proposed fix
               <Link
                 to="/login"
-                className="px-5 py-2.5 bg-gradient-to-r from-indigo-600 to-violet-600 hover:from-indigo-500 hover:to-violet-500 text-white font-extrabold text-xs rounded-xl shadow-lg shadow-indigo-600/30 transition flex items-center gap-2"
+                className="group px-5 py-2.5 bg-gradient-to-r from-indigo-600 to-violet-600 hover:from-indigo-500 hover:to-violet-500 text-white font-extrabold text-xs rounded-xl shadow-lg shadow-indigo-600/30 transition flex items-center gap-2"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/pages/SchoolLandingPage.jsx` around lines 97 - 101, Add the
`group` class to the sign-in `Link` containing the `LogIn` icon, preserving its
existing classes so the icon’s `group-hover:translate-x-0.5` behavior activates
on link hover.

Source: MCP tools

frontend/src/pages/TeacherDashboard.jsx-1189-1201 (1)

1189-1201: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Two different states render the same label.

Line 1190 renders "Free Period" when item.is_break is true. Line 1198 renders "Free Period" when no slot exists. The user cannot distinguish an explicitly marked free period from an unscheduled slot. The recess banner at line 1240 uses the term "Recess Break" for a third concept.

Use distinct labels, for example "Free Period" for is_break and "Not Scheduled" for a missing slot.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/pages/TeacherDashboard.jsx` around lines 1189 - 1201,
Differentiate the labels for explicit breaks and missing schedule slots in the
rendering branch around item.is_break: keep “Free Period” for break items, but
change the null-item fallback to “Not Scheduled.” Leave the subject label and
existing styling unchanged.
frontend/src/pages/TeacherDashboard.jsx-334-339 (1)

334-339: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

The success message is never visible.

Line 335 sets ttMessage. Line 336 closes the modal. ttMessage renders only inside the modal at lines 1324-1328. The user therefore receives no confirmation after a successful save.

Render the confirmation outside the modal, or clear ttMessage and use the existing page-level message state.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/pages/TeacherDashboard.jsx` around lines 334 - 339, Update the
success handling in the timetable save flow around the success branch and its
ttMessage rendering so “Slot saved successfully!” remains visible after
setShowTtModal(false). Render the confirmation outside the modal, or reuse the
existing page-level message state and rendering, while preserving the current
modal-close and refresh behavior.
backend/scratch/verify_timetable.js-22-47 (1)

22-47: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Guard the fixture queries against empty result sets.

Line 38 reads ct.teacher_user_id. If no row has is_class_teacher = 1, ct is undefined and the script throws a TypeError. The catch block then prints HTTP VERIFICATION FAILED with a null-reference message that hides the real cause, which is missing seed data. The same applies to st at line 46 and student at line 47.

Check each fixture and report a clear message.

🐛 Proposed fix
       const ct = ctRows[0];
+      if (!ct) {
+        throw new Error('No class teacher assignment found. Seed teacher_assignments before you run this script.');
+      }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/scratch/verify_timetable.js` around lines 22 - 47, Guard the fixture
results in the verification flow before dereferencing them: validate ct after
the class-teacher query, st after the substitute-teacher query, and student
after the student query. If any result is missing, report a clear
missing-seed-data message and stop before calling generateToken, while
preserving the existing success path when all fixtures exist.
🧹 Nitpick comments (3)
backend/modules/timetable/timetable.routes.js (2)

502-512: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

The UNION subquery scans the whole timetables table.

The second branch reads every row in timetables with no session filter and no teacher filter. The table grows with each academic session, so this query cost grows without bound while the result set stays small. Add a current-session filter to the timetable branch.

⚡ Proposed fix
        UNION
        SELECT DISTINCT t.teacher_user_id, t.subject_id, s.name AS subject_name
        FROM timetables t
        JOIN subjects s ON t.subject_id = s.id
-       WHERE t.subject_id IS NOT NULL`
+       WHERE t.subject_id IS NOT NULL
+         AND (t.session_id IS NULL OR t.session_id = (SELECT id FROM academic_sessions WHERE is_current = 1 LIMIT 1))`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/modules/timetable/timetable.routes.js` around lines 502 - 512, Update
the assignments query in the timetable route so the second UNION branch against
timetables filters rows to the current academic session using the existing
session context or symbol, while retaining the subject and teacher assignment
selection behavior.

163-184: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

classId is not validated against the section.

The route accepts classId, but the authorization check and the timetable query both use only sectionId. A caller can pass any classId with a valid sectionId and receive data. Either verify that sectionId belongs to classId, or remove classId from the path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/modules/timetable/timetable.routes.js` around lines 163 - 184, The
route handler for GET /class/:classId/section/:sectionId must validate that the
requested section belongs to classId before authorization and timetable
retrieval, using the existing database access flow and returning an appropriate
not-found or access-denied response on mismatch; alternatively remove classId
consistently from the route and handler. Ensure all downstream logic uses the
validated relationship.
frontend/src/pages/TeacherDashboard.jsx (1)

39-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

STANDARD_PERIOD_TIMES duplicates the backend defaults.

DEFAULT_PERIOD_TIMES in backend/modules/timetable/timetable.routes.js lines 295-303 defines the same seven period times with a :00 seconds suffix. The two definitions can drift. The UI at line 1205 renders the frontend copy while the backend persists its own copy, so a change in one place produces times that display differently from the stored values.

Serve the period times from the backend, or extract them into one shared constant.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/pages/TeacherDashboard.jsx` around lines 39 - 48, Remove the
duplicated STANDARD_PERIOD_TIMES definition and use the backend-provided
DEFAULT_PERIOD_TIMES through the existing data flow, or move both consumers to
one shared constant. Update the TeacherDashboard rendering logic to consume that
single source of truth while preserving the seven period mappings and their
stored time format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/migrations/010_timetable_breaks.sql`:
- Around line 1-2: Add an explicit is_break column definition to the
010_timetable_breaks.sql migration using ALTER TABLE timetables, with a type and
nullability/default matching the timetable router’s reads and writes. Keep the
existing subject_id alteration intact so fresh databases create both required
columns.

In `@backend/modules/homework/homework.routes.js`:
- Around line 141-146: Update the teacher deletion flow in the homework route
and its service delete operation so ownership validation is atomic with
deletion: pass the authenticated teacher ID through, include assigned_by in the
DELETE predicate, and treat zero affected rows as unauthorized. Preserve
existing behavior for non-teacher deletions and successful deletes.

In `@backend/modules/payments/payments.route.js`:
- Line 36: Update the /verify handler and svc.verifyPayment flow to pass the
authenticated request user through, fetch the Razorpay order’s fee_record_id and
student_id, and reject signature verification when a student caller does not own
that order; retain existing behavior for authorized non-student roles and only
update fees after ownership validation.

In `@backend/modules/timetable/timetable.routes.js`:
- Line 465: Update the GET /subjects and GET /teachers routes to require the
existing authorize middleware with the class-teacher and administrator roles,
not only verifyToken. In the teachers response mapping, remove u.email from the
returned payload unless it is required for editor name disambiguation.
- Around line 422-426: In backend/modules/timetable/timetable.routes.js lines
422-426, extract the DELETE /period/:periodId implementation into a named
deleteTimetablePeriod handler and register it directly on both /period/:periodId
and /:id with the existing middleware. In
backend/modules/timetable/timetable.routes.js lines 75-79, extract the GET
/my-class implementation into getStudentTimetable and register that handler on
both /my-class and /student/my-timetable; remove request mutation and
router.handle redispatching at both sites.
- Around line 305-354: Update the timetable handler’s period-processing flow to
acquire a pooled connection, validate every period and teacher clash before any
writes, then execute the upserts within an explicit transaction on that same
connection. Replace the direct 409 return from the clash path with a typed
conflict error, roll back on any validation or write failure, and send the 409
only after rollback; commit only after all periods succeed to prevent partial
updates and concurrent check-then-act double-bookings.
- Around line 221-229: Update the /section/:sectionId handler to enforce the
same teacher assignment check used by the main section route, in addition to the
existing student ownership validation. When req.user.role is ROLES.TEACHER,
verify the teacher is assigned to sectionId and return the established forbidden
response when not assigned; preserve the current student and admin access
behavior.
- Around line 362-384: Add validation and a try/catch around the POST / handler
body, validating required start_time, end_time, and period_no before any
database query and returning a 400 JSON response with the existing { success:
false, message } contract for invalid input. Catch database or other runtime
errors in this handler and return the established JSON error response instead of
allowing rejected promises to reach Express’s default error handler; keep the
parameterized SQL unchanged.
- Around line 291-315: In the timetable handler, remove the `sessionRow?.id ||
1` fallback and return an appropriate 500 or 409 response when no current
academic session exists before inserting rows. In the `for (const p of periods)`
loop, validate `periodNo` immediately after parsing it and skip or reject values
outside 1–7 before using `DEFAULT_PERIOD_TIMES[periodNo]` or persisting the row.
- Around line 9-24: Remove the duplicate numeric keys from DAY_MAP, retaining
the string keys used by parseDayOfWeek. Update parseDayOfWeek to return null for
unrecognized or invalid values while preserving the default only for omitted
input if intended. In the POST /upsert and other mutation handlers that consume
its result, reject null with a 400 response and the specified invalid
day_of_week message before writing rows.
- Around line 75-79: Extract the existing /my-class route logic into a shared
handler middleware, then register that handler for both /my-class and
/student/my-timetable while preserving the appropriate authentication and
authorization middleware. Remove the req.url reassignment and router.handle call
from the alias route.

In `@backend/scratch/verify_timetable.js`:
- Around line 126-138: Update the cleanup flow after the timetable upsert to
target only the row created by the current test session, rather than querying
all matching section, period, and day records. Reuse the upserted row’s
identifier when calling DELETE, or include the current session constraint in the
lookup; leave the console.log template string unchanged.
- Around line 49-142: Add explicit status and response assertions throughout the
verification flow so failures cause the script to exit non-zero instead of
always printing success. Validate each endpoint’s expected result, especially
requiring res5.status to equal 403 for the subject-teacher upsert, and ensure
cleanup assertions do not mask earlier failures before printing the final
success message.
- Around line 5-9: The JWT signing in generateToken must use the same
ACCESS_SECRET resolution as backend/middleware/auth.js: prefer
JWT_ACCESS_SECRET, then JWT_SECRET, without any hardcoded fallback. Remove
'your-super-secret-jwt-key' and ensure the script fails clearly when neither
environment variable is set.

In `@frontend/src/pages/TeacherDashboard.jsx`:
- Around line 1341-1352: Update the day-of-week select in the existing slot
editor, using editingSlot, so it is disabled while editing an existing slot and
remains editable for new slots. In the save/upsert flow, when editingSlot is
set, delete the original slot only after the upsert succeeds, preserving the
existing slot when the upsert fails.
- Around line 144-159: Remove the mount-time fetchTtMetadata() call from the
initial useEffect so metadata is loaded only by the selectedTtClass effect.
Update fetchAssignedTtClasses to explicitly load unfiltered metadata when no
assigned class leaves selectedTtClass null.
- Around line 347-358: Add a Delete button to the timetable modal’s action area,
alongside Cancel and Save, rendered only when editingSlot is set and wired to
handleDeleteTtSlot with the edited period ID. Also add the selectedTtClass guard
used by handleSaveTtSlot before handleDeleteTtSlot accesses class_id or
section_id.
- Around line 1302-1317: Update the showTtModal overlay container to match the
attendance modal’s dialog semantics by adding role="dialog" and
aria-modal="true"; add Escape-key handling that calls setShowTtModal(false), and
manage focus so the first modal control receives focus when it opens.
- Around line 1169-1233: Update the grid cell rendering around the clickable div
so editable cells use a keyboard-accessible button with appropriate semantics
and activation behavior, while preserving non-editable display cells. Ensure the
cell’s edit affordance remains keyboard reachable, and change the Edit button’s
visibility styling to reveal it on focus as well as mouse hover; keep its
existing click propagation handling and modal action.

---

Minor comments:
In `@backend/scratch/verify_timetable.js`:
- Around line 22-47: Guard the fixture results in the verification flow before
dereferencing them: validate ct after the class-teacher query, st after the
substitute-teacher query, and student after the student query. If any result is
missing, report a clear missing-seed-data message and stop before calling
generateToken, while preserving the existing success path when all fixtures
exist.

In `@frontend/src/pages/Login.jsx`:
- Around line 168-178: Update the password toggle button in Login.jsx to include
a state-dependent aria-label describing whether it will show or hide the
password, and set aria-pressed from showPassword so assistive technology exposes
the current toggle state.

In `@frontend/src/pages/SchoolLandingPage.jsx`:
- Around line 97-101: Add the `group` class to the sign-in `Link` containing the
`LogIn` icon, preserving its existing classes so the icon’s
`group-hover:translate-x-0.5` behavior activates on link hover.

In `@frontend/src/pages/TeacherDashboard.jsx`:
- Around line 1189-1201: Differentiate the labels for explicit breaks and
missing schedule slots in the rendering branch around item.is_break: keep “Free
Period” for break items, but change the null-item fallback to “Not Scheduled.”
Leave the subject label and existing styling unchanged.
- Around line 334-339: Update the success handling in the timetable save flow
around the success branch and its ttMessage rendering so “Slot saved
successfully!” remains visible after setShowTtModal(false). Render the
confirmation outside the modal, or reuse the existing page-level message state
and rendering, while preserving the current modal-close and refresh behavior.

---

Nitpick comments:
In `@backend/modules/timetable/timetable.routes.js`:
- Around line 502-512: Update the assignments query in the timetable route so
the second UNION branch against timetables filters rows to the current academic
session using the existing session context or symbol, while retaining the
subject and teacher assignment selection behavior.
- Around line 163-184: The route handler for GET
/class/:classId/section/:sectionId must validate that the requested section
belongs to classId before authorization and timetable retrieval, using the
existing database access flow and returning an appropriate not-found or
access-denied response on mismatch; alternatively remove classId consistently
from the route and handler. Ensure all downstream logic uses the validated
relationship.

In `@frontend/src/pages/TeacherDashboard.jsx`:
- Around line 39-48: Remove the duplicated STANDARD_PERIOD_TIMES definition and
use the backend-provided DEFAULT_PERIOD_TIMES through the existing data flow, or
move both consumers to one shared constant. Update the TeacherDashboard
rendering logic to consume that single source of truth while preserving the
seven period mappings and their stored time format.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7b6313c9-f2c8-4d65-a8f4-fa53f4371c78

📥 Commits

Reviewing files that changed from the base of the PR and between 5e10c08 and 0e1031f.

📒 Files selected for processing (13)
  • backend/index.js
  • backend/migrations/010_timetable_breaks.sql
  • backend/modules/homework/homework.routes.js
  • backend/modules/payments/payments.route.js
  • backend/modules/staff/teacherAssignment.routes.js
  • backend/modules/timetable/timetable.routes.js
  • backend/routes/authRoutes.js
  • backend/scratch/verify_timetable.js
  • frontend/src/App.jsx
  • frontend/src/pages/Login.jsx
  • frontend/src/pages/SchoolLandingPage.jsx
  • frontend/src/pages/StudentDashboard.jsx
  • frontend/src/pages/TeacherDashboard.jsx
💤 Files with no reviewable changes (1)
  • backend/routes/authRoutes.js

Comment thread backend/migrations/010_timetable_breaks.sql Outdated
Comment thread backend/modules/homework/homework.routes.js Outdated
Comment thread backend/modules/payments/payments.route.js
Comment thread backend/modules/timetable/timetable.routes.js
Comment thread backend/modules/timetable/timetable.routes.js Outdated
Comment thread frontend/src/pages/TeacherDashboard.jsx
Comment thread frontend/src/pages/TeacherDashboard.jsx
Comment thread frontend/src/pages/TeacherDashboard.jsx
Comment thread frontend/src/pages/TeacherDashboard.jsx
Comment on lines +1341 to +1352
<div>
<label className="font-extrabold text-slate-700 block mb-1">Day of Week</label>
<select
value={ttFormData.day_of_week}
onChange={(e) => setTtFormData({ ...ttFormData, day_of_week: e.target.value })}
className="w-full px-3 py-2 bg-slate-50 border border-slate-200 rounded-xl font-bold text-slate-800 focus:outline-none focus:ring-2 focus:ring-teal-500"
>
{daysOfWeek.map((d) => (
<option key={`opt-${d}`} value={d}>{d}</option>
))}
</select>
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Changing the day of an existing slot duplicates the period.

POST /upsert keys a slot on (section_id, day_of_week, period_no, session_id). See backend/modules/timetable/timetable.routes.js lines 334-338. When the user opens an existing Monday slot and changes this select to Tuesday, the request creates a new Tuesday row. It does not remove the Monday row. The timetable then holds two slots where the user expected one move.

If editingSlot is set, disable the day select. Otherwise delete the original slot after the upsert succeeds.

🐛 Proposed fix
                             <select
                               value={ttFormData.day_of_week}
+                              disabled={Boolean(editingSlot)}
                               onChange={(e) => setTtFormData({ ...ttFormData, day_of_week: e.target.value })}
-                              className="w-full px-3 py-2 bg-slate-50 border border-slate-200 rounded-xl font-bold text-slate-800 focus:outline-none focus:ring-2 focus:ring-teal-500"
+                              className="w-full px-3 py-2 bg-slate-50 border border-slate-200 rounded-xl font-bold text-slate-800 focus:outline-none focus:ring-2 focus:ring-teal-500 disabled:opacity-60 disabled:cursor-not-allowed"
                             >
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div>
<label className="font-extrabold text-slate-700 block mb-1">Day of Week</label>
<select
value={ttFormData.day_of_week}
onChange={(e) => setTtFormData({ ...ttFormData, day_of_week: e.target.value })}
className="w-full px-3 py-2 bg-slate-50 border border-slate-200 rounded-xl font-bold text-slate-800 focus:outline-none focus:ring-2 focus:ring-teal-500"
>
{daysOfWeek.map((d) => (
<option key={`opt-${d}`} value={d}>{d}</option>
))}
</select>
</div>
<div>
<label className="font-extrabold text-slate-700 block mb-1">Day of Week</label>
<select
value={ttFormData.day_of_week}
disabled={Boolean(editingSlot)}
onChange={(e) => setTtFormData({ ...ttFormData, day_of_week: e.target.value })}
className="w-full px-3 py-2 bg-slate-50 border border-slate-200 rounded-xl font-bold text-slate-800 focus:outline-none focus:ring-2 focus:ring-teal-500 disabled:opacity-60 disabled:cursor-not-allowed"
>
{daysOfWeek.map((d) => (
<option key={`opt-${d}`} value={d}>{d}</option>
))}
</select>
</div>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/pages/TeacherDashboard.jsx` around lines 1341 - 1352, Update the
day-of-week select in the existing slot editor, using editingSlot, so it is
disabled while editing an existing slot and remains editable for new slots. In
the save/upsert flow, when editingSlot is set, delete the original slot only
after the upsert succeeds, preserving the existing slot when the upsert fails.

@Kuzuri247
Kuzuri247 merged commit 0c40110 into main Aug 9, 2026
3 checks passed
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.

1 participant