You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "rfellows (via GitHub)" <gi...@apache.org> on 2026/08/17 18:32:12 UTC
[PR] NIFI-16217 - Fix: Unsaved Parameter Context changes silently lost when navigating to an inherited Parameter Context [nifi]
rfellows opened a new pull request, #11559:
URL: https://github.com/apache/nifi/pull/11559
# NIFI-16217: Prompt to save before navigating to an inherited Parameter Context
## Summary
Navigating from the Parameter Context edit dialog to an inherited parameter previously discarded any unsaved edits without warning. Users could lose local form changes simply by following an inherited parameter link.
This change adds a save-before-navigate flow for that action. When the edit form is dirty, NiFi prompts with **Save**, **Don't Save**, or **Cancel**. Choosing **Save** persists the update and keeps the user on the completed update review so they can explicitly continue with **Go to Parameter**—there is no automatic navigation after save. Clean forms navigate immediately; invalid dirty forms disable **Save** so users can discard and continue or cancel and keep editing.
JIRA: [NIFI-16217](https://issues.apache.org/jira/browse/NIFI-16217)
## Decision flow
```mermaid
flowchart TD
A[User clicks Go to inherited Parameter] --> B{Edit form dirty?}
B -->|No - clean| C[Navigate to inherited Parameter Context]
B -->|Yes - dirty| D{Form valid / saveable?}
D -->|Valid| E[Unsaved Changes dialog<br/>Save / Don't Save / Cancel]
D -->|Invalid| F[Unsaved Changes dialog<br/>Save disabled / Don't Save / Cancel]
E -->|Cancel| G[Stay on current edit dialog]
F -->|Cancel| G
E -->|Don't Save| C
F -->|Don't Save| C
E -->|Save| H[Submit Parameter Context update]
H --> I[Show completed update review<br/>Close + Go to Parameter]
I -->|Close| J[Close dialog - no navigation]
I -->|Go to Parameter| C
note1[No auto-navigate after save]
H -.-> note1
```
## What's Changed
- Intercept navigation to an inherited Parameter from the Parameter Context edit dialog when the form has unsaved changes
- Add an **Unsaved Changes** confirmation dialog with **Save**, **Don't Save**, and **Cancel**
- Disable **Save** (and focus **Don't Save**) when the form is dirty but not in a saveable state
- On **Save**, submit the Parameter Context update and retain pending navigation until the user chooses **Go to Parameter** on the completed update review (no auto-navigate)
- On **Don't Save**, discard pending edits and navigate; on **Cancel**, remain in the current edit dialog
- Carry optional `highlightedParameterName` through open/edit navigation so the target parameter can be highlighted after arrival
- Extend Parameter Context listing NgRx state/effects/selectors for pending post-update navigation
- Add unit coverage for the confirmation dialog, edit-dialog review actions, parameter-table go-to behavior, and listing effects/reducer paths
## Test plan
- [ ] With a clean Parameter Context edit form, click an inherited parameter and confirm navigation proceeds immediately (no prompt)
- [ ] With a dirty valid form, click an inherited parameter and confirm the Unsaved Changes dialog offers Save / Don't Save / Cancel
- [ ] Choose **Cancel** and confirm the edit dialog remains open with unsaved changes retained
- [ ] Choose **Don't Save** and confirm navigation proceeds and pending edits are discarded
- [ ] Choose **Save**, confirm the update completes, and confirm the review actions show **Close** and **Go to Parameter** (no automatic navigation)
- [ ] From the completed review, click **Go to Parameter** and confirm navigation to the inherited Parameter Context (with parameter highlight when applicable)
- [ ] From the completed review, click **Close** and confirm the dialog closes without navigating
- [ ] With a dirty invalid form, click an inherited parameter and confirm **Save** is disabled, messaging explains the form is not saveable, and **Don't Save** / **Cancel** behave as above
- [ ] Confirm a normal Apply/Save without inherited navigation still shows only **Close** on the completed update review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-16217 - Fix: Unsaved Parameter Context changes silently lost when navigating to an inherited Parameter Context [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on PR #11559:
URL: https://github.com/apache/nifi/pull/11559#issuecomment-5358060032
Will review...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-16217 - Fix: Unsaved Parameter Context changes silently lost when navigating to an inherited Parameter Context [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #11559:
URL: https://github.com/apache/nifi/pull/11559#discussion_r3830795100
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/parameter-contexts/state/parameter-context-listing/parameter-context-listing.effects.ts:
##########
@@ -338,6 +351,78 @@ export class ParameterContextListingEffects {
map((parameterContexts) => parameterContexts.filter((pc) => pc.id != parameterContextId))
);
editDialogReference.componentInstance.saving$ = this.store.select(selectSaving);
+ editDialogReference.componentInstance.hasPendingPostUpdateNavigation$ = this.store.select(
+ selectHasPendingPostUpdateNavigation
+ );
+
+ const navigateToInheritedParameter = (
Review Comment:
`navigateToInheritedParameter` and the `goTo` closure below it are fairly large and nested inside this effect. Consider extracting them into named, independently-testable functions for readability — not blocking, just a suggestion.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/parameter-context/index.ts:
##########
@@ -15,10 +15,18 @@
* limitations under the License.
*/
-import { ParameterContextEntity } from '../../../state/shared';
+import { ParameterContextEntity, PostUpdateNavigationState } from '../../../state/shared';
export interface EditParameterContextRequest {
Review Comment:
`EditParameterContextRequest` is declared here identically to the one in `apps/nifi/src/app/pages/parameter-contexts/state/parameter-context-listing/index.ts`. This copy isn't used by any production code — only `edit-parameter-context.component.spec.ts` imports it, for test-fixture typing. Since this PR already touches both files, can we drop this duplicate and have the spec import the type from the state module instead?
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/parameter-contexts/ui/parameter-context-listing/parameter-context-listing.component.ts:
##########
@@ -52,6 +53,7 @@ import { ParameterContextTable } from './parameter-context-table/parameter-conte
})
export class ParameterContextListing implements OnInit {
private store = inject<Store<ParameterContextListingState>>(Store);
+ private router = inject(Router);
Review Comment:
This introduces new `Router`-based logic to read `highlightedParameterName` off navigation state (see the `lastSuccessfulNavigation()` call below), but `parameter-context-listing.component.spec.ts` wasn't updated — there's no test asserting this reads the state and forwards it into `getEffectiveParameterContextAndOpenDialog` correctly. Could we add coverage for this?
Separate question (non-blocking): `lastSuccessfulNavigation()` is read after an async pipeline (`switchMap`/`filter`/`take(1)` gated on the parameter context appearing in the store). If that resolution is delayed and another navigation completes first, could the wrong `highlightedParameterName` get picked up? Worth a second look, even if the impact is only a wrong/missing highlight.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/parameter-contexts/ui/parameter-context-listing/parameter-table/parameter-table.component.ts:
##########
@@ -74,11 +82,15 @@ export class ParameterTable implements AfterViewInit, ControlValueAccessor {
private store = inject<Store<ParameterContextListingState>>(Store);
Review Comment:
This looks like it only ports the parameter-context-editor half of "navigate to and highlight a parameter" — I don't see corresponding changes to `property-table.component.ts` or any parameter-reference-parsing utility that would let a user click a `#{param}` reference inside a processor's/controller service's property value and jump to the highlighted parameter here. Is that intentionally out of scope for this PR, or planned separately? Would help to note it in the PR description either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-16217 - Fix: Unsaved Parameter Context changes silently lost when navigating to an inherited Parameter Context [nifi]
Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on code in PR #11559:
URL: https://github.com/apache/nifi/pull/11559#discussion_r3831863394
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/parameter-context/index.ts:
##########
@@ -15,10 +15,18 @@
* limitations under the License.
*/
-import { ParameterContextEntity } from '../../../state/shared';
+import { ParameterContextEntity, PostUpdateNavigationState } from '../../../state/shared';
export interface EditParameterContextRequest {
Review Comment:
Dropped. One clarification on the premise: this copy was used by production code — `parameter-context-listing.actions.ts` was importing `EditParameterContextRequest` from here, while `edit-parameter-context.component.ts` imported the state-module copy. All three importers (actions, component, spec) now reference `pages/parameter-contexts/state/parameter-context-listing`.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/parameter-contexts/ui/parameter-context-listing/parameter-context-listing.component.ts:
##########
@@ -52,6 +53,7 @@ import { ParameterContextTable } from './parameter-context-table/parameter-conte
})
export class ParameterContextListing implements OnInit {
private store = inject<Store<ParameterContextListingState>>(Store);
+ private router = inject(Router);
Review Comment:
Added coverage in `parameter-context-listing.component.spec.ts`: it forwards `highlightedParameterName` from the navigation state into `getEffectiveParameterContextAndOpenDialog`, dispatches without a highlight when there is no navigation state, and does not dispatch at all until the parameter context is present in the store.
On the timing question — you're right, and I've changed it. `lastSuccessfulNavigation` is a signal and was being read inside `subscribe()`, i.e. after the `switchMap`/`take(1)` that waits for the context to load, so a navigation completing during that wait would have been attributed to this request. It is now read in a `map` immediately after the route id emits, and the captured value is carried through the async wait.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-16217 - Fix: Unsaved Parameter Context changes silently lost when navigating to an inherited Parameter Context [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #11559:
URL: https://github.com/apache/nifi/pull/11559#discussion_r3830795105
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/parameter-contexts/ui/parameter-context-listing/parameter-table/parameter-table.component.ts:
##########
@@ -74,11 +82,15 @@ export class ParameterTable implements AfterViewInit, ControlValueAccessor {
private store = inject<Store<ParameterContextListingState>>(Store);
Review Comment:
This adds highlight/scroll-to support within the parameter context editor, but I don't see corresponding changes to `property-table.component.ts` or any parameter-reference-parsing utility that would let a user click a `#{param}` reference inside a processor's/controller service's property value and jump to the highlighted parameter here. Is that intentionally out of scope for this PR, or planned separately? Would help to note it in the PR description either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-16217 - Fix: Unsaved Parameter Context changes silently lost when navigating to an inherited Parameter Context [nifi]
Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on code in PR #11559:
URL: https://github.com/apache/nifi/pull/11559#discussion_r3831864549
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/parameter-contexts/state/parameter-context-listing/parameter-context-listing.effects.ts:
##########
@@ -338,6 +351,78 @@ export class ParameterContextListingEffects {
map((parameterContexts) => parameterContexts.filter((pc) => pc.id != parameterContextId))
);
editDialogReference.componentInstance.saving$ = this.store.select(selectSaving);
+ editDialogReference.componentInstance.hasPendingPostUpdateNavigation$ = this.store.select(
+ selectHasPendingPostUpdateNavigation
+ );
+
+ const navigateToInheritedParameter = (
Review Comment:
Extracted. They are now `navigateToInheritedParameter(...)` and `goToInheritedParameter(...)` private methods on `ParameterContextListingEffects`, taking the source parameter context id and the edit dialog reference as arguments rather than closing over them. The `openParameterContextDialog$` body is about 50 lines shorter as a result.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/parameter-contexts/ui/parameter-context-listing/parameter-table/parameter-table.component.ts:
##########
@@ -74,11 +82,15 @@ export class ParameterTable implements AfterViewInit, ControlValueAccessor {
private store = inject<Store<ParameterContextListingState>>(Store);
Review Comment:
Intentionally out of scope here. This PR adds the capability — `highlightedParameterName` on the Parameter Context edit route — and consumes it for inherited-parameter navigation within the Parameter Context editor. Wiring the property table's existing link to it (extracting the parameter name from the reference, threading it through `goToParameter`, and resolving the reference through inherited contexts so the highlight lands in the context that actually defines the parameter) is tracked in [NIFI-16240](https://issues.apache.org/jira/browse/NIFI-16240). That is also what the existing TODO in `PropertyTable.canGoToParameter` refers to. I have noted the boundary in the PR description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-16217 - Fix: Unsaved Parameter Context changes silently lost when navigating to an inherited Parameter Context [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman merged PR #11559:
URL: https://github.com/apache/nifi/pull/11559
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org