Skip to content

NMS-20107: versioned group management API and PrimeVue Manage Groups page - #8714

Open
joseanesONMS wants to merge 13 commits into
developfrom
jira/NMS-20107-manage-groups-primevue
Open

NMS-20107: versioned group management API and PrimeVue Manage Groups page#8714
joseanesONMS wants to merge 13 commits into
developfrom
jira/NMS-20107-manage-groups-primevue

Conversation

@joseanesONMS

Copy link
Copy Markdown
Contributor

NMS-20107: a versioned group management API (/api/v2/groups) and a PrimeVue Manage Groups page that is a straight visualization of it; independent companion to #8698. groups.xml stays the system of record via the existing GroupManager, with member order (the notification escalation order) and default-map preserved end to end.

  • Endpoints: list/get/create/update/rename/delete, admin-only via new Spring Security rules and in-code checks.
  • The Admin group cannot be deleted or renamed, enforced server-side (the legacy page only hid the buttons).
  • Renames follow the on-call roles' membership-group references in the same file write, deletes are rejected while a role still references the group, and both go through GroupService so the DB category authorizations are migrated or cleared as the legacy page did.
  • Overnight duty schedules are rejected for new entries because the runtime never matches them; stored strings, hand-edited comments and stale members are grandfathered so files never become uneditable.
  • The page validates fields inline, shows API rejections inside the dialogs, and includes a collapsed in-context help panel.
  • 19 integration tests (mock managers, groups.xml never touched) plus Vitest store/dialog tests; verified end to end including hand-editing groups.xml between API calls.

…page

Adds /api/v2/groups (list/get/create/update, rename, delete), admin-only
via Spring Security rules plus in-code checks, wrapping GroupManager so
groups.xml stays the system of record. The member list order is
preserved end to end — it drives the notification escalation order —
and fields the API does not expose (default-map) survive updates.
Requests are validated up front (name and comment markup, members must
exist, duty schedule grammar with overnight ranges legal) and applied to
a detached copy, so rejected requests leave no partial state. Unlike the
legacy page, which only hid the buttons, the Admin group is protected
server-side, renames follow the on-call roles' membership-group
references, and deletes are rejected while roles still reference the
group instead of leaving dangling references.

