Skip to content

NMS-20124: point the notification bell at the new Notifications page - #8719

Open
joseanesONMS wants to merge 1 commit into
jira/NMS-20100from
jira/NMS-20124-bell-notifications
Open

NMS-20124: point the notification bell at the new Notifications page#8719
joseanesONMS wants to merge 1 commit into
jira/NMS-20100from
jira/NMS-20124-bell-notifications

Conversation

@joseanesONMS

@joseanesONMS joseanesONMS commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tickets: NMS-20124 (epic NMS-20100).

Points the top-bar notification bell at the new Notifications page instead of the legacy JSPs, rebased onto the epic.

  • Bell entries deep-link to the notices page with a query preset (outstanding / team-outstanding views).
  • The preset re-applies when it changes without remounting the page.
  • Carries the supporting notices store and service changes.

@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@marshallmassengill review requested — stacks on #8709 (only the top commit is new here).

@joseanesONMS
joseanesONMS force-pushed the jira/NMS-20124-bell-notifications branch from 3cca45a to 00941d5 Compare July 31, 2026 19:33
@joseanesONMS
joseanesONMS requested a review from synqotik August 3, 2026 13:59
@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@marshallmassengill @synqotik — ready for review. Since the PR was opened:

  • Individual bell entries now open the new page (your-outstanding preset) instead of detail.jsp.
  • A bell click while already on the page re-applies the preset via a route.query.preset watch (a hash-query change doesn't remount the route).
  • excludeNotifiedUser now marks the criteria distinct (the usersNotified LEFT_JOIN otherwise duplicated notices and inflated the count) and ignores a blank value instead of 500ing.
  • Dark-mode text fix for the page and its dialogs.

Lint clean, UI suite green (2451 passed), backend module builds.

Integration note: the CSV export (fetchForExport) landed on #8709 after this branch cut, so it isn't here yet. When this rebases onto the base that has it, the export call must carry excludeUser for the team preset — otherwise a team-preset export silently widens to all users.

@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@marshallmassengill @synqotik — dark-mode fix for the notification dialogs. PrimeVue's dialog chrome (panel/header/title) doesn't follow the dark tokens in this build, so the panel stayed white while the content and fields went dark — the earlier text-only fix then left light text on a white panel (no contrast). The notification dialog classes never land on the teleported .p-dialog root, so this targets .p-dialog directly to pin the surface dark and force the title, field labels and header icons light. Verified in a running instance across all four config dialogs.

Heads up: it's a deliberately broad .p-dialog override (with !important to beat PrimeVue's runtime-injected styles), scoped to load with the Notifications page. Happy to relocate it to a global theme file if you'd rather it live app-wide.

Base automatically changed from jira/NMS-20100-notifications-page to jira/NMS-20100 August 5, 2026 10:52
Comment thread opennms-webapp-rest/127.0.0.1.tm0.epoch Outdated

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.

Do not check in this file! These are temporary files.

Comment thread opennms-webapp-rest/tmlog0.log Outdated

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.

Do not check in this file! These are temporary files.

@@ -1,42 +1,43 @@
<template>

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.

Use OnmsDialog, OnmsTab, etc. here.

<div class="general-tab">
<div class="status-toggle">
<OnmsToggleSwitch
<PToggleSwitch

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.

Use OnmsToggleSwitch

</div>

<OnmsTable
<DataTable

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.

Use OnmsTable

