Skip to content

Group diff output by process group#76

Merged
sfc-gh-pvillard merged 2 commits into
mainfrom
feature/group-diffs-by-process-group
Jul 8, 2026
Merged

Group diff output by process group#76
sfc-gh-pvillard merged 2 commits into
mainfrom
feature/group-diffs-by-process-group

Conversation

@sfc-gh-pvillard

Copy link
Copy Markdown
Collaborator

Closes #75.

Summary

  • Group headers per process group: each process group involved in changes gets its own **\Root > SubGroup`** — N changes` header, making it immediately clear which group a change belongs to. Groups are sorted alphabetically; the root group comes first, nested groups in path order.
  • (Parameter Contexts) section: parameter-context-level diffs (value changes, parameter adds/removes inside a context) are collected in a dedicated section printed last, separate from structural process group changes.
  • Snapshot A process groups registered: groups from the before-snapshot are now also registered so that components removed from groups that no longer exist in the after-snapshot still display a human-readable path instead of a raw UUID.
  • No change to per-diff message wording: every existing diff description is unchanged; only the grouping and headers are new.
  • Controller service added message simplified: since the group is already shown by the section header, the redundant has been added in Process Group X suffix was removed from the controller service added message.

Example output

#### Flow Changes

**`MyFlow`** — 5 changes
- A Processor of type `GenerateFlowFile` named `Gen` has been added ...
- In Processor of type `UpdateAttribute` named `Update`, the value of property ...

**`MyFlow > Ingestion Group`** — 2 changes
- In Processor of type `InvokeHTTP` named `Call API`, the Run Schedule changed from `1 min` to `* * * * * ?`
- A Processor of type `UpdateAttribute` named `Enrich` has been removed

**`(Parameter Contexts)`** — 1 change
- In the Parameter Context `MyCtx`, the value of the parameter `api.url` has changed from `https://old` to `https://new`

Test plan

  • All 26 existing tests continue to pass (mvn test)
  • testNestedGroupsHaveSeparateSections — verifies the raw diff set contains expected diff types when using the new nested-PG fixtures
  • testGroupedOutputContainsGroupHeaders — verifies group headers appear in the correct order and changes are placed under the right group
  • testSingleGroupOutputHasGroupHeader — verifies single-group flows still produce a group header and no spurious (Parameter Contexts) section

@sfc-gh-dchaffelson sfc-gh-dchaffelson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice one, Pierre — this reads cleanly and lines up with #75 exactly: full-path group headers, the (Parameter Contexts) section pulled out last, Bundle Changes left in its own section, and visual-only diffs still dropped. Moving the bundle/visual handling into the first pass keeps the behaviour identical while making the grouping logic easy to follow, and I like that the before-snapshot groups are now registered so removed components still show a readable path.

Especially appreciated the test coverage — the nested-group fixtures plus asserting header order and that each change lands under the right group is exactly what this kind of output change needs.

Two small things, neither blocking:

  1. Heads-up on a merge conflict with #66. That PR also rewrites FlowDiff.java, so whichever of the two lands first will leave the other needing a rebase. Just a sequencing note since both are yours.
  2. Added/removed process group placement. A diff whose component is a process group resolves to its parent's section (via getGroupIdentifier()), so e.g. "Group A added" shows under Root rather than under Root > Group A. That seems reasonable to me (the group has no section of its own yet) — just flagging to confirm it's what you intended.

Base is a couple of weeks back now, so a quick rebase + CI re-run before merge is probably worth it (also relevant given the #66 overlap). Otherwise LGTM.

Resolves #75. Diffs are now organised under a bold header per process
group path (e.g. **`Root > SubGroup`** — N changes) instead of a
single flat list. Parameter-context-level diffs (value changes, adds,
removes inside a context) are collected under a synthetic
**`(Parameter Contexts)`** section printed last.

- Register snapshot A process groups alongside B's so removed
  components whose group no longer exists in B still get a named path.
- Add buildProcessGroupPath() and getGroupPathForDiff() helpers.
- Refactor executeFlowDiffForOneFlow() into two phases: collect
  (bundles + visual-only handled as before; rest grouped by path) then
  print (group header + inner switch per group).
- Add test fixtures with nested process groups and four new tests.
@sfc-gh-pvillard sfc-gh-pvillard force-pushed the feature/group-diffs-by-process-group branch from ad25b76 to 9b4fc16 Compare July 8, 2026 20:43
@sfc-gh-pvillard

Copy link
Copy Markdown
Collaborator Author

Thanks for the review @sfc-gh-dchaffelson - pushed a commit to address

@sfc-gh-dchaffelson sfc-gh-dchaffelson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving — LGTM. The two notes in my review comment are non-blocking (merge-order heads-up vs #66/#78, and a confirm on process-group-level placement).

@sfc-gh-pvillard sfc-gh-pvillard merged commit 1f1a985 into main Jul 8, 2026
3 checks passed
@sfc-gh-pvillard sfc-gh-pvillard deleted the feature/group-diffs-by-process-group branch July 8, 2026 20:54
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.

Group diff output by process group

2 participants