The Manage Groups page (ui/#/admin/groups) visualizes the API: groups
table, editor with an ordered member list (move up/down), rename and
delete dialogs with the system group disabled. The menu entry now points
at the new page. 13 integration tests and 8 store tests.
Fixes from self-review. Duty schedules with begin after end are rejected
for new entries — DutySchedule.isInSchedule compares within one calendar
day, so an overnight range never matches and would silently take the
group off duty; strings already stored on the record keep round-tripping
so hand-edited files never make a group uneditable. Delete and rename go
through GroupService so the DB category authorizations are cleared and
migrated as the legacy page did (deleting a group otherwise leaked its
authorizations to a future group reusing the name). Renames re-point the
on-call roles in memory first, so the rename's single save persists
groups and roles together, with the role changes reverted on failure.
Both v2 services now serialize on a shared monitor because user
mutations cascade into GroupManager. Dot-segment names are rejected, and
clearing the comments field works (an explicit empty string clears; an
omitted key still preserves).

Adds a collapsed "?" help panel explaining what groups are for and how
to use the page, including the escalation-order and rename semantics.
… dialogs

Same treatment as the users page: group mutations return the server's error
message and the editor/rename dialogs show it inline instead of relying on a
toast hidden behind the modal; the group name and comments fields get live
client-side checks mirroring the server rules, and groups with names the
security filter cannot address in a URL path show a file-managed note in
place of row actions.
A hand-edited groups.xml can reference a user that no longer exists; since
the editor always sends the full member list, the unknown-user check made
such groups uneditable. Members already stored on the group now skip the
existence check, matching the duty-schedule and on-call-schedule handling;
members new to a request must still resolve to a real user.
The name and comments regexes now scan for forbidden characters instead of
full-string matches a newline could bypass. Hand-edited comments containing
markup characters are grandfathered while unchanged — server-side and in
the editor — so such groups stay editable; changed comments are still
validated. The rename rollback no longer re-points on-call roles in memory
when the rename already persisted to groups.xml (only the DB category
migration failed), which would have diverged memory from the file; that
case now reports the partial failure explicitly.
@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@marshallmassengill could you take a review pass when you get a chance?

@joseanesONMS
joseanesONMS requested a review from synqotik July 30, 2026 20:51
…uHeaderIT

The menu entry now lands on the Vue page, so the smoke test waits for its
page title instead of the legacy JSP breadcrumb.
@joseanesONMS
joseanesONMS force-pushed the jira/NMS-20107-manage-groups-primevue branch from deec164 to 02faec4 Compare July 31, 2026 01:24
Swap direct PrimeVue for the Onms-XXX seam wrappers across the groups
table and its editor/rename dialogs: Button->OnmsButton (text/outlined
mapped to variant), Dialog->OnmsDialog, InputText->OnmsInputText,
Select->OnmsSelect, DataTable->OnmsTable, Column->OnmsColumn, Tag->OnmsTag.
Message and IftaLabel have no wrapper yet and stay on PrimeVue. No
behaviour change.
@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@synqotik @marshallmassengill — ready for review. This page now uses the @opennms/onms-ui Onms-XXX wrappers instead of direct PrimeVue (Button, Dialog, InputText, Select, DataTable, Column, Tag). Message and IftaLabel have no wrapper yet, so they stay on PrimeVue. pnpm lint is clean (ran lint:fix), and the vue-tsc build and the UI test suite pass. No behaviour change.

…sDialog width prop

Icon-only buttons (add member, move up/down, remove) now use OnmsIconButton
with icon components instead of OnmsButton with a fallthrough icon="pi ...";
dialog sizing routes through the OnmsDialog width prop (min(Npx, 95vw)) rather
than :style. Both defeat the seam by riding $attrs into PrimeVue.

@marshallmassengill marshallmassengill 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.

One blocker: A failed rename reports the wrong failure and leaves a phantom rename in memory. GroupManager.renameGroup removes the old key, calls grp.setName(newName), puts the new key, and only then calls saveGroups() (GroupManager.java:476-488). If that save throws, memory holds the new name while groups.xml still holds the old one, and update() will not re-read because the file's mtime never changed. The catch block then evaluates m_groupManager.hasGroup(newName) && !m_groupManager.hasGroup(name) against that memory, takes it as proof the file rename succeeded, and raises "The group was renamed, but migrating its category authorizations failed; review the group's authorized categories" while skipping the role rollback. Nothing was persisted, the operator is pointed at the wrong thing, and every read serves the phantom rename plus the repointed roles until restart. The condition needs to distinguish "the file write succeeded" from "the in-memory map was mutated", and the failure path needs a reload from disk.

GroupManager.renameGroup mutated the in-memory map (removed the old key, set
the new name, put the new key) before calling saveGroups(). If that save threw,
memory held the rename while groups.xml still held the old name, and the
mtime-gated update() would never re-read. GroupsRestService then read that
phantom through hasGroup(), mistook it for a persisted rename, reported the
wrong failure, and skipped its role rollback -- serving the phantom plus the
repointed roles until restart.

Roll the in-memory rename back when the save fails, so hasGroup() only ever
reports what groups.xml actually received and the caller's rollback runs on the
real failure. New GroupManagerRenameTest covers both the rolled-back and the
committed paths.
Drop the nested primevue IftaLabel from every field for FormField's own
label/for/required/error props, and replace the direct primevue Message error
banners with a themed role=alert element, so the dialogs no longer import any
un-wrapped PrimeVue primitive. Field errors now render through FormField (the
test selectors follow the FormField-generated error ids).
@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@marshallmassengill Addressed the blocker:

A failed rename reports the wrong failure and leaves a phantom rename in memory. Fixed at the root in GroupManager.renameGroup. It was mutating the in-memory map before saveGroups(), so a failed save left memory holding the rename while groups.xml still held the old name — and the catch's hasGroup(newName) && !hasGroup(name) check read that phantom as proof the file rename had landed, raised the category-authorization error, and skipped the role rollback.

The fix makes the in-memory rename atomic: if saveGroups() throws, the map is rolled back to the old name before rethrowing. hasGroup() now only ever reports what groups.xml actually received, so the REST condition genuinely distinguishes "the file write succeeded" from "the map was mutated," and the role rollback runs on the real failure. I went with atomic rollback rather than a disk reload because update() is mtime-gated (the very reason the phantom survived) — rolling back at the source keeps memory equal to disk without depending on it. New GroupManagerRenameTest covers both the rolled-back and committed paths; GroupsRestServiceIT rename tests stay green.

Also brought the dialogs onto the current @opennms/onms-ui seam while here: dropped the nested IftaLabel for FormField's own label/for/error, and replaced the primevue/message banners with a themed role=alert element — the ManageGroups dialogs now import no un-wrapped PrimeVue primitive. eslint clean, vitest + vue-tsc build pass.

Drop the JAXB annotations from GroupDto and GroupRenameRequest so they
serialize as plain camelCase JSON (users, dutySchedules, newName) instead of
the hyphenated/singular XML element names, and carry that through the UI type,
service, dialogs, table, and tests. The groups.xml mapping already lives in the
REST service. Also swap the hand-rolled isBlank/trimToNull helpers for
org.apache.commons.lang3.StringUtils, matching the users service.
@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@synqotik Applied the same camelCase-JSON convention you asked for on #8713 here too, for consistency across the admin pages:

  • camelCase JSON, no XML — dropped the JAXB annotations from GroupDto and GroupRenameRequest, so they serialize as plain camelCase JSON (users, dutySchedules, newName) instead of the hyphenated/singular XML element names. The groups.xml mapping already lives in the REST service.
  • UI camelCaseui/src/types/groupAdmin.ts and everything downstream (service, store, dialogs, table, tests) follow.
  • Swapped the hand-rolled isBlank/trimToNull for org.apache.commons.lang3.StringUtils, matching the users service.

Verified: full GroupsRestServiceIT (19) green with the new wire format, UI eslint clean, 24 vitest, vue-tsc build passes.

…default template too

menu-template-default.json must stay identical to menu-template.json.
Mirror the Manage Users pattern: replace the inline About panel with an
Info-icon button in the table header that opens the help copy in an OnmsDialog
(shared AboutDialogButton), and collapse the per-row buttons to inline Edit and
Delete plus a overflow OnmsMenu carrying Rename. Protected-group disabling and
the file-managed note for unaddressable names are unchanged.
@joseanesONMS

Copy link
Copy Markdown
Contributor Author

Mirrored the confirmed Manage Users patterns here, and applied the menu fix:

  • Help — inline About panel replaced with an Info-icon button in the table header that opens the help in an OnmsDialog, reusing the shared Common/AboutDialogButton.vue.
  • Row actions — inline Edit and Delete plus a overflow OnmsMenu carrying Rename. Protected-group disabling (Admin) and the file-managed note are preserved.
  • menu-template-default.json — pointed the Manage Groups entry at ui/index.html#/admin/groups so it stays identical to menu-template.json (same fix as NMS-20106: versioned user management API and PrimeVue Manage Users page #8713).

eslint clean, full vitest (2191) green, vue-tsc build passes. (AboutDialogButton.vue is shared with #8713; it'll dedupe cleanly when both land on develop.)

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.

2 participants