const guarded = /^[=+\-@\t\r]/.test(value) ? `'${value}` : value
return `"${guarded.replaceAll('"', '""')}"`
}
const csv = [

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.

Use composables/useDownload here as much as possible.

const headers = Object.keys(rows[0])
const escapeHtml = (value: string) =>
value.replaceAll('&', '&amp;').replaceAll('<', '&lt;').replaceAll('>', '&gt;')
const html = `<!DOCTYPE html>

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.

Is there a way to make this a separate Vue component vs. inline html? We should avoid big HTML strings.

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.

Use OnmsIcon

@@ -1,2 +1,2 @@
<template>
<TableCard class="notification-queries-bar">

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.

Use OnmsCard

</div>
<div class="query-forms">
<div class="search-row">
<IftaLabel>

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.

Use OnmsInputText

<OnmsIconButton
variant="outlined"
:icon="Search"
<Button

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.

Use OnmsButton or OnmsIconButton

Comment thread ui/src/containers/Notifications.vue Outdated
issue shared with the dashboard). The `.open-dark` theme class on <html> IS
reliable, and teleported dialogs live under <html> too — so key explicit
readable colours off it for the notification page and its dialogs/tabs.
-->

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.

This should all be removed, don't want a bunch of custom CSS in individual files.

See if the dark mode issue goes away when you update to latest releaset-36.x or develop and use OnmsDialog, etc. If it doesn't then we should have a separate PR to fix that globally.

@synqotik

synqotik commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@marshallmassengill @synqotik — dark-mode fix for the notification dialogs

@joseanesONMS
Try getting latest release-36.x or develop (may need to first update the target branch), then use all OnmsUI components. If there's still an issue, we should fix it in a separate PR just for that. I think the latest updates I made should fix any dark mode issues, but there still may be some. I don't want a bunch of file-specific custom CSS.

@joseanesONMS
joseanesONMS force-pushed the jira/NMS-20124-bell-notifications branch from d859680 to 978f1c8 Compare August 5, 2026 14:18
@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@synqotik addressed — rebased onto the current base so the whole diff is now just the bell's own work (the deep-link + teamOutstanding preset):

  • The two temp files are gone, and the migrated OnmsDialog/tabs/table/card come from the base now (your wrapper comments on ConfigureNotificationsDialog / NoticesTable / NotificationQueriesCard resolve against the base's versions — the only bell change left in that card is the "anyone but you" link).
  • Removed the inline html.open-dark / .p-dialog CSS from Notifications.vue. If any dark-mode gap remains after your latest theme updates, I'll do it as a separate global PR rather than file-specific CSS, per your note.
  • Also fixed a real bug I'd flagged: fetchForExport now carries excludeUser, so a teamOutstanding export stays scoped instead of silently widening to everyone.

TableCard → OnmsCard (comment on line 2) is app-wide — 14 consumers — so I'd rather do that as its own cleanup than in the bell PR. Let me know if you'd prefer otherwise.

@joseanesONMS
joseanesONMS force-pushed the jira/NMS-20124-bell-notifications branch 2 times, most recently from 259b672 to c59c2e9 Compare August 5, 2026 14:27
@joseanesONMS
joseanesONMS force-pushed the jira/NMS-20124-bell-notifications branch 2 times, most recently from 933ae39 to 60feb3a Compare August 7, 2026 20:09
@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@synqotik @marshallmassengill Ready for another look. Rebased onto the epic (now 36.0.3-SNAPSHOT on release-36.x); the notices container now also hosts the About-Notices dialog and Configure button that arrived with the shared epic changes. Build, lint and store tests are green locally; CI is running.

@joseanesONMS
joseanesONMS requested a review from synqotik August 7, 2026 20:31
Deep-link the top-bar notification bell to the notices page with a query
preset so the outstanding/team views open directly, re-applying the preset
when it changes without remounting. Carries the supporting notices store and
service changes.
@joseanesONMS
joseanesONMS force-pushed the jira/NMS-20124-bell-notifications branch from 60feb3a to 4930198 Compare August 11, 2026 13:52
@joseanesONMS

Copy link
Copy Markdown
Contributor Author

@synqotik @marshallmassengill Updated and green on the latest epic. Temp files are gone and the container carries the About-Notices dialog + Configure button from the epic. The broader onms-ui wrapper suggestions (OnmsSearchInput/OnmsTabs/OnmsCard, inline-HTML extraction) are the routed-pages restructure, which lands as its own follow-up PR. combined-builds is passing.

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