[AGILE-31] Sprint Goals

[myabc]

Ticket

https://community.openproject.org/wp/AGILE-31

What are you trying to accomplish?

Add sprint goals to Backlogs so each sprint can store and display project-specific goal text.

This includes:

• A new SprintGoal model keyed by sprint and project.
• Sprint goal create/update/delete handling in the sprint dialog flow.
• Goal rendering in the sprint header.
• Shared sprint behavior where sprint metadata and project-specific goals can be edited according to the user's permissions in the relevant project.
• API v3 sprint goal rendering for project-scoped sprint responses.

Screenshots

Creating/Editing via Sprint dialog

Type	Screenshot
Project-specific sprint
Shared sprint, complete edit rights
Shared sprint, receiving project without edit rights

Displaying on Backlog and Sprints page

What approach did you choose and why?

Sprint goals are stored separately from the sprint itself because shared sprints need project-specific goal text. The sprint remains the shared planning object, while SprintGoal captures the per-project goal for that sprint.

The sprint dialog keeps using the existing sprint create/update flow, with goal persistence handled by the sprint services. Goal-only updates for shared sprints can succeed with permissions in the current project, while sprint attribute updates remain protected by permissions in the sprint's defining project.

The UI follows the existing Backlogs ViewComponent and Primer form patterns.

Testing

Added or updated coverage for:

• SprintGoal model associations, normalization, and uniqueness validation.
• Sprint#goal_for and Sprint#goal_text_for.
• Sprint create/update services persisting goals and surfacing goal persistence errors as failed service results.
• Sprint dialog form rendering and shared sprint permissions.
• Sprint header goal rendering.
• Controller/request-cycle behavior for the sprint dialog actions.

Merge checklist

• Added/updated tests
• Added/updated documentation in Lookbook (patterns, previews, etc)
• Tested major browsers (Chrome, Firefox, Edge, ...)

[Copilot]

Pull request overview

Adds “Sprint Goals” to Backlogs by introducing a per-(sprint, project) SprintGoal record, surfacing it in the Backlogs UI (sprint header + edit form), and exposing a new goal property in the API representer + schema.

Changes:

• Introduce SprintGoal model + DB table and wire it to Sprint (goal_for(project)).
• Add goal persistence to sprint create/update flows and render/edit goal in Backlogs sprint UI.
• Extend API v3 sprint representer + schema and add accompanying specs/factories/locales.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
modules/backlogs/app/models/sprint.rb	Adds has_many :sprint_goals and goal_for(project) lookup.
modules/backlogs/app/models/sprint_goal.rb	New model with uniqueness validation per sprint+project.
modules/backlogs/db/migrate/20260519144514_create_sprint_goals.rb	Creates sprint_goals table with FK + unique index.
modules/backlogs/app/services/sprints/create_service.rb	Persists initial goal when creating a sprint.
modules/backlogs/app/services/sprints/update_service.rb	Attempts to persist/update/delete goal on sprint update.
modules/backlogs/app/controllers/backlogs/sprints_controller.rb	Permits/threads goal param through create/update; passes project into components.
modules/backlogs/app/components/backlogs/sprint_form_component.rb	Adds shared-sprint UX logic and goal field helpers.
modules/backlogs/app/components/backlogs/sprint_form_component.html.erb	Renders shared-sprint banners and goal text field.
modules/backlogs/app/components/backlogs/sprint_dialog_component.rb	Requires project: so the form can be project-contextual.
modules/backlogs/app/components/backlogs/sprint_dialog_component.html.erb	Passes project into SprintFormComponent.
modules/backlogs/app/components/backlogs/sprint_component.rb	Adds goal_text for header rendering.
modules/backlogs/app/components/backlogs/sprint_component.html.erb	Displays goal under sprint metadata when present.
modules/backlogs/app/forms/backlogs/sprints/details_form.rb	Adds disabled support for shared sprint scenarios.
modules/backlogs/app/forms/backlogs/sprints/dates_form.rb	Adds disabled support for shared sprint scenarios.
modules/backlogs/lib/api/v3/sprints/sprint_representer.rb	Adds goal property with optional goal_project context.
docs/api/apiv3/components/schemas/sprint_model.yml	Documents new goal field in API schema.
modules/backlogs/config/locales/en.yml	Adds shared sprint banner/caption texts + sprint goal validation message.
modules/backlogs/spec/factories/sprint_goal_factory.rb	Factory for SprintGoal.
modules/backlogs/spec/models/sprint_spec.rb	Spec for Sprint#goal_for.
modules/backlogs/spec/models/sprint_goal_spec.rb	Specs for associations/validations (incl. uniqueness message).
modules/backlogs/spec/components/backlogs/sprint_form_component_spec.rb	Specs for goal field rendering + shared sprint UX rules.
modules/backlogs/spec/components/backlogs/sprint_component_spec.rb	Specs for header goal rendering.
modules/backlogs/spec/lib/api/v3/sprints/sprint_representer_rendering_spec.rb	Specs for API goal rendering with/without project context.

