Conversation
…taff routing with new frontend dashboard pages and initial database migrations
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
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 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe 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. ChangesTimetable management
Authorization and route changes
Frontend presentation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 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
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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 winGive the password toggle an accessible name.
The button contains only an icon. Add a state-dependent
aria-labelandaria-pressedvalue 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 winAdd
groupto the sign-inLink.
LogInusesgroup-hover:translate-x-0.5, so the parent link needs thegroupclass 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 winTwo different states render the same label.
Line 1190 renders "Free Period" when
item.is_breakis 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_breakand "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 winThe success message is never visible.
Line 335 sets
ttMessage. Line 336 closes the modal.ttMessagerenders 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
ttMessageand use the existing page-levelmessagestate.🤖 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 winGuard the fixture queries against empty result sets.
Line 38 reads
ct.teacher_user_id. If no row hasis_class_teacher = 1,ctisundefinedand the script throws aTypeError. The catch block then printsHTTP VERIFICATION FAILEDwith a null-reference message that hides the real cause, which is missing seed data. The same applies tostat line 46 andstudentat 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 winThe
UNIONsubquery scans the wholetimetablestable.The second branch reads every row in
timetableswith 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
classIdis not validated against the section.The route accepts
classId, but the authorization check and the timetable query both use onlysectionId. A caller can pass anyclassIdwith a validsectionIdand receive data. Either verify thatsectionIdbelongs toclassId, or removeclassIdfrom 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_TIMESduplicates the backend defaults.
DEFAULT_PERIOD_TIMESinbackend/modules/timetable/timetable.routes.jslines 295-303 defines the same seven period times with a:00seconds 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
📒 Files selected for processing (13)
backend/index.jsbackend/migrations/010_timetable_breaks.sqlbackend/modules/homework/homework.routes.jsbackend/modules/payments/payments.route.jsbackend/modules/staff/teacherAssignment.routes.jsbackend/modules/timetable/timetable.routes.jsbackend/routes/authRoutes.jsbackend/scratch/verify_timetable.jsfrontend/src/App.jsxfrontend/src/pages/Login.jsxfrontend/src/pages/SchoolLandingPage.jsxfrontend/src/pages/StudentDashboard.jsxfrontend/src/pages/TeacherDashboard.jsx
💤 Files with no reviewable changes (1)
- backend/routes/authRoutes.js
| <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> |
There was a problem hiding this comment.
🗄️ 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.
| <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.
…uthentication pages
Summary by CodeRabbit
New Features
Bug Fixes
Style