Skip to content

PR 4: refactor: dead-code removal + shared chrome extraction#177

Open
rlorenzo wants to merge 14 commits into
quality/analyzer-bulk-cleanupfrom
refactor/dead-code-and-shared-chrome
Open

PR 4: refactor: dead-code removal + shared chrome extraction#177
rlorenzo wants to merge 14 commits into
quality/analyzer-bulk-cleanupfrom
refactor/dead-code-and-shared-chrome

Conversation

@rlorenzo
Copy link
Copy Markdown
Contributor

@rlorenzo rlorenzo commented May 5, 2026

Summary

Part 4 of 6. Stacks on top of PR #176.

Cross-cutting refactors driven by fallow (dead-code detection) and jscpd (duplication detection). Net effect: ~2200 lines deleted, ~200 added.

What's in this PR

  • Phases 1–3: dead-code removal — unused exports, types, components surfaced by fallow.
  • Clinical Scheduler orphaned store cluster: a small group of Pinia stores with no remaining callers.
  • CSS @import migration (Phase 4c): move CSS @import rules to JS-side imports for more deterministic Vite processing.
  • createSpaRouter factory: consolidates the duplicated router setup that each Vue SPA was open-coding.
  • Shared layout chrome: extract EmulationBanner, FooterLinks, and ViperBrandButton from the monolithic ViperLayout.vue so layouts can share them without duplication.

Commits

  • refactor: remove dead code surfaced by fallow (Phase 1)
  • refactor(clinical-scheduler): drop orphaned store cluster (Phase 2)
  • refactor: remove unused type exports (Phase 3)
  • refactor: migrate CSS @import to JS-side imports (Phase 4c)
  • refactor(router): extract createSpaRouter factory
  • refactor(layouts): extract shared chrome components from ViperLayout

Note

The original branch had a fix(effort): restore StatusBanner import in InstructorEdit here. That fix repairs a bug introduced by the InstructorPageShell extraction (which lives in PR 5), so on this base the bug doesn't exist. The fix is moved into PR 5 alongside the extraction it patches.

PR stack

Test plan

  • CI green
  • Smoke-test all Vue SPAs that use createSpaRouter (route loads + navigation guard)
  • Verify EmulationBanner, FooterLinks, ViperBrandButton render correctly in ViperLayout and ViperLayoutSimple
  • npm run audit:fallow — no regressions

Summary by CodeRabbit

  • New Features

    • Added emulation banner displaying current emulated user status with end-emulation option.
    • Added VIPER brand button component for consistent navigation header.
    • Added footer links component for external resource navigation.
  • Refactor

    • Consolidated router configuration across multiple modules using shared factory.
    • Modernized stylesheet imports to direct script references instead of CSS @import directives.
    • Streamlined internal type and export surfaces across services and stores.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • review-ready

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f8ce3198-c752-4066-85cf-182a3f481c79

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Large-scale refactoring consolidating router initialization into a shared factory (createSpaRouter), removing ClinicalScheduler store state management files, pruning non-public type exports across multiple modules, migrating stylesheet imports from Vue style blocks to script sections, and introducing new layout components (EmulationBanner, FooterLinks, ViperBrandButton).

Changes

Shared Router Infrastructure & Layout Component Refactoring

Layer / File(s) Summary
Router Factory
VueApp/src/shared/createSpaRouter.ts
New shared helper wraps Vue Router initialization with createWebHistory, scroll-to-top behavior, and useRouteFocus integration. Accepts route records and returns configured Router instance.
Router Module Updates
VueApp/src/CAHFS/router/index.ts, VueApp/src/CMS/router/index.ts, VueApp/src/CTS/router/index.ts, VueApp/src/ClinicalScheduler/router/index.ts, VueApp/src/Computing/router/index.ts, VueApp/src/Effort/router/index.ts, VueApp/src/Students/router/index.ts
Each router delegates creation to createSpaRouter(routes) instead of manually configuring vue-router. Existing beforeEach guard logic (login/permission checks) preserved in each module.
Layout Components
VueApp/src/layouts/EmulationBanner.vue, VueApp/src/layouts/FooterLinks.vue, VueApp/src/layouts/ViperBrandButton.vue
New reusable components extract previously inline UI: emulation status banner, footer navigation links with icons, and VIPER brand button with environment badge support.
Layout Integration
VueApp/src/layouts/ViperLayout.vue, VueApp/src/layouts/ViperLayoutSimple.vue
Refactor to import and render new layout components instead of inline markup; remove local state management for environment/emulation UI.

ClinicalScheduler State Management Architecture