[Copilot]

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated 6 comments.

[dombesz]

As a general approach, I would suggest using accepts_nested_attributes_for method to handle the goal updates. It will simplify the create/update services by avoiding the goal specific persisting methods. It will also simplify the sprints controller too.

Here's a brief example of the approach:

# sprint.rb
accepts_nested_attributes_for :goals

# sprint_form_component.rb

 def goal_for_project
   @goal_for_project ||= sprint.goals.find_or_initialize_by(project:)
 end

# sprint_form_component.html.erb
# Replace the GoalForm row with:

 if shared_sprint?
   flex.with_row(mb: 3) { tag.hr }
 end
 flex.with_row do
   f.primer_fields_for :goals, goal_for_project, index: 0 do |goal_f|
     render Backlogs::Sprints::GoalFieldsForm.new(
       goal_f,
       label:    goal_label,
       caption:  goal_caption,
       disabled: !can_edit_goal?
     )
   end
 end

# goal_fields_form.rb
 module Backlogs
   module Sprints
     class GoalFieldsForm < ApplicationForm
       def initialize(label:, caption: nil, disabled: false)
         @label   = label
         @caption = caption
         @disabled = disabled
         super()
       end

       form do |f|
         f.hidden(name: :id,         value: model.id)         if model.persisted?
         f.hidden(name: :project_id, value: model.project_id) #project_id is already set by the find_or_initialize
         f.text_field(
           name:       :text,
           label:      @label,
           value:      model.text,
           caption:    @caption,
           disabled:   @disabled,
           full_width: true
         )
       end
     end
   end
 end

# sprints_controller.rb

 def sprint_params
   params.permit(sprint: [:name, :start_date, :finish_date,
                           goals_attributes: [:id, :project_id, :text]])
 end

 # Remove: goal_param, sprint_update_params.
 # Drop goal: and goal_project: keyword args from the create and update service calls.

Remove: set_attributes_params, after_perform, persist_goal, persist_sprint_goal from create_service.rb
Remove: set_attributes_params, after_perform, persist_goal, persist_goal_text, persist_sprint_goal, destroy_sprint_goal from update_service.rb

The validation errors from the goal uniqueness, can be also placed in the BaseContract, and any Goal validation error will prevent saving both the Sprint and Goal, thanks to accepts_nested_attributes. The nested Goal errors also bubble up to the Sprint object too.

Also left a few pointers regarding the api, but admittedly I haven't checked the specs yet.

[Copilot]

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.

[dombesz]

I like the direction in general 👍 However I left a few comments where the approach seems to be problematic, such as the GoalFormModel and the before filter logic of the SprintsController.

[dombesz]

I wonder why do we need the following extra logic?

skip_before_action :load_sprint_and_project, only: SPRINTLESS_ACTIONS
prepend_before_action :load_project, only: SPRINTLESS_ACTIONS

From my understanding having the suggested load_sprint method update in the BaseController should work just fine.

def load_sprint                                                                                                                                                                    
  sprint_id = params[:sprint_id] || params.dig(:sprint, :id)
  return unless sprint_id                                                                                                                                                          
                                                                                                                                                                                   
  @sprint = Sprint.for_project(@project).visible.find(sprint_id)
end

Since SPRINTLESS_ACTIONS do not have a params[:sprint_id] nor a params[:sprint][:id] present, the load_sprint method just returns early, resulting in the same behaviour is the more complex implementation above.

The same question goes for the before_action :load_sprint_from_form_id, only: :refresh_form too. The load_sprint implementation also makes this before action redundant, because having the params.dig(:sprint, :id) would also find the sprint. So the load_sprint_from_form_id can also be removed.

[myabc]

I ended up keeping explicit sprintless loading because generic params.dig(:sprint, :id) loading risks treating a submitted create-form ID as the current sprint (in fact current specs intentionally assert create ignores submitted sprint[id])

[github-actions]

Warning

Flaky specs

• rspec ./modules/my_page/spec/features/my/work_package_table_spec.rb[1:1:1]
• rspec ./spec/features/projects/lists/filters_spec.rb[1:6:1]

🤖 Ask Copilot to investigate

Copy 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.

@copilot The following spec(s) are flaky in CI (first seen on PR #23271, linked for reference only):

- `rspec ./modules/my_page/spec/features/my/work_package_table_spec.rb[1:1:1]`
- `rspec ./spec/features/projects/lists/filters_spec.rb[1:6:1]`

Treat this as a standalone task, unrelated to PR #23271. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #23271 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @myabc to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @myabc, and request a review from @myabc.
On every commit, set @myabc as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

[ulferts]

I am merging this now after only a functional test, trusting in @dombesz previously done review.