-
Notifications
You must be signed in to change notification settings - Fork 63
Show specific diverged workflows in sandbox merge dialog #4396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add diverged_workflows/2 function to return list of workflow names that have diverged between sandbox and target projects, rather than just a boolean flag. Update merge modal to display these specific workflow names in a bullet list when divergence is detected. This helps users make informed decisions about whether to proceed with a merge by showing exactly which workflows will be affected. Fixes #4001
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4396 +/- ##
==========================================
- Coverage 89.38% 89.36% -0.02%
==========================================
Files 425 425
Lines 20051 20052 +1
==========================================
- Hits 17923 17920 -3
- Misses 2128 2132 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@josephjclark let me know your thoughts on the wording |
- Remove dbg() call from diverged workflows conditional check - Extract duplicate diverged workflows logic into reusable helper function - Simplifies code maintenance by centralizing the workflow divergence check
|
Something is up here Frank - I see the warning even if the branches haven't diverged Divergence means that the parent project changed since the sandbox was forked. So the head version in the parent is not in the sandbox's history. |
| @doc """ | ||
| Returns the list of workflow names that have diverged between source and target projects. | ||
|
|
||
| Compares version hashes for workflows with matching names. A workflow is considered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep:
A workflow is considered
diverged if it exists in both projects but has different version hashes.
this isn't quite right: a workflow is diverged if the parent head is not in the workflow's history.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, great catch!! Let me add tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@josephjclark I have fixed this but I think there is another issue we need to fix. Currently, we're squashing the hashes if the source(cli, app) is the same. This means the workflows will always diverge after making a change because we will squash the "cloned" hash
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, it's bad. We don't clone the versions at all when creating sandbox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually #3958 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, it's bad. We don't clone the versions at all when creating sandbox
I lied, we do clone the versions during forking: https://github.com/OpenFn/lightning/blob/main/lib/lightning/projects/sandboxes.ex#L558
…rison Update divergence detection to properly identify when parent project has changed since fork. A workflow now diverges only if the parent's current HEAD version is absent from the sandbox's version history, not just when the two HEADs differ. This allows sandboxes that have pulled parent updates and moved forward to correctly show no divergence, while still detecting true conflicts where parent and sandbox evolved independently. Changes: - Modify get_workflow_version_hashes_by_name to return all version hashes grouped by workflow name instead of just HEAD hash - Update diverged_workflows/2 to check if parent HEAD exists in sandbox history list - Refactor has_diverged?/2 to delegate to diverged_workflows/2 - Add comprehensive tests validating version history comparison logic
|
@midigofrank I don't want to merge this until we've done the demo release - so I'm gonna pause review today. I'll review all your sandbox PRs together (maybe EOD tomorrow) |
Description
diverged_workflows/2function to return list of workflow names that have diverged between sandbox and target projectsFixes #4001
Validation steps
AI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
You can read more details in our
Responsible AI Policy
Pre-submission checklist
/reviewwith Claude Code)
(e.g.,
:owner,:admin,:editor,:viewer)