Skip to content

VPR-64 feat(phone): schoolwide and unit phone lists - #323

Merged
bniedzie merged 8 commits into
mainfrom
feature/VPR-64-phone-lists
Sep 1, 2026
Merged

VPR-64 feat(phone): schoolwide and unit phone lists#323
bniedzie merged 8 commits into
mainfrom
feature/VPR-64-phone-lists

Conversation

@bniedzie

@bniedzie bniedzie commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

This PR migrates the schoolwide and Dean's Office phone lists from Viper 1. Viewing the lists requires only basic permissions, while specific permissions allow users to edit and maintain the lists. The lists are now housed in the new Personnel area.

The migration makes the following functional changes from the Viper 1 version:

  • Rather than using the PhoneList database, creates a phones schema in the VIPER database.
  • The database structure is different, normalizing the existing data. Phone is now stored at a person level, meaning that the same person will have the same data every time they occur across lists. Name data now comes from the user.Person table, which more easily handles name changes. The primary person identifier is now IAM ID rather than Mothra ID, per an initial meeting with Brandon.
  • Change history is now tracked, rather than showing only the latest changes. This fixes a bug where deletions would not affect the last modified date shown to users.
  • A new SVMSecure.PhoneLists.SVMMaintain permission guards editing the SVM list. In Viper 1, anyone with the link could make changes.
  • Removed supervisor from the VMDO data, as it is not surfaced anywhere. This can be pulled from existing UCPath data if needed at a later date.
  • Generalized the VMDO table to support the creation of arbitrary department/unit phone lists. These are keyed by a code (e.g., VMDO), allowing the list name to change without breaking links. Each allows separate permissions. VMDO is the only list currently present.
  • Changed the general Phone List view to display direct numbers to those with list maintenance permissions as well. Previously, these users could view the data on the maintenance page only.
  • Fixed a broken link in the general Phone List view.
  • Changed phone lists to be filterable and (for relevant fields) sortable.

This PR also does some refactoring around Person selection and dialog boxes. There should be no end user impact to CMS, but a few files are affected.

This PR requires schema changes to the Production database:

CREATE SCHEMA phones;


