ALEC-298: Remove cloud feedback (gRPC) and data-sharing agreement flow#152
Conversation
The situation feedback pipeline that shipped ACCEPTED/REJECTED/edited situations to an external AlecCollectionService over gRPC is no longer in use. The data-sharing consent toggle (Agreement) existed solely to gate that client, and the first-run welcome/configuration screens were hosting that toggle, so everything comes out together: - Delete gRPC client, config, Proto mappers, situationset.proto, and the wrap/grpc Karaf wrap bundle. - Delete AgreementRest + AgreementRestImpl and the Agreement / Configuration data classes (nothing else referenced them). - Rewire SituationRestImpl to just forward situations to OpenNMS; drop the now-unused kvStore + runtimeInfo constructor args and the destroy() / updateAgreement() plumbing. - Drop AGREEMENT, ACCEPTED_SITUATION, REJECTED_SITUATION from KeyEnum (the latter two were only written by the removed cloud path). - Remove the alec-gateway and grpc.version properties and the wrap/grpc managed dep from the parent POM; drop the corresponding bundle entries from the alec-ui Karaf feature. - UI: delete WelcomePage.vue + ConfigurationPage.vue and their routes, strip savePermission / getUserInfo / allowSave / firstTime / getAlecInfo from AlecService + useUserStore, remove the OPT-IN indicator from ConfigurationInfo and the consent radio from AccountSettings, and stop gating the REJECT button on consent.
There was a problem hiding this comment.
Pull request overview
This PR removes the deprecated “cloud feedback” gRPC pipeline and the associated data-sharing consent (“Agreement”) flow across the backend, Karaf feature wiring, and the Vue UI extension, leaving situation feedback to be forwarded only to OpenNMS.
Changes:
- Removed gRPC wrap module, proto/mappers/client code, and Karaf feature bundles/dependencies that supported cloud feedback.
- Removed Agreement REST API + data classes, and simplified
SituationRestImplconstructor/plumbing accordingly. - Removed welcome/setup UI flow and consent gating from the Vue source (routes, store/service methods, and settings UI).
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wrap/pom.xml | Drops the grpc wrap module from the wrap reactor build. |
| wrap/grpc/pom.xml | Removes the grpc wrap bundle module (shade + OSGi export setup). |
| pom.xml | Removes grpc/alec-gateway properties and the wrap/grpc managed dependency. |
| karaf-features/src/main/resources/features.xml | Removes protobuf/alec-gateway/grpc wrap bundles from the alec-ui feature. |
| features/ui/pom.xml | Drops grpc wrap and alec-gateway dependencies from the UI feature bundle. |
| features/ui/src/main/resources/OSGI-INF/blueprint/blueprint.xml | Removes Agreement REST and gRPC config beans; rewires SituationRestImpl bean args. |
| features/ui/src/main/java/org/opennms/alec/rest/SituationRestImpl.java | Removes cloud storage/gRPC logic; forwards situations only to the datasource/OpenNMS. |
| features/ui/src/main/java/org/opennms/alec/rest/SituationRest.java | Removes updateAgreement(...) from the REST interface. |
| features/ui/src/main/java/org/opennms/alec/rest/AgreementRest.java | Deletes Agreement REST API interface. |
| features/ui/src/main/java/org/opennms/alec/rest/AgreementRestImpl.java | Deletes Agreement REST implementation. |
| features/ui/src/main/java/org/opennms/alec/data/KeyEnum.java | Removes AGREEMENT/ACCEPTED_SITUATION/REJECTED_SITUATION keys. |
| features/ui/src/main/java/org/opennms/alec/data/Agreement.java | Deletes Agreement model interface. |
| features/ui/src/main/java/org/opennms/alec/data/AgreementImpl.java | Deletes Agreement model implementation. |
| features/ui/src/main/java/org/opennms/alec/data/Configuration.java | Deletes configuration aggregate interface that included Agreement. |
| features/ui/src/main/java/org/opennms/alec/data/ConfigurationImpl.java | Deletes configuration aggregate implementation that included Agreement. |
| features/ui/src/main/java/org/opennms/alec/grpc/GrpcConnectionConfig.java | Deletes gRPC connection configuration class. |
| features/ui/src/main/java/org/opennms/alec/grpc/SituationClient.java | Deletes gRPC client used to send situations externally. |
| features/ui/src/main/java/org/opennms/alec/mapper/AlecProtoMapperUtils.java | Deletes timestamp/proto mapping utility used by gRPC payloads. |
| features/ui/src/main/java/org/opennms/alec/mapper/AlarmToAlarmProto.java | Deletes alarm-to-proto mapper used by gRPC payloads. |
| features/ui/src/main/java/org/opennms/alec/mapper/SituationToSituationProto.java | Deletes situation-to-proto mapper used by gRPC payloads. |
| features/ui/src/main/proto/situationset.proto | Deletes the proto definition for the former cloud collection service. |
| features/ui/src/test/java/org/opennms/alec/rest/SituationRestImplTest.java | Updates tests to reflect removal of kvstore/runtimeInfo/gRPC client behavior. |
| features/ui/src/test/java/org/opennms/alec/rest/AgreementRestImplTest.java | Deletes tests for the removed Agreement REST API. |
| features/ui/src/test/java/org/opennms/alec/rest/AgreementImplTest.java | Deletes tests for the removed Agreement data model. |
| features/ui/src/test/java/org/opennms/alec/BlueprintContextTest.java | Removes mocked services related to the deleted gRPC config/runtime info wiring. |
| ui/src/services/AlecService.ts | Removes Agreement endpoints (getUserInfo, savePermission). |
| ui/src/store/useUserStore.ts | Removes consent/first-run state and related actions; keeps engine config actions. |
| ui/src/router/index.ts | Removes welcome/setup routes and first-run redirect logic; routes directly to situations. |
| ui/src/containers/AccountSettings.vue | Removes consent controls and savePermission call from settings UI. |
| ui/src/components/WelcomePage.vue | Deletes the welcome / opt-in page component. |
| ui/src/components/ConfigurationPage.vue | Deletes the first-run engine configuration page component. |
| ui/src/components/ConfigurationInfo.vue | Removes OPT-IN indicator block from the header widget. |
| ui/src/components/SituationDetailTab.vue | Removes consent gating from the REJECT action rendering. |
| ui/src/tests/welcomePage.test.ts | Deletes tests for the removed welcome flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- SituationRestImpl.forwardSituation: wrap body in try/catch Exception and return ALECRestUtils.somethingWentWrong(e) to preserve the structured 5xx response that the previous forwardAndStoreSituation helper provided. - SituationDetailTab.vue: drop the dangling useUserStore() call left over after the consent gate was removed (the import was already deleted, the variable was unused). Note: the shipped Karaf UI bundle under features/ui/src/main/resources/ui-ext/ still contains the removed welcome/configuration flow and needs a rebuild. That rebuild is deferred to a follow-up - pre-existing case-sensitivity bugs in the ui/ sources (axiosinstances.ts vs ./axiosInstances import, TimeLine.vue vs Timeline import) prevent yarn build from succeeding on a case-sensitive filesystem, and resolving those is out of scope for this PR. Assisted-by: Claude Code:Opus 4.7
|
I resolved the copilot review issues. One of them was about rebuilding UI bits. That does need to be done but I don't think we need to fix that in this PR. |
cgorantla
left a comment
There was a problem hiding this comment.
If ALEC was installed and validated that UI works after all the changes. I'm good with these changes.
Also note that this needs to re-target to release-3.x where the next release will go.
If UI breaks with deleted endpoints, we may need to fix that in this PR |
The shipped Karaf UI extension under features/ui/src/main/resources/ui-ext/
still contained the removed welcome/configuration flow because the bundle
had not been regenerated after the cloud-feedback removal. Rebuilding
required first fixing two pre-existing case-mismatch bugs that had been
masked by case-insensitive filesystems (macOS):
- Rename ui/src/services/axiosinstances.ts -> axiosInstances.ts to match
the './axiosInstances' imports in AlarmService, AlecService, and
UserService.
- Rename ui/src/components/TimeLine.vue -> Timeline.vue to match the
'@/components/Timeline.vue' import in SituationMetricsTab.
With those fixed, 'yarn build' (which runs vue-tsc + vite build and
copies dist/* into ui-ext/) now succeeds on a case-sensitive filesystem.
The regenerated alecUiExtension.{es,umd}.js / style.css no longer
reference WelcomePage, ConfigurationPage, getAlecInfo, allowSave,
firstTime, savePermission, getUserInfo, or AGREEMENT.
Assisted-by: Claude Code:Opus 4.7
Should be fixed now with latest commit. Take note of the case sensitive thing for this because it could impact MacOS builds. It shouldn't break anything but it's a change. |
|
Let me know if we're good to merge @cgorantla. Tests passed and UI bits updated - should build on case insensitive OS now too. |
Rebased onto current develop after PR OpenNMS-Plugins#152 (ALEC-298) removed the cloud-feedback gRPC and data-sharing agreement flow. The agreement section that lived in the original commit on this branch is gone — it depended on infrastructure that no longer exists. The rest of the ALEC-296 scope is preserved: UI (ui/src/containers/AccountSettings.vue): - Page header renamed: "Configuration Page" → "Correlation Engine Configuration Page". - Deep Learning option hidden from the engine selector (the bundle is intentionally NOT unregistered: it remains available for tests and regression scenarios; this is a UI-only filter). - New disabled "LLM Based" radio with a "Coming soon" caption as a placeholder for the upcoming engine option. - DBScan correlation variables (alpha, beta, epsilon) editable as numeric inputs visible only when Clustering is selected; wired through useUserStore.setEngineInfo() (now accepts an overrides arg) into the existing /alec/engine/configuration endpoint, which already accepted these values — previously the UI sent only engineName and distanceMeasureName. - Click-toggle help popover next to the variables title with bulleted guidance + default values for each variable. (Feather's tooltip is string-only, hence a popover for bullets / inline code / italics.) - Circular-arrow Restore button to reset variables to defaults in one click. - Two action buttons at the bottom (left of Save Changes), each prompting window.confirm before firing: * "Close All Open Situations" → POST /alec/situation/closeAll * "Re-Evaluate All Open Alarms" → POST /alec/engine/reEvaluate Backend (features/ui): - SituationRest + SituationRestImpl: new POST /alec/situation/closeAll endpoint. Walks every situation that is not already ACCEPTED/REJECTED and forwards an empty-alarm REJECTED situation to OpenNMS — same mechanism the existing per-situation reject path uses. Post-OpenNMS-Plugins#152 the cloud-client (gRPC) sendSituation step the original commit had is intentionally absent. - EngineRest + EngineRestImpl: new POST /alec/engine/reEvaluate endpoint. Calls the existing driverInit() (destroy + initAsync) so the engine re-applies the current correlation variables to all currently active alarms. - driverInit() now returns an error Response on InterruptedException (was: silently returned null and let the caller report 200). Addresses Copilot review feedback from the original PR OpenNMS-Plugins#154. Tests: - UI: 13 vitest specs in src/tests/accountSettings.test.ts covering the new title, Deep-Learning removal, disabled LLM-Based + caption, variables form, save round-trip with alpha/beta/epsilon, help popover toggle (bullets + defaults), reset to defaults, the Clustering-only visibility rule, and both action buttons confirming and dispatching plus their no-op-on-cancel paths. - Backend: extend EngineRestImplTest with a reEvaluate-triggers-init test and an alpha/beta/epsilon-applied-to-factory test; extend SituationRestImplTest with closeAllOpenSituations marking every non-closed situation as REJECTED.
The situation feedback pipeline that shipped ACCEPTED/REJECTED/edited situations to an external AlecCollectionService over gRPC is no longer in use. If we ever needed this, we could pull it back in based on the older code but to be honest, I'm pretty sure there are much better ways to do this now.
The data-sharing consent toggle (Agreement) existed solely to gate that client, and the first-run welcome/configuration screens were hosting that toggle, so everything comes out together:
Assisted-by: Claude Code:Opus 4.6/4.7