PR 5: refactor(effort): component extraction + report context centralization#178
Open
rlorenzo wants to merge 12 commits intorefactor/dead-code-and-shared-chromefrom
Open
PR 5: refactor(effort): component extraction + report context centralization#178rlorenzo wants to merge 12 commits intorefactor/dead-code-and-shared-chromefrom
rlorenzo wants to merge 12 commits intorefactor/dead-code-and-shared-chromefrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
5 tasks
rlorenzo
force-pushed
the
refactor/effort-component-extraction
branch
from
eee8d10 to
8f5ffd2
Compare
rlorenzo
force-pushed
the
refactor/dead-code-and-shared-chrome
branch
from
ebe32ec to
75abeb2
Compare
Hoist breadcrumb, loading spinner, and not-found banner from EmergencyContactForm.vue and EmergencyContactView.vue into a new EmergencyContactPageShell.vue. Each page now owns only its h1 and body content. Pure template refactor; no runtime change.
AddCourseEffortDialog and EffortRecordEditDialog shared a Cancel + primary-action q-card-actions block with an identical #loading slot. Hoist into DialogSubmitActions.vue parameterized by submitLabel and isSaving. Other Effort dialogs still inline the pattern; migrate as they are touched.
PercentAssignmentAddDialog and PercentAssignmentEditDialog shared a 165-line q-select/q-input/q-checkbox form body. Hoist into PercentAssignmentFormFields.vue, v-modeled on the form state the usePercentageForm composable already centralized. Each dialog now owns only its title, save logic, banner variants, and footer. Also migrate both footers to DialogSubmitActions. PercentageFormState and TypeOption are re-exported from the composable so the new component can type its props.
Consolidates the h1 + ReportFilterForm + loading spinner + ReportLayout header/ExportToolbar + empty-state boilerplate that was duplicated across all 9 standard Effort report pages into a single wrapper, so individual pages only define their table content.
Share the Role / Effort Value / Notes / warning + error banner block between EffortRecordAddDialog and EffortRecordEditDialog, and route the Add dialog's footer through the existing DialogSubmitActions component.
Share the Instructors breadcrumb, loading spinner, and error banner between InstructorDetail and InstructorEdit via a slot-based shell.
Share the dialog scaffold (close affordance, title/subtitle, loading spinner, progress bar, and error state) between HarvestDialog, PercentRolloverDialog, and ClinicalImportDialog. Per-dialog preview bodies and action buttons stay in the consumers.
Share the desktop table + mobile list rendering across unopened / open / closed term sections instead of inlining three near-identical q-table blocks.
Move ITermService injection into BaseReportService and add shared helpers (LoadSingleTermContextAsync, LoadYearlyReportContextAsync, ExtractDistinctEffortTypes) so DeptSummary, SchoolSummary, and TeachingActivity services no longer hand-roll the same row + clinical-faculty + term-name plumbing.
…tService Replace four near-identical N5..N1 weighted-average + divide-by-zero guards in the summary and detail builders with a single helper.
The InstructorPageShell extraction (refactor 4601108) replaced the StatusBanner import in InstructorEdit.vue, but the template still references StatusBanner three times for save/error feedback. Vue logged "Failed to resolve component: StatusBanner" warnings and rendered those banners blank.
Carry-over from the analyzer-driven cleanup batch (PR 3). The materialization fix targets the helper extracted in d12711e — that helper doesn't exist in PR 3's base, so the fix moves here.
rlorenzo
force-pushed
the
refactor/effort-component-extraction
branch
from
8f5ffd2 to
9cad656
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part 5 of 6. Stacks on top of PR #177.
Effort-area refactors driven by jscpd (duplication detection): extract repeated UI shells, dialog scaffolding, and form-field clusters into reusable components. Plus a small report-service consolidation in C#. Also folds in the
EmergencyContactshared page shell because it pairs naturally with the Effort UI patterns.What's in this PR
Vue/UI extractions
EmergencyContactPageShell— shared page shell for emergency contact formsDialogSubmitActions— submit/cancel button cluster used across multiple dialogsPercentAssignmentFormFields— shared form-field clusterEffortReportPage— page-layout shell for report screensEffortRecordSharedFields— duplicate field cluster across record dialogsInstructorPageShell— breadcrumbs + loading/error states for instructor pagesAsyncOperationDialog— preview-dialog shellTermTable— term-selection rows componentC# / report services
BaseReportServicecentralizes report context loading; per-report services now consume it instead of each loading the term/context independentlyCalculateWeightedAverageextracted inEvaluationReportServiceto replace four near-identical N5..N1 weighted-average + divide-by-zero guardsBundled fixes
fix(effort): restore StatusBanner import in InstructorEdit— theInstructorPageShellextraction dropped aStatusBannerimport the template still referenced. Pairs with the extraction commit it patches.chore(quality): materialize CalculateWeightedAverage rows once— carry-over from PR 3'smaterialize IEnumerablebatch. The helper that needed materialization is introduced in this PR, so the analyzer fix is bundled here rather than in PR 3.Commits
refactor(emergency-contact): extract shared page shellrefactor(effort): extract DialogSubmitActions componentrefactor(effort): extract PercentAssignmentFormFieldsrefactor(effort): extract EffortReportPage layout shellrefactor(effort): extract EffortRecordSharedFields from dialogsrefactor(effort): extract InstructorPageShell for breadcrumbs and statesrefactor(effort): extract AsyncOperationDialog shell for preview dialogsrefactor(effort): extract TermTable for term selection rowsrefactor(effort): centralize report context loading in BaseReportServicerefactor(effort): extract CalculateWeightedAverage in EvaluationReportServicefix(effort): restore StatusBanner import in InstructorEditchore(quality): materialize CalculateWeightedAverage rows oncePR stack
Test plan