CREATE TABLE [phones].[Person](
	[PersonIam] [VARCHAR](10) NOT NULL,
	[Phone] [NVARCHAR](25) NOT NULL,
	[ModifiedDate] [DATETIME] NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[DirectPhone] [NVARCHAR](25) NULL,
	[Office] [NVARCHAR](100) NULL,
 CONSTRAINT [PK_Person] PRIMARY KEY CLUSTERED 
(
	[PersonIam] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];


CREATE TABLE [phones].[PhoneList](
	[PhoneListId] [INT] IDENTITY(1,1) NOT NULL,
	[Name] [NVARCHAR](100) NOT NULL,
	[MaintainRole] [VARCHAR](100) NOT NULL,
	[Code] [NVARCHAR](20) NOT NULL,
 CONSTRAINT [PK_PhoneList] PRIMARY KEY CLUSTERED 
(
	[PhoneListId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

CREATE UNIQUE NONCLUSTERED INDEX [UX_PhoneList_Code] ON [phones].[PhoneList]
(
	[Code] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY];


CREATE TABLE [phones].[PhoneListUnit](
	[PhoneListUnitId] [INT] IDENTITY(1,1) NOT NULL,
	[PhoneListId] [INT] NOT NULL,
	[Name] [NVARCHAR](100) NOT NULL,
	[SortOrder] [INT] NULL,
 CONSTRAINT [PK_PhoneListUnit] PRIMARY KEY CLUSTERED 
(
	[PhoneListUnitId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[PhoneListUnit]  WITH CHECK ADD  CONSTRAINT [FK_PhoneListUnit_PhoneListId] FOREIGN KEY([PhoneListId])
REFERENCES [phones].[PhoneList] ([PhoneListId]);
ALTER TABLE [phones].[PhoneListUnit] CHECK CONSTRAINT [FK_PhoneListUnit_PhoneListId];


CREATE TABLE [phones].[PhoneListUnitPerson](
	[PhoneListUnitPersonId] [INT] IDENTITY(1,1) NOT NULL,
	[PhoneListUnitId] [INT] NOT NULL,
	[PersonIam] [VARCHAR](10) NOT NULL,
	[ListFirst] [BIT] NOT NULL,
	[IsActive] [BIT] NOT NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[ModifiedDate] [DATETIME] NULL,
 CONSTRAINT [PK_PhoneListUnitPerson] PRIMARY KEY CLUSTERED 
(
	[PhoneListUnitPersonId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[PhoneListUnitPerson] ADD  DEFAULT ((0)) FOR [ListFirst];
ALTER TABLE [phones].[PhoneListUnitPerson] ADD  DEFAULT ((1)) FOR [IsActive];
ALTER TABLE [phones].[PhoneListUnitPerson]  WITH CHECK ADD  CONSTRAINT [FK_PhoneListUnitPerson_PersonIam] FOREIGN KEY([PersonIam])
REFERENCES [phones].[Person] ([PersonIam]);
ALTER TABLE [phones].[PhoneListUnitPerson] CHECK CONSTRAINT [FK_PhoneListUnitPerson_PersonIam];
ALTER TABLE [phones].[PhoneListUnitPerson]  WITH CHECK ADD  CONSTRAINT [FK_PhoneListUnitPerson_PhoneListUnitId] FOREIGN KEY([PhoneListUnitId])
REFERENCES [phones].[PhoneListUnit] ([PhoneListUnitId]);
ALTER TABLE [phones].[PhoneListUnitPerson] CHECK CONSTRAINT [FK_PhoneListUnitPerson_PhoneListUnitId];


CREATE TABLE [phones].[SVMSection](
	[SectionId] [INT] IDENTITY(1,1) NOT NULL,
	[Name] [NVARCHAR](100) NULL,
	[IncludeAbbrv] [BIT] NOT NULL,
	[DirectorTitle] [NVARCHAR](50) NOT NULL,
	[SortOrder] [INT] NULL,
	[UnitName] [NVARCHAR](50) NULL,
 CONSTRAINT [PK_SVMSection] PRIMARY KEY CLUSTERED 
(
	[SectionId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];


CREATE TABLE [phones].[SVMUnit](
	[UnitId] [INT] IDENTITY(1,1) NOT NULL,
	[SectionId] [INT] NOT NULL,
	[Name] [NVARCHAR](100) NOT NULL,
	[Abbrv] [NVARCHAR](20) NULL,
	[SortOrder] [INT] NULL,
	[Fax] [NVARCHAR](25) NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[ModifiedDate] [DATETIME] NULL,
 CONSTRAINT [PK_SVMUnit] PRIMARY KEY CLUSTERED 
(
	[UnitId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[SVMUnit]  WITH CHECK ADD  CONSTRAINT [FK_SVMUnit_SectionId] FOREIGN KEY([SectionId])
REFERENCES [phones].[SVMSection] ([SectionId]);
ALTER TABLE [phones].[SVMUnit] CHECK CONSTRAINT [FK_SVMUnit_SectionId];


CREATE TABLE [phones].[SVMUnitPerson](
	[UnitPersonId] [INT] IDENTITY(1,1) NOT NULL,
	[UnitId] [INT] NOT NULL,
	[PersonIam] [VARCHAR](10) NOT NULL,
	[ModifiedDate] [DATETIME] NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[Office] [NVARCHAR](50) NULL,
	[PosType] [NVARCHAR](25) NULL,
	[Interim] [NVARCHAR](10) NULL,
	[IsActive] [BIT] NOT NULL,
 CONSTRAINT [PK_SVMUnitPerson] PRIMARY KEY CLUSTERED 
(
	[UnitPersonId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[SVMUnitPerson] ADD  DEFAULT ((1)) FOR [IsActive];
ALTER TABLE [phones].[SVMUnitPerson]  WITH CHECK ADD  CONSTRAINT [FK_SVMUnitPerson_PersonId] FOREIGN KEY([PersonIam])
REFERENCES [phones].[Person] ([PersonIam]);
ALTER TABLE [phones].[SVMUnitPerson] CHECK CONSTRAINT [FK_SVMUnitPerson_PersonId];
ALTER TABLE [phones].[SVMUnitPerson]  WITH CHECK ADD  CONSTRAINT [FK_SVMUnitPerson_UnitId] FOREIGN KEY([UnitId])
REFERENCES [phones].[SVMUnit] ([UnitId]);
ALTER TABLE [phones].[SVMUnitPerson] CHECK CONSTRAINT [FK_SVMUnitPerson_UnitId];


CREATE TABLE [phones].[SVMFrequentNumber](
	[NumberID] [INT] IDENTITY(1,1) NOT NULL,
	[Label] [NVARCHAR](100) NOT NULL,
	[Phone] [NVARCHAR](25) NOT NULL,
	[SortOrder] [INT] NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[ModifiedDate] [DATETIME] NULL,
	[IsActive] [BIT] NOT NULL,
 CONSTRAINT [PK_SVMFrequentNumber] PRIMARY KEY CLUSTERED 
(
	[NumberID] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[SVMFrequentNumber] ADD  DEFAULT ((1)) FOR [IsActive];

This PR requires creating a new permission on Production: SVMSecure.PhoneLists.SVMMaintain.

This PR requires running the migration script .\RunMigrateData.bat Production for a dry run, and then .\RunMigrateData.bat Production --apply to migrate data into the new schema.

This PR will require a change to Viper 1 redirecting two Personnel left nav items and adding the SVMMaintain permission check to one.

@codecov-commenter

codecov-commenter commented Aug 25, 2026

Copy link
Copy Markdown

Bundle Report

Changes will increase total bundle size by 50.14kB (2.19%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
viper-frontend-esm 2.34MB 50.14kB (2.19%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: viper-frontend-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/GenericError-*.css 164 bytes 208.06kB 0.08%
assets/GenericError-*.js 13.04kB 58.03kB 28.99% ⚠️
assets/schedule-*.js 8 bytes 55.03kB 0.01%
assets/SortableList-*.js 8 bytes 45.54kB 0.02%
assets/PhotoGallery-*.js 8 bytes 36.16kB 0.02%
assets/InstructorList-*.js -32 bytes 26.04kB -0.12%
assets/effort-*.js -5 bytes 23.76kB -0.02%
assets/Files-*.js -1.77kB 20.8kB -7.86%
assets/ContentBlockEdit-*.js -32 bytes 20.74kB -0.15%
assets/MultiYearReport-*.js 8 bytes 18.81kB 0.04%
assets/EmergencyContactForm-*.js -32 bytes 17.2kB -0.19%
assets/CmsHome-*.js 8 bytes 11.43kB 0.07%
assets/ViperFetch-*.js 53 bytes 11.22kB 0.47%
assets/SVMPhonesMaintain-*.js (New) 10.54kB 10.54kB 100.0% 🚀
assets/SVMFrequentNumberTable-*.js (New) 10.15kB 10.15kB 100.0% 🚀
assets/ReportFilterForm-*.js 8 bytes 8.87kB 0.09%
assets/RecordActionCell-*.js (New) 5.95kB 5.95kB 100.0% 🚀
assets/PhoneListMaintain-*.js (New) 5.61kB 5.61kB 100.0% 🚀
assets/ClinicalEffort-*.js 8 bytes 5.33kB 0.15%
assets/PhoneListUnitTable-*.js (New) 5.01kB 5.01kB 100.0% 🚀
assets/ManageCourseCompetencies-*.js 3 bytes 3.71kB 0.08%
assets/RecordFormDialog-*.js (New) 3.69kB 3.69kB 100.0% 🚀
assets/students-*.js -5 bytes 3.5kB -0.14%
assets/dist-*.js (Deleted) -11.57kB 0 bytes -100.0% 🗑️
assets/ExportToolbar-*.js 8 bytes 2.95kB 0.27%
assets/SVMPhones-*.js (New) 2.7kB 2.7kB 100.0% 🚀
assets/PhoneList-*.js (New) 2.12kB 2.12kB 100.0% 🚀
assets/personnel-*.js (New) 2.05kB 2.05kB 100.0% 🚀
assets/PersonSelector-*.js (New) 1.79kB 1.79kB 100.0% 🚀
assets/CmsContent-*.js (New) 462 bytes 462 bytes 100.0% 🚀
assets/Home-*.js -343 bytes 221 bytes -60.82%
assets/Home-*.js (New) 221 bytes 221 bytes 100.0% 🚀
assets/RecordActionCell-*.css (New) 215 bytes 215 bytes 100.0% 🚀
assets/SVMPhones-*.css (New) 110 bytes 110 bytes 100.0% 🚀

Files in assets/Files-*.js:

  • ./src/CMS/components/FileFormDialog.vue → Total Size: 150 bytes

  • ./src/CMS/components/PersonSelector.vue → Total Size: 150 bytes

Files in assets/personnel-*.js:

  • ./src/Personnel/App.vue → Total Size: 112 bytes

@codecov-commenter

codecov-commenter commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.84712% with 153 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.40%. Comparing base (c6f64b5) to head (1ed3786).

Files with missing lines Patch % Lines
...s/Personnel/Controllers/PhoneListUnitController.cs 69.13% 23 Missing and 2 partials ⚠️
...el/Controllers/PhoneSVMFrequentNumberController.cs 50.00% 24 Missing ⚠️
...as/Personnel/Controllers/PhoneSVMUnitController.cs 68.75% 15 Missing ⚠️
VueApp/src/Personnel/composables/svm-data-fetch.ts 92.15% 3 Missing and 5 partials ⚠️
VueApp/src/Personnel/pages/PhoneListMaintain.vue 87.93% 4 Missing and 3 partials ⚠️
...b/Areas/Personnel/Services/PhoneListUnitService.cs 96.56% 2 Missing and 5 partials ⚠️
.../Personnel/components/PhoneListAddRecordDialog.vue 82.35% 5 Missing and 1 partial ⚠️
VueApp/src/Personnel/pages/SVMPhonesMaintain.vue 92.95% 2 Missing and 3 partials ⚠️
VueApp/src/components/PersonSearchSelect.vue 70.58% 4 Missing and 1 partial ⚠️
...src/Personnel/composables/use-add-record-dialog.ts 88.88% 3 Missing and 1 partial ⚠️
... and 24 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
+ Coverage   42.38%   44.40%   +2.01%     
==========================================
  Files         994     1066      +72     
  Lines       49877    51942    +2065     
  Branches     5887     6119     +232     
==========================================
+ Hits        21142    23063    +1921     
- Misses      27798    27906     +108     
- Partials      937      973      +36     
Flag Coverage Δ
backend 41.81% <93.06%> (+1.45%) ⬆️
frontend 63.35% <92.52%> (+4.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...App/src/Personnel/components/MobileSortControl.vue 100.00% <100.00%> (ø)
...ueApp/src/Personnel/components/ModifiedSummary.vue 100.00% <100.00%> (ø)
VueApp/src/Personnel/components/PersonSelector.vue 100.00% <100.00%> (ø)
...ueApp/src/Personnel/components/PhoneListFilter.vue 100.00% <100.00%> (ø)
...pp/src/Personnel/components/RecordActionButton.vue 100.00% <100.00%> (ø)
...eApp/src/Personnel/components/RecordActionCell.vue 100.00% <100.00%> (ø)
...pp/src/Personnel/components/SVMAddRecordDialog.vue 100.00% <100.00%> (ø)
...eApp/src/Personnel/components/SectionJumpLinks.vue 100.00% <100.00%> (ø)
...src/Personnel/composables/phone-list-data-fetch.ts 100.00% <100.00%> (ø)
...App/src/Personnel/composables/svm-phone-columns.ts 100.00% <100.00%> (ø)
... and 69 more

... and 2 files with indirect coverage changes

@rlorenzo

Copy link
Copy Markdown
Contributor

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review skipped: 122 files exceed the limit of 100.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the schoolwide (SVM) and Dean's Office (VMDO) phone lists from Viper 1 into a new Personnel area, backed by a new normalized phones schema in the VIPER database. Viewing requires basic SVMSecure permission, while a new SVMSecure.PhoneLists.SVMMaintain permission (and per-list MaintainRole) gates editing. It adds EF Core models/services/controllers plus a full Vue 3/Quasar SPA, and refactors shared person-search logic into a reusable PersonSearchHelper used by both CMS and Personnel.

Changes:

  • New phones schema + PhonesDbContext, EF models, area services and /api/phones/... controllers with dynamic per-list maintain permissions and direct-number masking.
  • New Personnel Vue SPA (lists, maintenance views, person selector, record dialogs) plus data-migration scripts from the legacy PhoneList database.
  • Shared PersonSearchHelper extracted and adopted by CMS's SearchPeople, forcing EF parameterization (ESCAPE clause) to prevent per-term query plans and %/_ wildcard injection.

Reviewed changes

Copilot reviewed 121 out of 122 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
web/Viper.csproj Excludes Areas\Personnel\Scripts\** (separate migration project) from the web build, mirroring the Effort area.
web/Program.cs Registers PhonesDbContext, adds Personnel SPA name and the Personnel services namespace to Scrutor registration.
web/Classes/Utilities/PersonSearchHelper.cs New shared expression-tree helper for name-search autocomplete with parameterized Contains matching.
web/Areas/Personnel/Services/PhoneSVMSectionService.cs Read-only query for SVM sections, ordered with null-safe sort.
web/Areas/Personnel/Services/PhoneSVMFrequentNumberService.cs CRUD + soft-delete for SVM frequent numbers, with modified-date tracking.
web/Areas/Personnel/Services/PhonePersonLookupService.cs Looks up phone people by IAM IDs (direct number masked unless maintainer) and current-employee search.
web/Areas/Personnel/Services/PhonePermissionsService.cs Resolves edit permission from the list's MaintainRole column.
web/Areas/Personnel/Controllers/PhonePersonController.cs Person-picker endpoint merging Viper and phone data; uses foreach/Add where .Select() is preferred.
web/Areas/Personnel/Controllers/PhoneSVMModifiedDateController.cs Returns latest SVM modified date; contains a comment typo ("Identfies").
web/Areas/Personnel/Models/*, VueApp/src/Personnel/** New EF models/DTOs/Mapperly mapper and the Personnel Vue SPA (services, composables, components, tests).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread web/Areas/Personnel/Controllers/PhonePersonController.cs Outdated
Comment thread web/Areas/Personnel/Controllers/PhoneSVMModifiedDateController.cs Outdated

@rlorenzo rlorenzo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Solid work, and the parts that are easy to get wrong are right: ResolveListForMaintain, VerifyUnitInList, and GetUnitPersonInList each re-scope by list rather than trusting the id in the request, with a test proving one list's role grants nothing on another. I ran the branch against dev, so the inline notes are reproductions. Four things block deployment, none of them in the code:

  1. The DDL won't run. CREATE SCHEMA Inventory; should be phones, so every CREATE TABLE [phones].[...] fails. Four ALTER TABLE [phones].[SVMUnitPerson] CHECK CONSTRAINT statements also name the wrong table, and three run before that table exists.
  2. The DDL is missing the unique index on PhoneList.Code, and dev already has it. UX_PhoneList_Code was added to dev by hand, so Production won't get it and a duplicate code would resolve arbitrarily, including for the permission check.
  3. A permission is missing from the steps. VMDO's MaintainRole is SVMSecure.PhoneLists.VMDOMaintain, but only SVMMaintain is listed, so nobody could maintain VMDO.
  4. The pages aren't reachable from the nav. MainNav.cs:29 and MiniNav/Default.cshtml:105-109 still point Personnel at VIPER 1, though App.vue sets highlighted-top-nav="Personnel".

Also Home.vue needs a personnel-home CMS record per environment, or redirected non-maintainers land on a blank page. Everything else is inline, tagged minor where it's a nit rather than a fix I'd hold the PR for.

Comment thread VueApp/src/Personnel/components/SVMAddRecordDialog.vue Outdated
Comment thread web/Areas/Personnel/Controllers/PhoneSVMUnitController.cs
Comment thread web/Areas/Personnel/Services/PhoneListUnitService.cs
Comment thread web/Areas/Personnel/Services/PhonePersonLookupService.cs
Comment thread VueApp/src/Personnel/composables/use-add-record-dialog.ts Outdated
Comment thread VueApp/src/Personnel/pages/PhoneListMaintain.vue Outdated
Comment thread web/Areas/Personnel/Controllers/PhoneListUnitController.cs Outdated
Comment thread web/Areas/Personnel/Services/PhonePermissionsService.cs Outdated
Comment thread VueApp/src/Personnel/composables/phone-list-data-fetch.ts Outdated
Comment thread VueApp/src/Personnel/components/PersonSelector.vue
@bniedzie

Copy link
Copy Markdown
Contributor Author

Solid work, and the parts that are easy to get wrong are right: ResolveListForMaintain, VerifyUnitInList, and GetUnitPersonInList each re-scope by list rather than trusting the id in the request, with a test proving one list's role grants nothing on another. I ran the branch against dev, so the inline notes are reproductions. Four things block deployment, none of them in the code:

  1. The DDL won't run. CREATE SCHEMA Inventory; should be phones, so every CREATE TABLE [phones].[...] fails. Four ALTER TABLE [phones].[SVMUnitPerson] CHECK CONSTRAINT statements also name the wrong table, and three run before that table exists.
  2. The DDL is missing the unique index on PhoneList.Code, and dev already has it. UX_PhoneList_Code was added to dev by hand, so Production won't get it and a duplicate code would resolve arbitrarily, including for the permission check.
  3. A permission is missing from the steps. VMDO's MaintainRole is SVMSecure.PhoneLists.VMDOMaintain, but only SVMMaintain is listed, so nobody could maintain VMDO.
  4. The pages aren't reachable from the nav. MainNav.cs:29 and MiniNav/Default.cshtml:105-109 still point Personnel at VIPER 1, though App.vue sets highlighted-top-nav="Personnel".

Also Home.vue needs a personnel-home CMS record per environment, or redirected non-maintainers land on a blank page. Everything else is inline, tagged minor where it's a nit rather than a fix I'd hold the PR for.

I've addressed 1 and 2 in the DDL in the description.

3 is a non-issue - SVMSecure.PhoneLists.VMDOMaintain is a legacy permission, present in all environments, and assigned to users with no changes to scope. SVMMaintain is being added to address a legacy security issue.

  1. I've fixed the nav; personnel-home is an existing CMS record on both test and prod, so no changes should be needed.

@bniedzie

Copy link
Copy Markdown
Contributor Author

Tagging @JasonRobertFrancis mostly so he is aware of what I've done in Personnel as part of this task

Comment thread VueApp/vite.config.ts Outdated
Comment thread VueApp/vite.config.ts Outdated
Comment thread web/Views/Shared/Components/MiniNav/Default.cshtml Outdated
Comment thread web/Views/Shared/Components/MiniNav/Default.cshtml Outdated

@rlorenzo rlorenzo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-review of d4aaacb9..dcd328cf. The earlier threads are all resolved; these are new in the delta. Items on PhoneListFilter.vue, SectionJumpLinks.vue, SVMPhones.vue and PhoneListUnitService.cs I'd hold for, the rest are marked minor. The three UI ones reproduced in the browser at 390px.

Comment thread VueApp/src/Personnel/components/PhoneListFilter.vue Outdated
Comment thread VueApp/src/Personnel/components/SectionJumpLinks.vue Outdated
Comment thread VueApp/src/Personnel/pages/SVMPhones.vue Outdated
Comment thread VueApp/src/Personnel/pages/SVMPhonesMaintain.vue Outdated
Comment thread web/Areas/Personnel/Services/PhoneListUnitService.cs Outdated
Comment thread web/Areas/Personnel/Controllers/PhoneSVMUnitController.cs Outdated
Comment thread web/Areas/Personnel/Models/PersonnelMapper.cs Outdated
Comment thread web/Areas/Personnel/PhonesDbContext.cs Outdated
Comment thread test/Personnel/PhoneListUnitControllerTests.cs
Comment thread VueApp/src/Personnel/components/MobileCardList.vue Outdated
@bniedzie
bniedzie requested a review from rlorenzo August 31, 2026 22:59
@bniedzie
bniedzie merged commit e9442e2 into main Sep 1, 2026
13 of 14 checks passed
@bniedzie
bniedzie deleted the feature/VPR-64-phone-lists branch September 1, 2026 22: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.

5 participants