Layer / File(s) Summary
Store File Removal
VueApp/src/ClinicalScheduler/stores/schedule.ts, VueApp/src/ClinicalScheduler/stores/schedule-actions.ts, VueApp/src/ClinicalScheduler/stores/schedule-cache.ts
Delete entire Pinia store, action creators (load schedule, manage instructors, set primary evaluator), and in-memory cache class with staleness tracking and invalidation logic.
Utility Adjustment
VueApp/src/ClinicalScheduler/stores/permissions-utils.ts
De-export PermissionsGetters interface; implementation remains unchanged.

Module Export Surface Pruning

Layer / File(s) Summary
ClinicalScheduler Services
VueApp/src/ClinicalScheduler/services/clinician-service.ts, VueApp/src/ClinicalScheduler/services/error-transformer.ts, VueApp/src/ClinicalScheduler/services/instructor-schedule-service.ts, VueApp/src/ClinicalScheduler/services/page-data-service.ts, VueApp/src/ClinicalScheduler/services/permission-service.ts
Narrow exports to only primary service classes and core type aliases; remove re-exports of request/response/result/error types and internal utility types.
ClinicalScheduler Types
VueApp/src/ClinicalScheduler/types/api-responses.ts, VueApp/src/ClinicalScheduler/types/index.ts, VueApp/src/ClinicalScheduler/types/rotation-types.ts, VueApp/src/ClinicalScheduler/utils/schedule-update-helpers.ts
Remove exports for API envelopes, schedule/instructor/conflict types, and internal helper types; retain only high-level data shapes.
ClinicalScheduler Constants
VueApp/src/ClinicalScheduler/constants/permission-messages.ts, VueApp/src/ClinicalScheduler/constants/schedule-config.ts
Stop exporting aggregated message constants and view configuration; export only underlying message/label/config blocks.
ClinicalScheduler Components
VueApp/src/ClinicalScheduler/components/ScheduleView.vue, VueApp/src/ClinicalScheduler/components/WeekCell.vue
De-export WeekItem and WeekAssignment interfaces; remove global style imports from scoped blocks.
Effort Module Types
VueApp/src/Effort/types/dashboard-types.ts, VueApp/src/Effort/types/evaluation-types.ts, VueApp/src/Effort/types/harvest-types.ts, VueApp/src/Effort/types/report-types.ts, VueApp/src/Effort/types/report-types-extended.ts, VueApp/src/Effort/types/verification-types.ts, VueApp/src/Effort/types/index.ts
Remove detailed row/group/preview/error type exports; retain only high-level report and DTO types; narrow cross-module re-exports in index.
Effort Services & Composables
VueApp/src/Effort/services/harvest-service.ts, VueApp/src/Effort/composables/use-effort-type-columns.ts, VueApp/src/Effort/composables/use-percentage-form.ts
De-export progress event types, column option type, and date formatting helpers; keep only primary composable and service exports.
Effort Config
VueApp/src/config/colors.ts
Replace multi-layer palette (CSS variables, shade objects, caching) with simplified BRAND_COLORS and semanticColors map; export only semantic colors and contrast/accessibility helpers.
Shared & Other Composables
VueApp/src/composables/DateFunctions.ts, VueApp/src/composables/QuasarConfig.ts, VueApp/src/composables/validation-error.ts
Remove formatting helper and config composable exports; de-export internal error types.
CTS & Students
VueApp/src/CTS/types/index.ts, VueApp/src/Students/services/photo-gallery-service.ts
Update type exports; stop exporting service class in favor of singleton instance and helper functions.

Asset Organization & Stylesheet Migration

Layer / File(s) Summary
Stylesheet Scoping
VueApp/src/ClinicalScheduler/assets/schedule-shared.css
Scope header typography rules (h2/h3) to .clinical-scheduler-container instead of applying globally across all breakpoints.
Stylesheet Import Pattern
VueApp/src/ClinicalScheduler/pages/ClinicianScheduleView.vue, VueApp/src/ClinicalScheduler/pages/RotationScheduleView.vue, VueApp/src/Effort/pages/ClinicalEffort.vue, VueApp/src/Effort/pages/DeptSummary.vue, VueApp/src/Effort/pages/EvalDetail.vue, VueApp/src/Effort/pages/EvalSummary.vue, VueApp/src/Effort/pages/MeritAverage.vue, VueApp/src/Effort/pages/MeritDetail.vue, VueApp/src/Effort/pages/MeritSummary.vue, VueApp/src/Effort/pages/MultiYearReport.vue, VueApp/src/Effort/pages/SchoolSummary.vue, VueApp/src/Effort/pages/TeachingActivityGrouped.vue, VueApp/src/Effort/pages/TeachingActivityIndividual.vue, VueApp/src/Students/EmergencyContact/pages/EmergencyContactForm.vue
Move shared stylesheet includes from unscoped <style>@import url(...) blocks into script-level ES imports; remove empty/import-only style sections.
Component Removals
VueApp/src/CTS/pages/CourseList.vue, VueApp/src/Students/components/PhotoGallery/PhotoGrid.vue, VueApp/src/Students/components/PhotoGallery/PhotoList.vue, VueApp/src/components/icons/IconCommunity.vue, VueApp/src/components/icons/IconDocumentation.vue, VueApp/src/components/icons/IconEcosystem.vue, VueApp/src/components/icons/IconSupport.vue, VueApp/src/components/icons/IconTooling.vue
Delete empty or template-only component stubs (icon templates, photo gallery list/grid, course list placeholder).
Composable Organization
VueApp/src/Effort/components/InstructorPercentEditDialog.vue
Component implementation shown in full-context diff; no functional change (diff is display of existing state).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/dead-code-and-shared-chrome

