[AGILE-301] Fix goal text being wiped on date selection in Sprint dialog#23851
Merged
Conversation
Reuses `Sprint#goal_for` when rebuilding the sprint form so a date refresh no longer clears an unsaved goal. Covers the refresh path that previously wiped the field. https://community.openproject.org/wp/AGILE-301
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the Backlogs sprint creation/edit dialog where changing the date range triggers a Turbo Stream form refresh that could discard the typed sprint goal (AGILE-301). The PR preserves the in-flight goal value across refreshes and reduces stale refresh overwrites client-side.
Changes:
- Adjust sprint goal lookup/building to prefer the in-memory goal object during dialog refreshes.
- Make Turbo Stream refresh requests debounced and abortable to prevent slower, stale responses from overwriting newer form state.
- Add regression coverage (RSpec model + feature spec, plus a new Stimulus controller unit spec).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/backlogs/spec/models/sprint_spec.rb | Adds a spec ensuring goal_for can return an in-memory goal for unsaved sprints. |
| modules/backlogs/spec/features/backlogs/create_spec.rb | Adds a regression feature spec and stabilizes date-change refresh timing around goal entry. |
| modules/backlogs/app/models/sprint.rb | Changes goal_for implementation used to preserve goal state across refreshes. |
| modules/backlogs/app/components/backlogs/sprint_form_component.rb | Uses goal_for / build instead of DB-backed find_or_initialize_by to keep the typed goal. |
| frontend/src/stimulus/controllers/refresh-on-form-changes.controller.ts | Debounces refresh and aborts in-flight requests using @rails/request.js Turbo Stream handling. |
| frontend/src/stimulus/controllers/refresh-on-form-changes.controller.spec.ts | Adds unit tests for query params, abort behavior, debouncing/coalescing, and error handling. |
80507c5 to
356ba4c
Compare
Debounces the Stimulus refresh and switches to `@rails/request.js`, aborting superseded requests so a slow earlier response cannot land last and overwrite the form with stale state. https://community.openproject.org/wp/AGILE-301
356ba4c to
04e21cb
Compare
Extract the query serialization into serializeFormQuery, which copies only string entries so file inputs cannot leak into the URL and the FormData cast to undefined is gone. Use location.replace so a refresh does not push a history entry.
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
ulferts
approved these changes
Jun 24, 2026
ulferts
left a comment
Contributor
There was a problem hiding this comment.
Change looks good. I have questions but those do not prevent merging the PR.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Ticket
https://community.openproject.org/wp/AGILE-301
What are you trying to accomplish?
Entering a start/finish date in the New sprint dialog triggered a form refresh (the duration preview) that wiped any goal text already typed. End users lost their sprint goal whenever they filled the dates after the goal.
Root cause: on refresh the form rebuilt the goal via
find_or_initialize_by, which queried the database for an unsaved sprint (sprint_id IS NULL) and ignored the in-memory goal submitted with the request — rendering a blank field.Note
This fixes a recently-introduced flake in
modules/backlogs/spec/features/backlogs/create_spec.rb(e.g. "allows creating a new sprint with a sprint goal"), where the goal occasionally vanished before submission. The same out-of-order refresh races could surface intermittently across the backlogs create specs in CI.What approach did you choose and why?
Sprint#goal_fornow always reads the in-memorygoalsassociation, so a refresh re-renders the submitted goal instead of a blank one. This is the actual bug fix.refresh-on-form-changescontroller debounces the refresh and aborts any superseded request, so a slow earlier response can no longer land last and overwrite the form with stale state (the source of the residual flake).Drive-by
Not related to the bug, but worth fixing while here: this PR includes a fix to make the controller safer to use with forms containing form inputs.
the controller's query serialization cast
FormDatatoundefinedto satisfyURLSearchParams. Replaced with a smallserializeFormQueryhelper that copies only string entries (so file inputs can't leak into the URL), drops the cast, and switches the reload tolocation.replaceso a refresh no longer pushes a history entry.Merge checklist