Flush pending auto-saves before publishing a card - #3039
Open
a-abdellatif98 wants to merge 1 commit into
Open
Conversation
Cmd/Ctrl+Enter went straight to the publish button: Stimulus keyboard filters only match when no modifier is held, so the title textarea's keydown.enter->auto-save#submit action never fired, and the title only reached the server when the form controller disconnected during the navigation that follows publishing. The board then rendered the card as "Untitled", and the late PATCH recorded a phantom card_title_changed event plus its system comment. The clicker now awaits the auto-save controller through an outlet, and auto-save#submit resolves once any in-flight save has landed, not just the one it starts itself.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a regression test and updates the keyboard-shortcut publish flow to ensure the last typed card title is persisted before publishing.
Changes:
- Added a system test validating Ctrl+Enter publishing uses the latest typed title and does not emit a title-changed event.
- Wired the publish buttons’ keyboard shortcut handler to trigger the card form auto-save before clicking publish.
- Updated the auto-save controller to expose an awaitable “in-flight save” via
#saving.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/system/card_creation_test.rb | New system test covering shortcut-based publish behavior and event leakage regression |
| app/views/cards/container/footer/_create.html.erb | Connects keyboard shortcut click handler to the card form auto-save outlet |
| app/javascript/controllers/clicker_controller.js | Awaits auto-save submission (if present) before clicking the target button |
| app/javascript/controllers/auto_save_controller.js | Tracks in-flight save promise to allow callers to await ongoing saves |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
18
to
24
| async submit() { | ||
| if (this.#dirty) { | ||
| await this.#save() | ||
| this.#save() | ||
| } | ||
|
|
||
| await this.#saving | ||
| } |
| #save() { | ||
| this.#resetTimer() | ||
| await submitForm(this.element) | ||
| this.#saving = submitForm(this.element) |
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
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.
Fixes #2778.
Creating a card with Cmd/Ctrl+Enter right after typing the title publishes it as "Untitled" on the board. Reloading shows the real title.
Root cause
Two things combine.
The title textarea has
keydown.enter->auto-save#submit:prevent, but Stimulus keyboard filters require an exact modifier match:So
keydown.enteris skipped whenever meta or ctrl is held. No blur fires either, because focus stays in the textarea, and the 3sAUTOSAVE_INTERVALtimer has not elapsed yet.Meanwhile
clicker#clickpublishes immediately, and the publishbutton_tois a separate form that carries onlycreation_type. The title lives in#card_formand is never part of that request.The title therefore only reaches the server from
auto_save_controller#disconnect, which runs while Turbo is already swapping in the board page. The card is published withtitle == nil,Card#set_default_titlewrites "Untitled", and the board renders that. The late PATCH then renames the card, which also leaves debris:Card::Eventable#track_title_changerecords acard_title_changedevent plus its system comment and webhook delivery.Fix
clickerawaits the auto-save controller through an outlet before clicking publish.auto_save_controller#submitnow resolves once any in-flight save has landed, not only one it started itself. Previously it returned immediately when#dirtywas false, so if the 3s timer had already fired and a PATCH was in flight, an external "flush now" caller would not actually wait. Storing the promise in#savingunconditionally meanssubmit()has one code path whether it just started a save, one was already running, or nothing ever ran, sinceawait undefinedresolves immediately.Relationship to #2812 and #2895
Same outlet wiring, which is the only place the two async handlers can be sequenced. Two deliberate differences:
finally. That is wrong when two saves overlap: the first save'sfinallynulls out the second save's promise, so a subsequentsubmit()returns without waiting. Storing#savingunconditionally with no reset avoids that.CardsController#updateto insert asleep 0.5. That is not needed. On main the PATCH is only sent fromdisconnect(), which by construction runs after the publish request, so the failure is already deterministic. Dropping the alias also avoids leaking a patched controller if teardown is skipped.Testing
New system test, written first and confirmed failing on unmodified code:
The failure screenshot showed the board card titled "Untitled", confirming the mechanism rather than a selector problem. The test also asserts no
card_title_changedevent is left behind, which pins the phantom-event half of the bug.After the fix:
Known gaps
ctrl+enteris exercised. Themeta+enterandctrl/meta+shift+entervariants share identical wiring but have no coverage.