@rlorenzo rlorenzo force-pushed the quality/analyzer-bulk-cleanup branch from 0d26c81 to 438134a Compare May 5, 2026 07:39
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from ebe32ec to 75abeb2 Compare May 5, 2026 07:39
@rlorenzo rlorenzo force-pushed the quality/analyzer-bulk-cleanup branch from 438134a to 11799f3 Compare May 5, 2026 14:47
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from 75abeb2 to 0c49b01 Compare May 5, 2026 14:47
@rlorenzo rlorenzo force-pushed the quality/analyzer-bulk-cleanup branch from 11799f3 to 2580a27 Compare May 7, 2026 13:59
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from 0c49b01 to 2b9ea83 Compare May 7, 2026 14:09
@rlorenzo
Copy link
Copy Markdown
Contributor Author

rlorenzo commented May 7, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
VueApp/src/ClinicalScheduler/assets/schedule-shared.css (1)

18-23: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Replace px typography sizes with rem in the scoped heading rules.

These changed heading rules still use px, which violates the stylesheet guideline. Convert to rem (e.g., 1rem, 1.125rem) in all three scoped selectors.

Suggested diff
 .clinical-scheduler-container h3 {
     color: var(--ucdavis-black-80);
-    font-size: 16px;
+    font-size: 1rem;
     margin-top: 20px;
     margin-bottom: 10px;
 }

 `@media` (max-width: 768px) {
     .clinical-scheduler-container h2 {
-        font-size: 18px;
+        font-size: 1.125rem;
     }
 }

 `@media` (max-width: 480px) {
     .clinical-scheduler-container h2 {
-        font-size: 16px;
+        font-size: 1rem;
     }
 }

