Skip to content

Fix/warnings UI#75

Merged
BeWelsh merged 3 commits into
mainfrom
fix/warnings-ui
Apr 24, 2026
Merged

Fix/warnings UI#75
BeWelsh merged 3 commits into
mainfrom
fix/warnings-ui

Conversation

@BeWelsh
Copy link
Copy Markdown
Collaborator

@BeWelsh BeWelsh commented Apr 24, 2026

Description

Update warning UI to include dismissal, and removal of warnings on section update

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (code improvement without changing functionality)
  • Documentation update
  • Configuration/infrastructure change
  • Performance improvement
  • Test coverage improvement

What Changed?

Testing & Validation

How this was tested

Screenshots/Recordings

Unfinished Work & Known Issues

  • None, this PR is complete and production-ready
  • The following items are intentionally deferred:



Notes & Nuances



Reviewer Notes

  • Areas needing extra attention: ...
  • Questions for reviewers: ...

Summary by CodeRabbit

  • New Features

    • Warnings now display the admin who dismissed them.
    • Dismissed warnings can be restored to active status.
    • Warning UI separates active warnings from dismissed ones.
  • Improvements

    • Warning management endpoints now require admin authorization.
    • Warning dismissals now persist per-section across algorithm runs.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

This PR implements a warning dismissal system with user tracking across the backend and frontend. It adds section-level warning identification to time assignment results, implements dismissal persistence with authenticated user attribution, separates dismissed vs. active warnings in UI, and broadcasts warning changes via WebSocket.

Changes

Cohort / File(s) Summary
Data Models & API Schema
backend/app/algorithms/models.py, backend/app/schemas/warning.py, frontend/src/api/generated.ts
Added warning_section_ids to TimeBlockAssignmentResult and dismissed_by field to WarningResponse to track warning dismissal attribution and section associations.
Time Assignment Logic
backend/app/algorithms/time_assignment.py
Updated assign_time_blocks to track section IDs for unplaced sections and include them in the result payload via warning_section_ids.
Warning Persistence & Reconciliation
backend/app/repositories/schedule_warning.py, backend/app/services/algorithm.py
Refactored warning synchronization to preserve dismissed warnings while deduplicating per-section, added _persist_per_section_warnings routine with section ID mapping, and extended _persist_warnings signature to handle section-level dismissal deduplication.
Warning Route Handlers
backend/app/routers/schedule_warning.py
Added admin-only authorization to dismiss/restore/delete endpoints, converted handlers to async, recorded dismissing user via dismissed_by, and added WebSocket broadcasts after state changes.
Section Update Broadcasting
backend/app/routers/section.py
Modified update_section to unconditionally broadcast section_warnings after updates instead of only when warnings exist.
Test Updates
backend/tests/test_algorithm_service.py
Updated _persist_warnings call signatures to include new list arguments and section ID mapping parameter.
Frontend Warning UI & Data Fetching
frontend/src/components/SectionMutationDrawer.tsx, frontend/src/components/ScheduleSectionRowView.tsx, frontend/src/stores/scheduleDataStore.ts
Implemented active/dismissed warning separation in drawer with dismiss/restore buttons, user attribution display, and loading states; updated warning queries to include dismissed items; modified section edit drawer to include all warnings regardless of dismissal status.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Frontend
    participant API as Backend API
    participant DB as Database
    participant WS as WebSocket Manager

    Client->>API: PATCH /schedules/{id}/warnings/{warning_id}/dismiss<br/>(with user context)
    API->>API: Verify require_admin authorization
    API->>DB: Update warning row<br/>SET dismissed=true, dismissed_by=user_name
    DB-->>API: Row updated
    API->>WS: broadcast("section_warnings",<br/>{section_id, warnings: []})
    WS->>Client: WebSocket message received
    Client->>Client: Update scheduleDataStore<br/>with new warning state
    Client->>Client: Re-render SectionMutationDrawer<br/>(move to dismissed set)
    API-->>Client: 200 OK (WarningResponse with dismissed_by)

    Note over Client,WS: Restore flow is similar,<br/>setting dismissed=false, dismissed_by=null
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • PR #39: Introduces and extends time-assignment warning tracking with TimeBlockAssignmentResult and warning_section_ids field population in assign_time_blocks.
  • PR #59: Modifies sync_section_warnings reconciliation logic and warning persistence flow; directly affected by section ID mapping introduced here.
  • PR #67: Updates warning API responses and WebSocket broadcasting patterns in section routers, providing foundation for dismissal state propagation.

Suggested reviewers

  • eliannemejia
  • jobvengalil
  • Saisri24

Poem

🐰 A dismissal with flair!
Warnings now tracked who's to blame,
Admins dismiss with a name,
WebSockets relay the news,
UI shows dismissed or not—choose! 📢

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fix/warnings UI' is vague and does not clearly convey the main changes; it lacks specificity about what aspect of the warnings UI is being fixed or updated. Use a more descriptive title that specifically mentions the key changes, such as 'Add warning dismissal UI and clear warnings on section update' or 'Implement warning dismissal functionality and auto-clear on section changes'.
Description check ❓ Inconclusive The PR description includes the required sections and headers but leaves critical sections largely empty: 'What Changed?' lists only empty bullet points, 'Testing & Validation' and 'Notes & Nuances' sections are unfilled placeholder text, and 'Reviewer Notes' contains only template instructions. Complete the empty sections with concrete details: list specific changes in 'What Changed?', describe testing steps in 'Testing & Validation', and add relevant notes about implementation decisions and edge cases in 'Notes & Nuances' and 'Reviewer Notes'.
✅ Passed checks (2 passed)
Check name Status Explanation
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/warnings-ui

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 and usage tips.

@BeWelsh BeWelsh merged commit 5e43fe0 into main Apr 24, 2026
2 of 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