fix(flow-editor): stop Save from silently doing nothing - #608
Merged
Conversation
Closes #607. Three defects combined so that pressing Save could do nothing at all, with no way for the user to find out. INITIALISE BEFORE THE NETWORK. `emptyFlow()` has `name: ''`, and only `open('new')` supplies the default — but `open()` ran at the TAIL of `load()`, behind `await GET /api/flows`, a flow LIST a blank flow does not need. Meanwhile the sidebar was already rendered and Save already enabled, because `nodeCatalog` had been filled by the index page and this store is a singleton that survives the route change. Saving in that window posts `name: ""`, which `FlowController::create()` answers 400 "A flow needs a name." — 9 of 10 attempts against a fresh instance. A blank flow needs nothing from the server, so it is now opened first, and NOT re-opened afterwards. Re-opening was the second defect: the late `open('new')` reset `this.flow`, so a step placed during the window vanished from the canvas, and a save landing after it stored `nodes: []`. That is silent data loss, and in openregister's e2e it also made the suite pass while asserting nothing — the flow it saved was empty, so it could not fail on its contents. `id === 'new'` only, never `id === null`. `save()` calls `load({ app })` to refresh the list, and a null id has always meant "reload the list, leave the open flow alone"; treating it as blank would reset the flow immediately after storing it and discard the id the server had just returned. A test pins that. RENDER THE FAILURE. `store.error` was set on every rejected save and run and displayed nowhere, so a refusal looked exactly like success: the button flickered and nothing else happened. No server log line either — a 400 JSONResponse is not an exception. There is now an error card, preferring the API's own sentence over axios's, because "A flow needs a name." says what to do and "Request failed with status code 400" does not. Save is also disabled while the flow has no name, so the state the API refuses is one the UI stops offering. Every test was checked against the unfixed code: the two race tests and the three visibility tests fail without their fix, and the two that pin the guards — a null-id reload, and an error card absent when nothing failed — pass either way, which is what makes them worth keeping. Full suite: 6032 tests, 510 suites, green.
Contributor
Quality Report — ConductionNL/nextcloud-vue @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ⏭️ | ||||
| phpcs | ⏭️ | ||||
| phpmd | ⏭️ | ||||
| psalm | ⏭️ | ||||
| phpstan | ⏭️ | ||||
| phpmetrics | ⏭️ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| test | ✅ | ||||
| check-build | ✅ | ||||
| check-docs | ✅ | ||||
| check-jsdoc | ✅ | ||||
| check-integration-parity | ✅ | ||||
| check-peers | ✅ | ||||
| check-docs-fresh | ✅ | ||||
| check-a11y | ✅ | ||||
| composer | ✅ | ✅ | |||
| npm | ✅ | ✅ 531/531 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ | ||||
| Hydra gates | ⏭️ |
Quality workflow — 2026-08-06 11:15 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
to ConductionNL/openregister
that referenced
this pull request
Aug 6, 2026
Carries ConductionNL/nextcloud-vue#608, which closes #607: the flow editor no longer lets a user press Save before the store has a flow to save. The window was real and wide. `emptyFlow()` has `name: ''`, only `open('new')` supplies the default, and `open()` ran behind `await GET /api/flows` — a list a blank flow does not need — while the sidebar was already rendered and Save already enabled. Saving in that window posted `name: ""` and the API answered 400 "A flow needs a name." A refused save rendered nothing at all, so the user saw the button flicker and no more. The same late `open('new')` also reset the flow, wiping a step already placed on the canvas. Verified against this bundle, in the window itself: clicking Save 120ms after opening a blank flow — where a 400 was previously reproducible 9 times in 10 — returns 201 with `name: "New flow"` and the route advances to the new uuid. `vue` is pinned back to ^3.5.18 by hand again. `npm install` rewrites it to ^3.5.0 to match the new package's own range, which contradicts the `overrides` entry, and CI's npm 10.8.2 refuses that with EOVERRIDE while local npm 11 accepts it silently. `npx npm@10.8.2 ci --dry-run` is clean with the range restored. Worth automating; for now it is a hand check on every bump of this package.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #607.
Three defects combined so that pressing Save could do nothing at all, with no way for the user to find out.
1. The editor was interactive before it was initialised
emptyFlow()hasname: '', and onlyopen('new')supplies the default — butopen()ran at the tail ofload(), behindawait GET /api/flows, a flow list that a blank flow does not need. Meanwhile the sidebar was already rendered and Save already enabled, becausenodeCataloghad been filled by the index page and the store is a singleton that survives the route change.Saving in that window posts
name: "", whichFlowController::create()answers 400 "A flow needs a name." — 9 of 10 attempts against a fresh instance.A blank flow needs nothing from the server, so it is now opened first.
2. Re-opening threw the user's work away
The late
open('new')resetthis.flow, so a step placed during the window vanished from the canvas, and a save landing after it storednodes: []. Silent data loss — and in openregister's e2e it also made the suite pass while asserting nothing, because the flow it saved was empty and so could not fail on its contents.The blank flow is no longer re-opened at the tail.
id === 'new'only, neverid === nullsave()callsload({ app })to refresh the list, and a null id has always meant "reload the list, leave the open flow alone". Treating it as blank would reset the flow immediately after storing it and discard the id the server had just returned. A test pins that.3. A refused action was invisible
store.errorwas set on every rejected save and run and rendered nowhere, so a refusal looked exactly like success: the button flickered and nothing else happened. No server log line either — a 400JSONResponseis not an exception.There is now an error card, preferring the API's own sentence over axios's: "A flow needs a name." says what to do; "Request failed with status code 400" does not.
Save is additionally disabled while the flow has no name, so the state the API refuses is one the UI stops offering.
Tests, checked in both directions
The last two pin the guards rather than the fixes, which is why they are worth keeping.
Full suite: 6032 tests, 510 suites, green.
Not addressed here
The issue's fourth point —
load()refetching both catalogues on every call, ~6 redundant requests per save — is a performance change with a different blast radius and is left for its own PR.