WIP: discipline table#1549
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a discipline section to the LicensingDetail page that displays licensee adverse actions. It introduces a new AlertCircle icon component, extends the Licensee and AdverseAction data models, adds localization strings, and implements responsive UI with collapsible discipline records including sorting by start date and state. ChangesDiscipline and Adverse Action UI and Data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 1
🧹 Nitpick comments (1)
webroot/src/pages/LicensingDetail/LicensingDetail.less (1)
299-378: 💤 Low value
.no-disciplineelement has no style rule defined.The Vue template renders
<div class="no-discipline">when there are no discipline records, but no corresponding CSS rule exists for it. The text will display with no indentation, padding, or font styling — inconsistent with other "empty state" presentations on the page.🎨 Suggested fix
.discipline-list { ... } + + .no-discipline { + padding: 1.6rem 0; + color: `@fontColorLight`; + } }🤖 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 `@webroot/src/pages/LicensingDetail/LicensingDetail.less` around lines 299 - 378, Add a CSS rule for the .no-discipline element in LicensingDetail.less (place it near .discipline-section/.discipline-list for discoverability) so the empty-state matches other rows: give .no-discipline similar margin/padding and top-border as .discipline-row (padding: 1.6rem 0; border-top when not-first or explicit top border), set font-weight or muted color consistent with .cell-title/.header, and ensure responsive behavior (tablet/desktop media queries) so it aligns with other .discipline-cell layout; target the class name ".no-discipline" to implement these styles.
🤖 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 `@webroot/src/pages/LicensingDetail/LicensingDetail.ts`:
- Around line 348-360: The comparator sortByDisciplineStart currently compares
display-formatted dates from AdverseAction.startDateDisplay(), which are
M/D/YYYY strings and sort incorrectly; change it to compare the underlying ISO
date values (e.g., AdverseAction.startDate) or parse the ISO strings into Date
objects and compare numerically so ordering is correct; update
sortByDisciplineStart to read action1.startDate and action2.startDate (or
Date.parse(action1.startDate)/Date.parse(action2.startDate)) and return -1/0/1
based on those comparisons.
---
Nitpick comments:
In `@webroot/src/pages/LicensingDetail/LicensingDetail.less`:
- Around line 299-378: Add a CSS rule for the .no-discipline element in
LicensingDetail.less (place it near .discipline-section/.discipline-list for
discoverability) so the empty-state matches other rows: give .no-discipline
similar margin/padding and top-border as .discipline-row (padding: 1.6rem 0;
border-top when not-first or explicit top border), set font-weight or muted
color consistent with .cell-title/.header, and ensure responsive behavior
(tablet/desktop media queries) so it aligns with other .discipline-cell layout;
target the class name ".no-discipline" to implement these styles.
🪄 Autofix (Beta)
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
Run ID: 6ac74b7b-4607-4a72-86d5-502d050c4be2
📒 Files selected for processing (15)
webroot/src/components/Icons/AlertCircle/AlertCircle.lesswebroot/src/components/Icons/AlertCircle/AlertCircle.spec.tswebroot/src/components/Icons/AlertCircle/AlertCircle.tswebroot/src/components/Icons/AlertCircle/AlertCircle.vuewebroot/src/locales/en.jsonwebroot/src/locales/es.jsonwebroot/src/models/AdverseAction/AdverseAction.model.spec.tswebroot/src/models/AdverseAction/AdverseAction.model.tswebroot/src/models/Licensee/Licensee.model.spec.tswebroot/src/models/Licensee/Licensee.model.tswebroot/src/models/StaffUser/StaffUser.model.spec.tswebroot/src/network/mocks/mock.data.tswebroot/src/pages/LicensingDetail/LicensingDetail.lesswebroot/src/pages/LicensingDetail/LicensingDetail.tswebroot/src/pages/LicensingDetail/LicensingDetail.vue
Requirements List
Description List
adverseActionstop-level propertyTesting List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsCloses #1477
Summary by CodeRabbit
New Features
Style
Tests