As per coding guidelines, **/*.{vue,css,scss}: "Use rem (not px) for spacing, sizing, and typography."

Also applies to: 41-43, 96-98

🤖 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 `@VueApp/src/ClinicalScheduler/assets/schedule-shared.css` around lines 18 -
23, Update the scoped heading rules to use rem units instead of px: in the
.clinical-scheduler-container h3 rule change font-size: 16px → 1rem, margin-top:
20px → 1.25rem, and margin-bottom: 10px → 0.625rem; apply the same px→rem
conversions to the other two scoped heading selectors mentioned in the review so
all typography and spacing use rem (e.g., 16px→1rem, 18px→1.125rem,
20px→1.25rem).
VueApp/src/Effort/router/index.ts (1)

13-25: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset the Eval permission latch after successful loads.

This latch only clears on failure. After the first successful fetch, later session changes will keep awaiting the old resolved promise instead of refetching Eval permissions, so access can stay stale until a hard reload.

Suggested fix
 async function loadEvalPermissions() {
-    const userStore = useUserStore()
-    const existingPermissions = userStore.userInfo?.permissions ?? []
-    const { get } = useFetch()
-    const apiUrl = import.meta.env.VITE_API_URL
-    const evalPerms = await get(`${apiUrl}loggedInUser/permissions?prefix=SVMSecure.Eval`)
-    if (evalPerms.success && Array.isArray(evalPerms.result)) {
-        userStore.setPermissions([...existingPermissions, ...evalPerms.result])
-    } else {
-        // Reset latch so the next navigation retries the fetch
+    try {
+        const userStore = useUserStore()
+        const existingPermissions = userStore.userInfo?.permissions ?? []
+        const { get } = useFetch()
+        const apiUrl = import.meta.env.VITE_API_URL
+        const evalPerms = await get(`${apiUrl}loggedInUser/permissions?prefix=SVMSecure.Eval`)
+        if (evalPerms.success && Array.isArray(evalPerms.result)) {
+            userStore.setPermissions([...existingPermissions, ...evalPerms.result])
+        }
+    } finally {
         evalPermissionsPromise = null
     }
 }
🤖 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 `@VueApp/src/Effort/router/index.ts` around lines 13 - 25, The
loadEvalPermissions function leaves the global latch evalPermissionsPromise set
after a successful fetch, causing later session changes to reuse the old
resolved promise and never refetch Eval permissions; update loadEvalPermissions
(and the success branch where userStore.setPermissions is called) to clear/reset
evalPermissionsPromise (set it to null) after the successful update so
subsequent navigations will trigger a fresh fetch for Eval permissions.
🤖 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 `@VueApp/src/Effort/pages/DeptSummary.vue`:
- Line 168: Remove the empty <style scoped> block in DeptSummary.vue (the empty
<style scoped> at the end of the file) — simply delete that empty tag pair; also
apply the same trivial cleanup to the corresponding empty <style scoped> in
MeritSummary.vue so neither file contains an unused empty scoped style block.

In `@VueApp/src/Effort/pages/MeritSummary.vue`:
- Line 148: Remove the now-empty <style scoped> block in the MeritSummary.vue
component (the leftover <style scoped></style> after migrating imports to
report-tables.css); simply delete that empty style tag so the no-op compile step
is removed and nothing else in the component is changed.

In `@VueApp/src/layouts/EmulationBanner.vue`:
- Around line 9-26: Replace the raw q-banner with the app's StatusBanner
component: remove the <q-banner> usage and instead render <StatusBanner
type="warning"> with the emulation text (using userStore.userInfo.firstName and
userStore.userInfo.lastName) in the default slot and move the End Emulation
<q-btn> (using the existing clearEmulationHref, no-caps, dense,
color="secondary", class="text-white q-px-sm q-ml-md") into the `#action` slot;
ensure the v-if="userStore.isEmulating" is applied to the StatusBanner and drop
redundant props like dense/rounded/inline-actions since StatusBanner manages
styling and ARIA semantics internally.

---

Outside diff comments:
In `@VueApp/src/ClinicalScheduler/assets/schedule-shared.css`:
- Around line 18-23: Update the scoped heading rules to use rem units instead of
px: in the .clinical-scheduler-container h3 rule change font-size: 16px → 1rem,
margin-top: 20px → 1.25rem, and margin-bottom: 10px → 0.625rem; apply the same
px→rem conversions to the other two scoped heading selectors mentioned in the
review so all typography and spacing use rem (e.g., 16px→1rem, 18px→1.125rem,
20px→1.25rem).

In `@VueApp/src/Effort/router/index.ts`:
- Around line 13-25: The loadEvalPermissions function leaves the global latch
evalPermissionsPromise set after a successful fetch, causing later session
changes to reuse the old resolved promise and never refetch Eval permissions;
update loadEvalPermissions (and the success branch where
userStore.setPermissions is called) to clear/reset evalPermissionsPromise (set
it to null) after the successful update so subsequent navigations will trigger a
fresh fetch for Eval permissions.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 23d60fc1-4699-4438-a8a4-48c39393093d

📥 Commits

Reviewing files that changed from the base of the PR and between 2580a27 and 2b9ea83.

📒 Files selected for processing (77)
  • VueApp/src/CAHFS/assets/cahfs.css
  • VueApp/src/CAHFS/router/index.ts
  • VueApp/src/CMS/assets/cms.css
  • VueApp/src/CMS/router/index.ts
  • VueApp/src/CTS/pages/CourseList.vue
  • VueApp/src/CTS/pages/MyAssessmentCharts.vue
  • VueApp/src/CTS/router/index.ts
  • VueApp/src/CTS/types/index.ts
  • VueApp/src/ClinicalScheduler/assets/schedule-shared.css
  • VueApp/src/ClinicalScheduler/components/ScheduleView.vue
  • VueApp/src/ClinicalScheduler/components/WeekCell.vue
  • VueApp/src/ClinicalScheduler/composables/use-schedule-normalization.ts
  • VueApp/src/ClinicalScheduler/constants/permission-messages.ts
  • VueApp/src/ClinicalScheduler/constants/schedule-config.ts
  • VueApp/src/ClinicalScheduler/pages/ClinicianScheduleView.vue
  • VueApp/src/ClinicalScheduler/pages/RotationScheduleView.vue
  • VueApp/src/ClinicalScheduler/router/index.ts
  • VueApp/src/ClinicalScheduler/services/clinician-service.ts
  • VueApp/src/ClinicalScheduler/services/error-transformer.ts
  • VueApp/src/ClinicalScheduler/services/instructor-schedule-service.ts
  • VueApp/src/ClinicalScheduler/services/page-data-service.ts
  • VueApp/src/ClinicalScheduler/services/permission-service.ts
  • VueApp/src/ClinicalScheduler/stores/permissions-utils.ts
  • VueApp/src/ClinicalScheduler/stores/permissions/index.ts
  • VueApp/src/ClinicalScheduler/stores/schedule-actions.ts
  • VueApp/src/ClinicalScheduler/stores/schedule-cache.ts
  • VueApp/src/ClinicalScheduler/stores/schedule-state.ts
  • VueApp/src/ClinicalScheduler/stores/schedule.ts
  • VueApp/src/ClinicalScheduler/types/api-responses.ts
  • VueApp/src/ClinicalScheduler/types/index.ts
  • VueApp/src/ClinicalScheduler/types/rotation-types.ts
  • VueApp/src/ClinicalScheduler/utils/schedule-update-helpers.ts
  • VueApp/src/Computing/router/index.ts
  • VueApp/src/Effort/components/InstructorPercentEditDialog.vue
  • VueApp/src/Effort/composables/use-effort-type-columns.ts
  • VueApp/src/Effort/composables/use-percentage-form.ts
  • VueApp/src/Effort/pages/ClinicalEffort.vue
  • VueApp/src/Effort/pages/DeptSummary.vue
  • VueApp/src/Effort/pages/EvalDetail.vue
  • VueApp/src/Effort/pages/EvalSummary.vue
  • VueApp/src/Effort/pages/MeritAverage.vue
  • VueApp/src/Effort/pages/MeritDetail.vue
  • VueApp/src/Effort/pages/MeritSummary.vue
  • VueApp/src/Effort/pages/MultiYearReport.vue
  • VueApp/src/Effort/pages/SchoolSummary.vue
  • VueApp/src/Effort/pages/TeachingActivityGrouped.vue
  • VueApp/src/Effort/pages/TeachingActivityIndividual.vue
  • VueApp/src/Effort/router/index.ts
  • VueApp/src/Effort/services/harvest-service.ts
  • VueApp/src/Effort/types/dashboard-types.ts
  • VueApp/src/Effort/types/evaluation-types.ts
  • VueApp/src/Effort/types/harvest-types.ts
  • VueApp/src/Effort/types/index.ts
  • VueApp/src/Effort/types/report-types-extended.ts
  • VueApp/src/Effort/types/report-types.ts
  • VueApp/src/Effort/types/verification-types.ts
  • VueApp/src/Students/EmergencyContact/pages/EmergencyContactForm.vue
  • VueApp/src/Students/components/PhotoGallery/PhotoGrid.vue
  • VueApp/src/Students/components/PhotoGallery/PhotoList.vue
  • VueApp/src/Students/router/index.ts
  • VueApp/src/Students/services/photo-gallery-service.ts
  • VueApp/src/components/icons/IconCommunity.vue
  • VueApp/src/components/icons/IconDocumentation.vue
  • VueApp/src/components/icons/IconEcosystem.vue
  • VueApp/src/components/icons/IconSupport.vue
  • VueApp/src/components/icons/IconTooling.vue
  • VueApp/src/composables/DateFunctions.ts
  • VueApp/src/composables/QuasarConfig.ts
  • VueApp/src/composables/validation-error.ts
  • VueApp/src/config/colors.ts
  • VueApp/src/layouts/EmulationBanner.vue
  • VueApp/src/layouts/FooterLinks.vue
  • VueApp/src/layouts/ViperBrandButton.vue
  • VueApp/src/layouts/ViperFooter.vue
  • VueApp/src/layouts/ViperLayout.vue
  • VueApp/src/layouts/ViperLayoutSimple.vue
  • VueApp/src/shared/createSpaRouter.ts
💤 Files with no reviewable changes (25)
  • VueApp/src/ClinicalScheduler/stores/schedule-state.ts
  • VueApp/src/CTS/pages/MyAssessmentCharts.vue
  • VueApp/src/CTS/pages/CourseList.vue
  • VueApp/src/CTS/types/index.ts
  • VueApp/src/components/icons/IconSupport.vue
  • VueApp/src/Students/components/PhotoGallery/PhotoList.vue
  • VueApp/src/ClinicalScheduler/stores/schedule-cache.ts
  • VueApp/src/ClinicalScheduler/stores/schedule.ts
  • VueApp/src/components/icons/IconTooling.vue
  • VueApp/src/Students/components/PhotoGallery/PhotoGrid.vue
  • VueApp/src/ClinicalScheduler/stores/schedule-actions.ts
  • VueApp/src/ClinicalScheduler/stores/permissions/index.ts
  • VueApp/src/Effort/composables/use-effort-type-columns.ts
  • VueApp/src/ClinicalScheduler/types/index.ts
  • VueApp/src/Effort/components/InstructorPercentEditDialog.vue
  • VueApp/src/ClinicalScheduler/services/page-data-service.ts
  • VueApp/src/Effort/types/evaluation-types.ts
  • VueApp/src/components/icons/IconCommunity.vue
  • VueApp/src/components/icons/IconDocumentation.vue
  • VueApp/src/Effort/types/report-types-extended.ts
  • VueApp/src/components/icons/IconEcosystem.vue
  • VueApp/src/Effort/types/harvest-types.ts
  • VueApp/src/Effort/types/verification-types.ts
  • VueApp/src/Effort/types/report-types.ts
  • VueApp/src/Effort/types/index.ts

Comment thread VueApp/src/Effort/pages/DeptSummary.vue
Comment thread VueApp/src/Effort/pages/MeritSummary.vue
Comment thread VueApp/src/layouts/EmulationBanner.vue
@rlorenzo rlorenzo force-pushed the quality/analyzer-bulk-cleanup branch from 2580a27 to 6b500eb Compare May 7, 2026 14:26
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch 2 times, most recently from aaada02 to d26c5d8 Compare May 7, 2026 15:38
@rlorenzo rlorenzo force-pushed the quality/analyzer-bulk-cleanup branch from 6b500eb to f9e9c1a Compare May 7, 2026 15:57
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from d26c5d8 to 8cbfb69 Compare May 7, 2026 15:57
@rlorenzo rlorenzo changed the base branch from quality/analyzer-bulk-cleanup to Development May 7, 2026 19:11
@rlorenzo rlorenzo changed the base branch from Development to main May 7, 2026 19:15
@rlorenzo rlorenzo changed the base branch from main to quality/analyzer-bulk-cleanup May 7, 2026 19:15
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from 8cbfb69 to 0d62a4e Compare May 7, 2026 19:24
rlorenzo added 6 commits May 8, 2026 22:14
- Bulk-applied via `jb cleanupcode --profile=OptimizeUsings` then
  `dotnet format`. Removes unused usings and sorts/groups remaining
  ones — compiler-equivalent, no behavior change.
- Adds Viper.sln.DotSettings with OptimizeUsings, ShortenReferences,
  and RemoveRedundancies profiles for this and follow-up PRs.
- Reproduction note: cleanupcode deadlocks on MSBuild's shared
  compiler server while the dev server runs; set
  DOTNET_USE_COMPILER_SERVER=0 to bypass.
- Bulk-applied via `jb cleanupcode --profile=ShortenReferences` then
  `dotnet format`. Replaces inline `Namespace.Type` with imported
  `Type` (or `using Alias = Namespace.Type;` for ambiguous names).
  Compiler-equivalent — no behavior change.
- PhotoExportService.cs gets the largest single diff because it
  juggles four DocumentFormat.OpenXml sub-namespaces with conflicting
  type names (`Picture`, `Paragraph`, `Run`, etc.); aliases at the
  top replace the inline full qualifiers throughout.
- Drop dead `?.` and `!= null` guards in 10 files where the analyzer
  proves the value is non-null (DI-injected helpers, catch
  parameters, values guaranteed by preceding null-checks).
- ViteProxyHelpers.cs: suppress CA1508 with `#pragma warning disable`
  on the inner check of a double-checked locking pattern. The outer
  guard may have raced; CA1508's dataflow analysis doesn't model
  thread interleaving, so this is a known false positive.
- Strip empty `<param name="X"></param>` tags whose corresponding
  parameter doesn't exist (or where the tag adds no doc value).
  Removes ~24 InvalidXmlDocComment findings.
- Fix malformed XML: escape literal `<T>` as `&lt;T&gt;` in three
  doc comments (HttpHelper, IamApi, UinformService); promote two
  `// <summary>` typos in IamApi to `///`; demote orphan `///`
  block on Program.cs SetAwsCredentials (top-level local function
  doesn't accept doc comments) to plain comment.
- Rename three orphan `<param>` tags to match real parameters
  (UserHelper.HasPermission, RAPSSecurityService.IsAllowedTo,
  VMACSExport.ExportToInstances).

Partial-coverage cases (methods with descriptive `<param>` tags
for some but not all parameters) intentionally left alone — their
existing documentation has real value, and selectively adding
empty tags or deleting good ones would be a net loss.
Fixes the 29 PossibleMultipleEnumeration findings flagged by ReSharper.
Each call site enumerated an IEnumerable two-or-more times (e.g.,
`source.Any()` then `source.First()`, or `source.Sum(a)` then
`source.Sum(b)`). Materialising once with `.ToList()` is semantically
equivalent and avoids re-evaluating deferred queries.

- Test files: cast assertion targets to lists so xUnit's `Assert.NotEmpty`
  + `Assert.Equal(N, x.Count())` doesn't iterate twice.
- VueTableDefault.cs: `data`, `skipColumns`, `altColumnNames` are
  enumerated 3+ times per helper and 3 times across helpers from
  `InvokeAsync`. Materialise at both layers.
- EvaluationPolicyService.IsRequiredFor*: `rotationWeeks` is enumerated
  by `.Any()`, `.FirstOrDefault(currentWeek)`, and `.FirstOrDefault(nextWeek)`.
  Materialise at top of method.
- CMSContentController.GetContentBlockByFn: switch `Any()/First()`
  to `Count == 0` / indexer.
- CliniciansController.FilterCliniciansByPermissions: rename param to
  `cliniciansSource`, materialise once into `clinicians`.
- Smaller fixes in RotationsController, EvaluationReportService.
- Bulk-applied via `jb cleanupcode --profile=RemoveRedundancies`
  then `dotnet format`. Removes redundant argument default values,
  redundant member initializers, redundant `else` after `return`,
  redundant anonymous-type property names, and other rewrites in
  the ReSharper "Remove code redundancies" category — all
  compiler-equivalent, no behavior change.
- Updates `Viper.sln.DotSettings`: the umbrella
  `<CSRemoveCodeRedundancies>` flag alone is a no-op. ReSharper
  also requires the (sparsely-documented) sibling
  `<RemoveCodeRedundancies>` flag plus per-rule sub-flags
  (`<CSRemoveRedundantArgumentDefaultValues>`,
  `<CSRemoveRedundantInitializers>`) to actually trigger the
  rewrites.
- Fixes a minor follow-up: `CliniciansController.cs:569` was
  changed to `clinicians.Count` (List property) from
  `clinicians.Count()` (LINQ). The IEnumerable to List
  materialisation in `9823a4c9` made `clinicians` a List, which
  Sonar S2971 then flagged as preferring the property. No new
  lint rule violations introduced by this commit.
@rlorenzo rlorenzo force-pushed the quality/analyzer-bulk-cleanup branch from f9e9c1a to 55cbcab Compare May 9, 2026 05:22
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from 0d62a4e to a0c55c4 Compare May 9, 2026 05:22
@rlorenzo rlorenzo force-pushed the quality/analyzer-bulk-cleanup branch from 55cbcab to 7f36555 Compare May 9, 2026 17:19
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from a0c55c4 to d8a3735 Compare May 9, 2026 17:27
rlorenzo added 2 commits May 11, 2026 09:27
- Real cleanups across CMS / ClinicalScheduler / Effort / RAPS / Students:
  drop dead null-checks Roslyn flow analysis can prove unreachable,
  collapse a duplicated return path, simplify the cache lookup that
  was capturing a never-assigned outer variable, fix the stale XML doc
  param tag left behind by the IEnumerable materialisation rename,
  remove a virtual call from a DTO constructor, and split a SqlCommand
  object initializer outside the using statement.
- Replace anonymous-type byte[]? gymnastics in PhotoExportService with
  a named record so CodeQL stops flagging the casts as redundant.
- Gate now supports `--exclude-rule` and ships defaults for rules that
  fire false positives on ASP.NET Core / EF surfaces (DTO accessors
  bound at runtime by JSON / MVC, record positional properties used
  via record pattern Equals, and the EF nav-property NRT contract that
  Roslyn intentionally distrusts because Include() can be missing).
The UserHelper.GetByLoginId cache populator doesn't need the
ICacheEntry parameter; using `_` silences the new ReSharper
UnusedParameter.Local finding the previous commit introduced
when it collapsed the closure.
@rlorenzo rlorenzo force-pushed the quality/analyzer-bulk-cleanup branch from 7f36555 to 918ce04 Compare May 11, 2026 16:28
rlorenzo added 5 commits May 11, 2026 09:29
- Delete 13 unused files: Vite-starter icon components, an
  orphan footer layout, two abandoned CTS pages, a refactor-
  leftover percent-edit dialog, two unused photo-gallery
  variants, and CSS assets with zero imports.
- Drop @quasar/extras and @pinia/testing from package.json
  after verifying zero usages. Future SVG-icon migration is
  tracked separately in PLAN-svg.md.
- Remove unused exports (and the now-dead local declarations
  that supported them) from colors.ts, DateFunctions.ts,
  QuasarConfig.ts, photo-gallery-service.ts, use-percentage-
  form.ts, error-transformer.ts, permission-messages.ts, and
  schedule-config.ts. colors.ts in particular collapses from
  256 lines to 31 after the dead palette plumbing is removed.
- Fallow's `percentRule` and SCHEDULE_LABELS/SCHEDULE_MESSAGES
  findings are false positives (direct imports and dynamic
  imports fallow missed) — kept as-is.
- Delete schedule.ts + schedule-{actions,cache,state}.ts: a 549-line
  state-management refactor landed in Sep 2025 ("Phase 4 - Frontend
  state management refactoring") was never wired in. `useScheduleStore`
  has zero importers anywhere in the codebase; RotationScheduleView and
  ClinicianScheduleView talk to services directly.
- Delete stores/permissions/index.ts: shadowed by sibling
  stores/permissions.ts (TypeScript resolves the .ts file before the
  directory index), making it structurally unreachable.
- Drop 5 redundant named exports from use-schedule-normalization.ts
  (normalizeSemesterWeeks, filterExcludedRotations, getAssignedRotationNames,
  normalizeClinicianSchedule, normalizeRotationSchedule). All five are
  only accessed via the useScheduleNormalization() composable; the
  module-level duplicates had no importers. normalizeWeek and
  normalizeScheduleSemesters stay exported — tests import them directly.
- Drop 42 dead re-exports from Effort/types/index.ts barrel: types
  passed through but no consumer imported them via the barrel.
- Drop 15 types each from report-types.ts and report-types-extended.ts
  and 9 from harvest-types.ts: row/group intermediates used only as
  property types within the same module; now file-local.
- Drop 8 service response types from api-responses.ts and parallel
  cleanups in instructor-schedule-service.ts, clinician-service.ts,
  ClinicalScheduler/types/index.ts, rotation-types.ts.
- Drop singletons: PageInitialData, PermissionErrorType,
  PermissionsGetters, UpdateRotationParams, EffortColumnOptions,
  HarvestProgressEvent, DataHygieneSummaryDto, EvalCourseInfoDto,
  EmailFailure, ValidationErrors, 3 CTS types.
- Fix BreadCrumb duplicate-export across ViperLayout.vue and
  ViperLayoutSimple.vue by making the local-only type file-private
  in both.
- WeekAssignment, WeekItem, ViewMode dropped to file-local in their
  component files.
- UserInfo stays exported — TypeScript requires it externally
  nameable because ProfilePic.vue's script-setup exposes store
  values typed with it.
Replace style-block @import url() with script-setup side-effect
imports across 15 .vue files. Vite extracts shared CSS chunks
instead of inlining the file into each consumer's stylesheet, and
fallow/jscpd can now follow the dependency graph (the two CSS
files no longer surface as "unused"). Behavior-preserving for
report-tables.css and compact-form.css since their selectors are
already class-prefixed.

For schedule-shared.css, the bare h2/h3 element selectors are
re-scoped under .clinical-scheduler-container so the rules keep
their previous reach now that the file applies globally.

Drop the redundant colors.css @import in WeekCell.vue —
bootstrap-spa.ts already loads colors via styles/index.css for
every SPA.
Seven SPA routers (CAHFS, CMS, CTS, ClinicalScheduler, Computing, Effort,
Students) duplicated the same createRouter + VITE_VIPER_HOME + scroll-top
+ useRouteFocus setup. Hoist into src/shared/createSpaRouter.ts so each
router only owns its bespoke beforeEach guard. No behavior change.
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from d8a3735 to ddd64e7 Compare May 11, 2026 16:29
Extract EmulationBanner, FooterLinks, and ViperBrandButton from the
ViperLayout/ViperLayoutSimple pair so both Vue layouts render identical
header badges, emulation indicator, and footer links from a single
source.

Apply the same pattern on the Razor side: introduce an EmulationBanner
ViewComponent used by _VIPERLayout and VIPERLayoutSimplified.

Use a raw q-banner (matching the Razor markup) in the Vue
EmulationBanner and align .mainLayoutViperMode spacing with site.css so
the environment badge sits at the same height on both stacks.
@rlorenzo rlorenzo force-pushed the refactor/dead-code-and-shared-chrome branch from ddd64e7 to 66fe537 Compare May 12, 2026 02:33
@rlorenzo rlorenzo requested a review from bsedwards May 13, 2026 03:09
@rlorenzo
Copy link
Copy Markdown
Contributor Author

@bsedwards This is a bulk of the dead code cleanup.

@rlorenzo rlorenzo force-pushed the quality/analyzer-bulk-cleanup branch from a350ca9 to 282bb6d Compare May 14, 2026 17:06
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