-
Notifications
You must be signed in to change notification settings - Fork 61
Fix History Panel bugs and add version mismatch warning in collaborative editor #3941
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
Display warning banner when viewing latest workflow version but selected run was executed on an older version. This prevents user confusion when workflow structure has changed since the run executed (jobs added, removed, or modified). Features: - VersionMismatchBanner component using bg-yellow-100 styling to match other warning elements (VersionDropdown, credential badges) - useVersionMismatch hook encapsulating detection logic - Full-width banner at top of canvas, consistent with LiveView alerts - 8 passing tests covering all edge cases and null safety The banner shows when: - A run is selected from history - Viewing "latest" workflow (not a specific snapshot) - Run's version differs from current workflow version
Fixes two related bugs that prevented the history page from loading when navigating from the collaborative editor: 1. Backend: Fix MatchError crash in humanize_workflow - Handle nil case when Enum.find doesn't find the workflow - Previously crashed with "no match of right hand side value: nil" - Now returns empty string when workflow not found 2. Frontend: Fix workflow ID extraction in MiniHistory - Extract workflow ID from position [1] instead of last element - Previously took "collaborate" instead of the actual workflow UUID - Now works for both /w/:id and /w/:id/collaborate URLs The bug occurred when clicking "View History" from the collaborative editor because the frontend was sending workflow_id="collaborate" which the backend couldn't find, causing a 500 error.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3941 +/- ##
=======================================
Coverage 88.79% 88.79%
=======================================
Files 422 422
Lines 18772 18772
=======================================
+ Hits 16668 16669 +1
+ Misses 2104 2103 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds test case to cover the nil branch when a workflow_id filter references a workflow that doesn't exist in the workflows list.
|
This one is ready for review @stuartc @taylordowns2000 @theroinaochieng |
|
@stuartc I decided to put the version mismatch stuff in a hook to not overcomplicate components. There's also a test to test the logic of the hook. |
taylordowns2000
left a comment
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.
the history navigation link works perfectly, but I'm seeing two bugs in the version matching stuff:
- when going to an old run, it should take you to that version—not keep you on
latestand show the mismatch banner. (play around with this feature on the legacy canvas to see how it should work.) - i'm seeing all the nodes collapse into the middle.
Both of these show up in the video here: https://www.loom.com/share/5fd45ef0e7bf4911a56bf511c03729c4
elias-ba
left a comment
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.
Hey @lmac-1 this is looking great to me from a code point of view. I alkso clicked tested and it looks great. I will leave the UI approvals to Taylor. @taylordowns2000 just wanna flag that the nodes collapsing issue might be out of scope of this work. It has became rare now but it's still happening from time to time in the app on main. I think we may need to have a proper issue for it and do deep diagnostic for it. I do think that it's deeply related to nodes positionning and layout algorithm.
5a92ab4 to
899f337
Compare
|
Hey @taylordowns2000 thanks for the Loom, yes that makes sense, will take a look! I didn't realise that the version should update at the top. And for the nodes collapsing, I have also been seeing it a bit in other bits of dev work that I'm working on in other branches / main, but not sure what are the exact steps to replicate. Seems to be a bit random. As @elias-ba suggested, it might be worth diving deeper into that in a separate issue. Will try to keep an eye out for it though to see what is causing it. |
Description
This PR improves the history panel experience in the collaborative editor by fixing critical bugs that prevented navigation to the "view all history" page and adding a version mismatch warning to prevent user confusion.
Closes #3814 and point 1 of #3940
History page 500 error when navigating from collaborative editor
Feature: Version mismatch warning banner
Shows a warning when viewing latest workflow but selected run was executed on an older version. Prevents confusion when workflow structure has changed since the run executed. Appears when:
Example message:
Testing
Validation steps
Version mismatch warning:
You have two choices
Once you have a workflow where the latest version is different to previous version:
Bug fix:
Additional notes for the reviewer
I tried to make the banner look similar to existing banner in LiveView using Tailwind colours.
AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to know!):
You can read more details in our Responsible AI Policy
Pre-submission checklist
:owner,:admin,:editor,:viewer)