From b5435b8219d1715a80eece2c90afffb0bb7b7db9 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Tue, 19 May 2026 16:57:07 +0200 Subject: [PATCH 01/10] [#71059] Add SprintGoal persistence model Adds project-specific sprint goal records and lets Rails assign the association through the sprint save path. https://community.openproject.org/wp/71059 --- .../backlogs/concerns/container_loading.rb | 2 +- modules/backlogs/app/models/sprint.rb | 24 ++++++ modules/backlogs/app/models/sprint_goal.rb | 41 ++++++++++ modules/backlogs/config/locales/en.yml | 4 + .../20260519144514_create_sprint_goals.rb | 43 +++++++++++ .../spec/factories/sprint_goal_factory.rb | 37 ++++++++++ .../backlogs/spec/models/sprint_goal_spec.rb | 69 +++++++++++++++++ modules/backlogs/spec/models/sprint_spec.rb | 74 +++++++++++++++++++ 8 files changed, 293 insertions(+), 1 deletion(-) create mode 100644 modules/backlogs/app/models/sprint_goal.rb create mode 100644 modules/backlogs/db/migrate/20260519144514_create_sprint_goals.rb create mode 100644 modules/backlogs/spec/factories/sprint_goal_factory.rb create mode 100644 modules/backlogs/spec/models/sprint_goal_spec.rb diff --git a/modules/backlogs/app/controllers/backlogs/concerns/container_loading.rb b/modules/backlogs/app/controllers/backlogs/concerns/container_loading.rb index e383a2afbc18..2b8b6458c119 100644 --- a/modules/backlogs/app/controllers/backlogs/concerns/container_loading.rb +++ b/modules/backlogs/app/controllers/backlogs/concerns/container_loading.rb @@ -41,7 +41,7 @@ def load_sprint_data @sprints = Sprint.for_project(@project) .not_completed .order_by_date - .includes(:project, :task_boards) + .includes(:project, :task_boards, :goals) @active_sprint_ids = @sprints.select(&:active?).map(&:id) @work_packages_by_sprint_id = WorkPackage diff --git a/modules/backlogs/app/models/sprint.rb b/modules/backlogs/app/models/sprint.rb index d03b02190193..a06f17792446 100644 --- a/modules/backlogs/app/models/sprint.rb +++ b/modules/backlogs/app/models/sprint.rb @@ -33,6 +33,18 @@ class Sprint < ApplicationRecord belongs_to :project has_many :work_packages, inverse_of: :sprint, dependent: :nullify + has_many :goals, + class_name: "SprintGoal", + inverse_of: :sprint, + dependent: :delete_all + + accepts_nested_attributes_for :goals, + allow_destroy: true, + reject_if: ->(attributes) { + attributes["id"].blank? && attributes["text"].blank? + }, + limit: 1 + has_many :task_boards, as: :linked, class_name: "Boards::Grid", @@ -100,5 +112,17 @@ def visible_to?(project) self.class.for_project(project).exists?(id:) end + def goal_for(project) + if goals.loaded? + goals.find { |goal| goal.project_id == project.id } + else + goals.find_by(project:) + end + end + + def goal_text_for(project) + goal_for(project)&.text + end + def to_s = name end diff --git a/modules/backlogs/app/models/sprint_goal.rb b/modules/backlogs/app/models/sprint_goal.rb new file mode 100644 index 000000000000..9fe1b8aa720a --- /dev/null +++ b/modules/backlogs/app/models/sprint_goal.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class SprintGoal < ApplicationRecord + belongs_to :sprint, inverse_of: :goals + belongs_to :project + + normalizes :text, with: ->(text) { text.strip.presence } + + validates :text, presence: true + + validates :project_id, + uniqueness: { scope: :sprint_id, message: :project_already_has_goal } +end diff --git a/modules/backlogs/config/locales/en.yml b/modules/backlogs/config/locales/en.yml index 9c4e11169826..0c238c090fb0 100644 --- a/modules/backlogs/config/locales/en.yml +++ b/modules/backlogs/config/locales/en.yml @@ -49,6 +49,8 @@ en: active: "Active" completed: "Completed" work_packages: "Work packages" + sprint_goal: + text: "Sprint goal" work_package: backlog_bucket: "Backlog bucket" backlogs_work_package_type: "Backlog type" @@ -68,6 +70,8 @@ en: sprint_sharing: share_all_projects_already_taken: "cannot be set because project \"%{name}\" is already sharing with all projects." share_all_projects_already_taken_anonymous: "cannot be set because another project is already sharing with all projects." + sprint_goal: + project_already_has_goal: "already has a goal for this sprint." sprint: attributes: base: diff --git a/modules/backlogs/db/migrate/20260519144514_create_sprint_goals.rb b/modules/backlogs/db/migrate/20260519144514_create_sprint_goals.rb new file mode 100644 index 000000000000..bb5bd818f495 --- /dev/null +++ b/modules/backlogs/db/migrate/20260519144514_create_sprint_goals.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class CreateSprintGoals < ActiveRecord::Migration[8.0] + def change + create_table :sprint_goals do |t| + t.references :sprint, null: false, foreign_key: { on_delete: :cascade } + t.references :project, null: false, foreign_key: { on_delete: :cascade } + t.text :text, null: false + + t.timestamps + end + + add_index :sprint_goals, %i[sprint_id project_id], unique: true + end +end diff --git a/modules/backlogs/spec/factories/sprint_goal_factory.rb b/modules/backlogs/spec/factories/sprint_goal_factory.rb new file mode 100644 index 000000000000..1e9f562cdfd9 --- /dev/null +++ b/modules/backlogs/spec/factories/sprint_goal_factory.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +FactoryBot.define do + factory :sprint_goal do + sprint + project { sprint.project } + text { "Sprint goal" } + end +end diff --git a/modules/backlogs/spec/models/sprint_goal_spec.rb b/modules/backlogs/spec/models/sprint_goal_spec.rb new file mode 100644 index 000000000000..6750e7faef82 --- /dev/null +++ b/modules/backlogs/spec/models/sprint_goal_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "rails_helper" + +RSpec.describe SprintGoal do + let(:project) { create(:project) } + let(:sprint) { create(:sprint, project:) } + + subject(:sprint_goal) do + described_class.new(sprint:, project:, text: "Deliver reporting dashboard") + end + + describe "associations" do + it { is_expected.to belong_to(:sprint).inverse_of(:goals) } + it { is_expected.to belong_to(:project) } + end + + describe "normalization" do + it { is_expected.to normalize(:text).from(" My awesome sprint\n").to("My awesome sprint") } + it { is_expected.to normalize(:text).from(" \n").to(nil) } + end + + describe "validations" do + it "is valid with a sprint, project, and text" do + expect(sprint_goal).to be_valid + end + + it "is invalid without text" do + sprint_goal.text = nil + + expect(sprint_goal).not_to be_valid + end + + it "validates uniqueness of project_id scoped to sprint_id" do + sprint_goal.save! + expect(sprint_goal).to validate_uniqueness_of(:project_id) + .scoped_to(:sprint_id) + .with_message(I18n.t("activerecord.errors.models.sprint_goal.project_already_has_goal")) + end + end +end diff --git a/modules/backlogs/spec/models/sprint_spec.rb b/modules/backlogs/spec/models/sprint_spec.rb index 0839b9c39ffd..fe59d21ad020 100644 --- a/modules/backlogs/spec/models/sprint_spec.rb +++ b/modules/backlogs/spec/models/sprint_spec.rb @@ -130,9 +130,53 @@ describe "associations" do it { is_expected.to have_many(:work_packages).inverse_of(:sprint).dependent(:nullify) } it { is_expected.to have_many(:task_boards).dependent(:nullify) } + it { is_expected.to have_many(:goals).class_name("SprintGoal").inverse_of(:sprint).dependent(:delete_all) } it { is_expected.to belong_to(:project) } end + describe "nested goal attributes" do + let(:sprint) { create(:sprint, project:) } + + it { is_expected.to accept_nested_attributes_for(:goals).allow_destroy(true).limit(1) } + + it "assigns goal attributes" do + sprint.assign_attributes( + goals_attributes: [{ project_id: project.id, text: "Ship MVP" }] + ) + + expect(sprint.goals.first).to have_attributes(project_id: project.id, text: "Ship MVP") + end + + it "rejects blank new goals" do + expect do + sprint.assign_attributes( + goals_attributes: [{ project_id: project.id, text: "" }] + ) + end.not_to change { sprint.goals.length } + end + + it "marks existing blanked goals for destruction" do + goal = create(:sprint_goal, sprint:, project:, text: "Old goal") + + sprint.assign_attributes( + goals_attributes: [{ id: goal.id, text: "", _destroy: "1" }] + ) + + expect(sprint.goals.find { |sprint_goal| sprint_goal.id == goal.id }).to be_marked_for_destruction + end + + it "limits nested assignment to one contextual goal" do + expect do + sprint.assign_attributes( + goals_attributes: [ + { project_id: project.id, text: "First goal" }, + { project_id: project.id, text: "Second goal" } + ] + ) + end.to raise_error(ActiveRecord::NestedAttributes::TooManyRecords) + end + end + describe "#task_board_for" do let(:sprint) { create(:sprint, project:) } let(:other_project) { create(:project) } @@ -387,6 +431,36 @@ end end + describe "#goal_for" do + let(:sprint) { create(:sprint, project:) } + let!(:sprint_goal) { create(:sprint_goal, sprint:, project:, text: "Ship dashboard") } + + it "returns the goal for the given project" do + expect(sprint.goal_for(project)).to eq(sprint_goal) + end + + it "returns nil when no goal exists for the project" do + other_project = create(:project) + expect(sprint.goal_for(other_project)).to be_nil + end + end + + describe "#goal_text_for" do + let(:sprint) { create(:sprint, project:) } + + it "returns the goal text for the given project" do + create(:sprint_goal, sprint:, project:, text: "Ship dashboard") + + expect(sprint.goal_text_for(project)).to eq("Ship dashboard") + end + + it "returns nil when no goal exists for the project" do + other_project = create(:project) + + expect(sprint.goal_text_for(other_project)).to be_nil + end + end + describe "#to_s" do it "returns the name" do expect(sprint.to_s).to eq("Sprint 1") From 10500f8059a271439183d707eed5fe39f57a4b50 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Tue, 19 May 2026 17:10:27 +0200 Subject: [PATCH 02/10] [#71059] Add sprint goal CRUD to modal Adds goal editing to the sprint dialog while keeping browser params scoped to one contextual project goal. https://community.openproject.org/wp/71059 --- .../backlogs/sprint_component.html.erb | 4 +- .../components/backlogs/sprint_component.rb | 28 ++ .../backlogs/sprint_dialog_component.html.erb | 2 +- .../backlogs/sprint_dialog_component.rb | 5 +- .../backlogs/sprint_form_component.html.erb | 25 +- .../backlogs/sprint_form_component.rb | 54 ++- .../backlogs/sprints/base_contract.rb | 38 +- .../controllers/backlogs/base_controller.rb | 2 +- .../backlogs/sprints_controller.rb | 83 +++-- .../app/forms/backlogs/sprints/dates_form.rb | 6 + .../forms/backlogs/sprints/details_form.rb | 15 +- .../forms/backlogs/sprints/owned_goal_form.rb | 47 +++ .../backlogs/sprints/shared_goal_form.rb | 52 +++ .../sprints/set_attributes_service.rb | 50 +++ modules/backlogs/config/locales/en.yml | 8 + .../backlogs/sprint_component_spec.rb | 37 ++ .../backlogs/sprint_form_component_spec.rb | 128 ++++++- .../sprints/shared_contract_examples.rb | 8 + .../backlogs/sprints_controller_spec.rb | 351 +++++++++++++++++- .../backlogs/sprints/owned_goal_form_spec.rb | 89 +++++ .../backlogs/sprints/shared_goal_form_spec.rb | 92 +++++ .../backlogs/sprints/create_service_spec.rb | 36 ++ .../sprints/set_attributes_service_spec.rb | 44 +++ .../backlogs/sprints/update_service_spec.rb | 161 ++++++++ 24 files changed, 1299 insertions(+), 66 deletions(-) create mode 100644 modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb create mode 100644 modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb create mode 100644 modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb create mode 100644 modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb create mode 100644 modules/backlogs/spec/services/backlogs/sprints/update_service_spec.rb diff --git a/modules/backlogs/app/components/backlogs/sprint_component.html.erb b/modules/backlogs/app/components/backlogs/sprint_component.html.erb index 410649bccca8..f837ffb245f3 100644 --- a/modules/backlogs/app/components/backlogs/sprint_component.html.erb +++ b/modules/backlogs/app/components/backlogs/sprint_component.html.erb @@ -44,7 +44,7 @@ See COPYRIGHT and LICENSE files for more details. ) ) do |list| %> - <% list.with_header(title: sprint.name) do |header| %> + <% list.with_header(title: sprint.name, title_arguments:) do |header| %> <% header.with_description(display: :flex, direction: :column, classes: "row-gap-2") do %> <%= render(Primer::Alpha::Stack.new(direction: :horizontal, align: :center)) do %> <%= render(Backlogs::SprintStatusBadgeComponent.new(sprint:)) %> @@ -84,7 +84,7 @@ See COPYRIGHT and LICENSE files for more details. header.with_menu(button_aria_label: t(".label_actions")) do |menu| with_item_group(menu) do - if user_allowed?(:create_sprints) + if can_open_edit_dialog? menu.with_item( id: dom_target(sprint, :menu, :edit_sprint), label: t(".action_menu.edit_sprint"), diff --git a/modules/backlogs/app/components/backlogs/sprint_component.rb b/modules/backlogs/app/components/backlogs/sprint_component.rb index c812c30deadf..b40ce46129b7 100644 --- a/modules/backlogs/app/components/backlogs/sprint_component.rb +++ b/modules/backlogs/app/components/backlogs/sprint_component.rb @@ -91,6 +91,22 @@ def finish_sprint_button_arguments } end + def goal_text + return @goal_text if defined?(@goal_text) + + @goal_text = sprint.goal_text_for(project) + end + + def sprint_goal_id + dom_target(sprint, :goal) + end + + def title_arguments + return {} if goal_text.blank? + + { aria: { describedby: sprint_goal_id } } + end + def story_points_total work_packages.filter_map(&:story_points).sum end @@ -120,5 +136,17 @@ def show_task_board_link? def show_burndown_link? sprint.active? end + + def can_open_edit_dialog? + if sprint.owned_by?(project) + user_allowed?(:create_sprints) + else + user_allowed?(:create_sprints) || user_allowed?(:create_sprints, project: sprint.project) + end + end + + def user_allowed?(permission, project: self.project) + current_user.allowed_in_project?(permission, project) + end end end diff --git a/modules/backlogs/app/components/backlogs/sprint_dialog_component.html.erb b/modules/backlogs/app/components/backlogs/sprint_dialog_component.html.erb index cf8c5535bb12..1c7a1caef20c 100644 --- a/modules/backlogs/app/components/backlogs/sprint_dialog_component.html.erb +++ b/modules/backlogs/app/components/backlogs/sprint_dialog_component.html.erb @@ -38,7 +38,7 @@ See COPYRIGHT and LICENSE files for more details. d.with_header(variant: :large) d.with_body do - render(Backlogs::SprintFormComponent.new(sprint: @sprint)) + render(Backlogs::SprintFormComponent.new(sprint: @sprint, project: @project)) end d.with_footer do diff --git a/modules/backlogs/app/components/backlogs/sprint_dialog_component.rb b/modules/backlogs/app/components/backlogs/sprint_dialog_component.rb index dd9f110b52ff..0fd707ce0cb3 100644 --- a/modules/backlogs/app/components/backlogs/sprint_dialog_component.rb +++ b/modules/backlogs/app/components/backlogs/sprint_dialog_component.rb @@ -41,14 +41,15 @@ class SprintDialogComponent < ApplicationComponent STATE_DEFAULT = :create STATE_OPTIONS = [STATE_DEFAULT, :edit].freeze - attr_reader :sprint, :state + attr_reader :sprint, :project, :state delegate :create?, :edit?, to: :state - def initialize(sprint:, state: STATE_DEFAULT) + def initialize(sprint:, project:, state: STATE_DEFAULT) super @sprint = sprint + @project = project @state = ActiveSupport::StringInquirer.new(fetch_or_fallback(STATE_OPTIONS, state, STATE_DEFAULT).to_s) end diff --git a/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb b/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb index 5c764662d848..b7c5b696e22f 100644 --- a/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb +++ b/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb @@ -40,15 +40,30 @@ See COPYRIGHT and LICENSE files for more details. ) do |f| flex_layout(mb: 2) do |flex| if base_errors&.any? - flex.with_row do - render(Primer::Alpha::Banner.new(mb: 3, icon: :stop, scheme: :danger)) { base_errors.join("\n") } + flex.with_row(mb: 3) do + render(Primer::Alpha::Banner.new(mb: 3, scheme: :danger)) { base_errors.join("\n") } + end + end + if shared_sprint? + flex.with_row(mb: 3) do + render(Primer::Alpha::Banner.new(scheme: banner_scheme)) { banner_text } end end flex.with_row(mb: 3) do - render Backlogs::Sprints::DetailsForm.new(f) + render Backlogs::Sprints::DetailsForm.new(f, disabled: !can_edit_sprint?) + end + flex.with_row(mb: 3, classes: "FormControl-horizontalGroup--sm-vertical") do + render Backlogs::Sprints::DatesForm.new(f, disabled: !can_edit_sprint?) end - flex.with_row(classes: "FormControl-horizontalGroup--sm-vertical") do - render Backlogs::Sprints::DatesForm.new(f) + if shared_sprint? + flex.with_row(mb: 3) do + render(Primer::Forms::Separator.new) + end + end + flex.with_row do + f.fields_for(:goal, goal) do |goal_fields| + render goal_form_class.new(goal_fields, disabled: !can_edit_goal?) + end end end end diff --git a/modules/backlogs/app/components/backlogs/sprint_form_component.rb b/modules/backlogs/app/components/backlogs/sprint_form_component.rb index 2ecf0f9c6fb1..a538f40e219f 100644 --- a/modules/backlogs/app/components/backlogs/sprint_form_component.rb +++ b/modules/backlogs/app/components/backlogs/sprint_form_component.rb @@ -37,15 +37,57 @@ class SprintFormComponent < ApplicationComponent FORM_ID = SprintDialogComponent::FORM_ID - attr_reader :sprint, :base_errors + attr_reader :sprint, :project, :current_user, :base_errors - def initialize(sprint:, base_errors: nil) + def initialize(sprint:, project:, current_user: User.current, base_errors: nil) super @sprint = sprint + @project = project + @current_user = current_user @base_errors = base_errors end + def shared_sprint? + sprint.persisted? && !sprint.owned_by?(project) + end + + def can_edit_sprint? + return true unless shared_sprint? + + current_user.allowed_in_project?(:create_sprints, sprint.project) + end + + def can_edit_goal? + return true unless shared_sprint? + + current_user.allowed_in_project?(:create_sprints, project) + end + + def banner_scheme + can_edit_sprint? ? :default : :warning + end + + def banner_text + if can_edit_sprint? + t(".shared_sprint_info_banner") + else + t(".shared_sprint_warning_banner") + end + end + + def goal + sprint.goals.find_or_initialize_by(project:) + end + + def goal_form_class + if shared_sprint? + Backlogs::Sprints::SharedGoalForm + else + Backlogs::Sprints::OwnedGoalForm + end + end + private def http_verb @@ -54,9 +96,9 @@ def http_verb def form_url if sprint.new_record? - project_backlogs_sprints_path(sprint.project_id, all_backlogs_params) + project_backlogs_sprints_path(project, all_backlogs_params) else - project_backlogs_sprint_path(sprint.project_id, sprint.id, all_backlogs_params) + project_backlogs_sprint_path(project, sprint, all_backlogs_params) end end @@ -64,8 +106,8 @@ def data_attributes { controller: "refresh-on-form-changes", "refresh-on-form-changes-target": "form", - "refresh-on-form-changes-turbo-stream-url-value": refresh_form_project_backlogs_sprints_path(sprint.project_id, - all_backlogs_params) + "refresh-on-form-changes-turbo-stream-url-value": + refresh_form_project_backlogs_sprints_path(project, all_backlogs_params) } end end diff --git a/modules/backlogs/app/contracts/backlogs/sprints/base_contract.rb b/modules/backlogs/app/contracts/backlogs/sprints/base_contract.rb index 95ececc11406..0bdf04f512dc 100644 --- a/modules/backlogs/app/contracts/backlogs/sprints/base_contract.rb +++ b/modules/backlogs/app/contracts/backlogs/sprints/base_contract.rb @@ -30,7 +30,10 @@ module Backlogs::Sprints class BaseContract < ::ModelContract - validate :user_authorized + SPRINT_ATTRIBUTES = %w[name project_id start_date finish_date].freeze + + validate :user_authorized_for_sprint_attributes + validate :user_authorized_for_goal_attributes def self.model Sprint @@ -40,15 +43,46 @@ def self.model attribute :project_id attribute :start_date attribute :finish_date + attribute :goals_attributes, readable: false private - def user_authorized + def user_authorized_for_sprint_attributes return unless model.project + return unless sprint_attributes_changed? unless user.allowed_in_project?(:create_sprints, model.project) errors.add :base, :error_unauthorized end end + + def sprint_attributes_changed? + model.new_record? || model.changed.intersect?(SPRINT_ATTRIBUTES) + end + + def user_authorized_for_goal_attributes + changed_goals.each do |goal| + project = goal.project + + unless project && sprint_visible_to_goal_project?(project) && user.allowed_in_project?(:create_sprints, project) + errors.add :base, :error_unauthorized + end + end + end + + def changed_goals + goals_association = model.association(:goals) + return [] unless goals_association.loaded? || goals_association.target.any? + + goals_association.target.select { |goal| goal.changed? || goal.marked_for_destruction? } + end + + def sprint_visible_to_goal_project?(project) + if model.new_record? + model.project == project + else + model.visible_to?(project) + end + end end end diff --git a/modules/backlogs/app/controllers/backlogs/base_controller.rb b/modules/backlogs/app/controllers/backlogs/base_controller.rb index 268c0780ca5a..347fa056fbca 100644 --- a/modules/backlogs/app/controllers/backlogs/base_controller.rb +++ b/modules/backlogs/app/controllers/backlogs/base_controller.rb @@ -51,7 +51,7 @@ def load_project end def load_sprint - @sprint_id = params.delete(:sprint_id) + @sprint_id = params[:sprint_id].presence return unless @sprint_id @sprint = Sprint.for_project(@project).visible.find(@sprint_id) diff --git a/modules/backlogs/app/controllers/backlogs/sprints_controller.rb b/modules/backlogs/app/controllers/backlogs/sprints_controller.rb index 7b7fb801e4d3..cea059dd477e 100644 --- a/modules/backlogs/app/controllers/backlogs/sprints_controller.rb +++ b/modules/backlogs/app/controllers/backlogs/sprints_controller.rb @@ -32,19 +32,16 @@ module Backlogs class SprintsController < BaseController include OpTurbo::ComponentStream - ACTIONS_WITHOUT_SPRINT = %i[ - new_dialog - edit_dialog - index - create - refresh_form - ].freeze SPRINT_STATE_ACTIONS = %i[start finish].freeze + SHARED_SPRINT_EDIT_ACTIONS = %i[edit_dialog update refresh_form].freeze + SPRINTLESS_ACTIONS = %i[index new_dialog create].freeze - skip_before_action :load_sprint_and_project, only: ACTIONS_WITHOUT_SPRINT - skip_before_action :authorize, only: SPRINT_STATE_ACTIONS + skip_before_action :load_sprint_and_project, only: SPRINTLESS_ACTIONS + skip_before_action :authorize, only: SPRINT_STATE_ACTIONS + SHARED_SPRINT_EDIT_ACTIONS - before_action :load_project, only: ACTIONS_WITHOUT_SPRINT + prepend_before_action :load_project, only: SPRINTLESS_ACTIONS + before_action :load_sprint_from_form_id, only: :refresh_form + before_action :authorize_sprint_edit!, only: SHARED_SPRINT_EDIT_ACTIONS before_action :authorize_start!, only: :start before_action :authorize_finish!, only: :finish @@ -72,18 +69,15 @@ def new_dialog contract_class: ::EmptyContract ).call(attributes: converted_sprint_params) - respond_with_dialog Backlogs::SprintDialogComponent.new(sprint: call.result) + respond_with_dialog Backlogs::SprintDialogComponent.new(sprint: call.result, project: @project) end def edit_dialog - @sprint = Sprint.for_project(@project).visible.find(params.expect(:sprint_id)) - - respond_with_dialog Backlogs::SprintDialogComponent.new(sprint: @sprint, state: :edit) + respond_with_dialog Backlogs::SprintDialogComponent.new(sprint: @sprint, project: @project, state: :edit) end def refresh_form - id = edit_sprint_params.dig(:sprint, :id) - sprint = id.present? ? Sprint.for_project(@project).visible.find(id) : Sprint.new + sprint = @sprint || Sprint.new call = ::Backlogs::Sprints::SetAttributesService.new( user: current_user, @@ -91,19 +85,18 @@ def refresh_form contract_class: ::EmptyContract ).call(attributes: converted_sprint_params) - update_via_turbo_stream(component: Backlogs::SprintFormComponent.new(sprint: call.result)) + update_via_turbo_stream(component: Backlogs::SprintFormComponent.new(sprint: call.result, project: @project)) respond_with_turbo_streams end - def create # rubocop:disable Metrics/AbcSize + def create call = ::Backlogs::Sprints::CreateService .new(user: current_user) .call(attributes: converted_sprint_params) if call.success? - flash[:notice] = I18n.t(:notice_successful_create) - render turbo_stream: turbo_stream.redirect_to(project_backlogs_backlog_path(@project, helpers.all_backlogs_params)) + respond_with_create_success else update_sprint_form_component_via_turbo_stream(sprint: call.result, base_errors: call.errors[:base]) respond_with_turbo_streams @@ -113,7 +106,7 @@ def create # rubocop:disable Metrics/AbcSize def update call = ::Backlogs::Sprints::UpdateService .new(user: current_user, model: @sprint) - .call(attributes: sprint_params[:sprint]) + .call(attributes: converted_sprint_params) if call.success? render_success_flash_message_via_turbo_stream(message: I18n.t(:notice_successful_update)) @@ -165,6 +158,7 @@ def update_sprint_form_component_via_turbo_stream(sprint:, base_errors: nil) update_via_turbo_stream( component: Backlogs::SprintFormComponent.new( sprint:, + project: @project, base_errors: ), status: :bad_request @@ -181,26 +175,53 @@ def show_finish_sprint_dialog ) end - def load_sprint_and_project - load_project + def authorize_sprint_edit! + return deny_access unless current_user.allowed_in_project?(:view_sprints, @project) - sprint_id = params[:sprint_id] - @sprint = Sprint.for_project(@project).visible.find(sprint_id) if sprint_id + if @sprint&.persisted? + can_edit_sprint = current_user.allowed_in_project?(:create_sprints, @sprint.project) + can_edit_goal = current_user.allowed_in_project?(:create_sprints, @project) + deny_access unless can_edit_sprint || can_edit_goal + else + deny_access unless current_user.allowed_in_project?(:create_sprints, @project) + end + end + + def respond_with_create_success + flash[:notice] = I18n.t(:notice_successful_create) + render turbo_stream: turbo_stream.redirect_to(project_backlogs_backlog_path(@project, helpers.all_backlogs_params)) end def sprint_params - params.permit(sprint: %i[name start_date finish_date]) + params.permit(sprint: [ + :name, + :start_date, + :finish_date, + { goal: %i[text] } + ]) end - def edit_sprint_params - params.permit(sprint: %i[id name start_date finish_date]) + def goal_params + sprint_params.dig(:sprint, :goal) end def converted_sprint_params - converted_params = sprint_params[:sprint].to_h - converted_params[:project] = @project + attributes = sprint_params[:sprint].to_h.symbolize_keys + attributes = attributes.merge(project: @project) unless @sprint&.persisted? + attributes = attributes.merge(goal_project: @project) if attributes.key?(:goal) + + attributes + end + + def load_sprint_from_form_id + @sprint_id = sprint_id_param + return unless @sprint_id + + @sprint = Sprint.for_project(@project).visible.find(@sprint_id) + end - converted_params + def sprint_id_param + params.permit(sprint: [:id]).dig(:sprint, :id).presence end def start_sprint diff --git a/modules/backlogs/app/forms/backlogs/sprints/dates_form.rb b/modules/backlogs/app/forms/backlogs/sprints/dates_form.rb index e6e3a43ddae2..8b3ba00e128d 100644 --- a/modules/backlogs/app/forms/backlogs/sprints/dates_form.rb +++ b/modules/backlogs/app/forms/backlogs/sprints/dates_form.rb @@ -31,6 +31,10 @@ module Backlogs module Sprints class DatesForm < ApplicationForm + extend Dry::Initializer + + option :disabled, default: -> { false } + delegate :active?, to: :model form do |f| @@ -41,6 +45,7 @@ class DatesForm < ApplicationForm label: attribute_name(:start_date), placeholder: attribute_name(:start_date), required: active?, + disabled:, input_width: :small, data: { action: "change->refresh-on-form-changes#triggerTurboStream" @@ -52,6 +57,7 @@ class DatesForm < ApplicationForm label: attribute_name(:finish_date), placeholder: attribute_name(:finish_date), required: active?, + disabled:, input_width: :small, data: { action: "change->refresh-on-form-changes#triggerTurboStream" diff --git a/modules/backlogs/app/forms/backlogs/sprints/details_form.rb b/modules/backlogs/app/forms/backlogs/sprints/details_form.rb index d49b9ccc3c22..5e47ee7c355c 100644 --- a/modules/backlogs/app/forms/backlogs/sprints/details_form.rb +++ b/modules/backlogs/app/forms/backlogs/sprints/details_form.rb @@ -31,6 +31,10 @@ module Backlogs module Sprints class DetailsForm < ApplicationForm + extend Dry::Initializer + + option :disabled, default: -> { false } + form do |f| f.hidden(name: :id) @@ -38,17 +42,10 @@ class DetailsForm < ApplicationForm label: attribute_name(:name), name: :name, required: true, - autofocus: true, + autofocus: !disabled, + disabled:, w: :full ) - - # f.text_area( - # label: attribute_name(:goal), - # name: :goal, - # required: false, - # w: :full, - # rows: 3 - # ) end end end diff --git a/modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb b/modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb new file mode 100644 index 000000000000..a577a4686270 --- /dev/null +++ b/modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Backlogs + module Sprints + class OwnedGoalForm < ApplicationForm + extend Dry::Initializer + + option :disabled, default: -> { false } + + form do |f| + f.text_field( + name: :text, + label: Sprint.human_attribute_name(:goal), + disabled: + ) + end + end + end +end diff --git a/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb b/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb new file mode 100644 index 000000000000..2e24d8db5e2c --- /dev/null +++ b/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Backlogs + module Sprints + class SharedGoalForm < ApplicationForm + extend Dry::Initializer + + option :disabled, default: -> { false } + + form do |f| + f.text_field( + name: :text, + label: goal_label, + caption: I18n.t("backlogs.sprint_form.goal_caption"), + disabled: + ) + end + + def goal_label + I18n.t("backlogs.sprint_form.goal_for_this_project_label", attribute: Sprint.human_attribute_name(:goal)) + end + end + end +end diff --git a/modules/backlogs/app/services/backlogs/sprints/set_attributes_service.rb b/modules/backlogs/app/services/backlogs/sprints/set_attributes_service.rb index 56b65b0169c9..3d82a1ae94c4 100644 --- a/modules/backlogs/app/services/backlogs/sprints/set_attributes_service.rb +++ b/modules/backlogs/app/services/backlogs/sprints/set_attributes_service.rb @@ -32,6 +32,56 @@ module Backlogs::Sprints class SetAttributesService < ::BaseServices::SetAttributes private + def set_attributes(params) + params = params.to_h.deep_symbolize_keys + + super(sprint_params_with_goal_attributes(params.fetch(:attributes, params))) + end + + def sprint_params_with_goal_attributes(params) + attributes = params.reject { |key, _| %i[goal goal_project].include?(key.to_sym) } + goal_attributes = goal_nested_attributes(params) + + if goal_attributes + attributes.merge(goals_attributes: [goal_attributes]) + else + attributes + end + end + + def goal_nested_attributes(params) + attributes = goal_params(params) + return unless attributes + + project = goal_project(params) + existing_id = existing_goal_id(project) + + return if existing_id.blank? && attributes[:text].blank? + + nested_goal_attributes(project, attributes[:text], existing_id) + end + + def goal_params(params) + params[:goal]&.to_h&.symbolize_keys + end + + def goal_project(params) + params[:goal_project] || params[:project] || model.project + end + + def nested_goal_attributes(project, text, existing_id) + attributes = { project_id: project.id, text: } + attributes[:id] = existing_id if existing_id.present? + attributes[:_destroy] = "1" if existing_id.present? && text.blank? + attributes + end + + def existing_goal_id(goal_project) + return unless model.persisted? + + model.goal_for(goal_project)&.id + end + def sprint_name_from_predecessor return model.name unless model.new_record? diff --git a/modules/backlogs/config/locales/en.yml b/modules/backlogs/config/locales/en.yml index 0c238c090fb0..df1bc9bdcb2c 100644 --- a/modules/backlogs/config/locales/en.yml +++ b/modules/backlogs/config/locales/en.yml @@ -178,6 +178,14 @@ en: edit_sprint: "Edit sprint" add_work_package: "Add work package" + sprint_form: + goal_caption: "The sprint goal is unique to this project and is not shared with other projects also using this sprint." + goal_for_this_project_label: "%{attribute} (for this project)" + + sprint_form_component: + shared_sprint_info_banner: "This is a shared sprint. Modifications will be reflected in all projects using it." + shared_sprint_warning_banner: "This is a shared sprint. You do not have the necessary permissions to edit it." + sprints_component: blankslate: title: "No sprints present yet" diff --git a/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb b/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb index 7c2876a089a4..eee5df1b46f6 100644 --- a/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb @@ -205,6 +205,43 @@ def menu_items end end + describe "edit menu visibility for shared sprints" do + let(:source_project) { create(:project, sprint_sharing: "share_all_projects", types: [type_feature, type_task]) } + let(:project) { create(:project, sprint_sharing: "receive_shared", types: [type_feature, type_task]) } + let(:sprint) do + create(:sprint, project: source_project, name: "Shared Sprint", + start_date: Date.yesterday, finish_date: Date.tomorrow) + end + + context "when user has create_sprints only in the defining project" do + let(:user) do + create(:user, + member_with_roles: { + project => create(:project_role, permissions: %i[view_sprints view_work_packages]), + source_project => create(:project_role, permissions: %i[view_sprints create_sprints]) + }) + end + + it "renders the edit sprint menu item" do + expect(rendered_component).to have_text("Edit sprint") + end + end + + context "when user has create_sprints in neither project" do + let(:user) do + create(:user, + member_with_roles: { + project => create(:project_role, permissions: %i[view_sprints view_work_packages]), + source_project => create(:project_role, permissions: %i[view_sprints]) + }) + end + + it "does not render the edit sprint menu item" do + expect(rendered_component).to have_no_text("Edit sprint") + end + end + end + describe "sprint actions in header" do context "when the sprint is in planning with date range set" do let(:sprint) do diff --git a/modules/backlogs/spec/components/backlogs/sprint_form_component_spec.rb b/modules/backlogs/spec/components/backlogs/sprint_form_component_spec.rb index 31f90239bb95..66c41ddbf5e1 100644 --- a/modules/backlogs/spec/components/backlogs/sprint_form_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/sprint_form_component_spec.rb @@ -35,7 +35,8 @@ shared_let(:sprint) { create(:sprint, project:) } let(:base_errors) { ["Sprint failed"] } - let(:component) { described_class.new(sprint:, base_errors:) } + let(:current_user) { create(:admin) } + let(:component) { described_class.new(sprint:, project:, current_user:, base_errors:) } subject(:rendered_component) do render_inline(component) @@ -56,4 +57,129 @@ it "renders base errors" do expect(rendered_component).to have_text("Sprint failed") end + + describe "goal field" do + context "when the sprint is not shared" do + it "renders the goal text field" do + expect(rendered_component).to have_field(Sprint.human_attribute_name(:goal)) + end + + it "renders the goal text field under the goal command param" do + expect(rendered_component).to have_field("sprint[goal][text]") + end + + it "does not render the goal section separator" do + expect(rendered_component).to have_no_css(".border-top.color-border-muted") + end + + it "does not render the shared sprint banner" do + expect(rendered_component).to have_no_text("This is a shared sprint") + end + + it "does not render the project suffix on the label" do + expect(rendered_component).to have_no_field( + I18n.t("backlogs.sprint_form.goal_for_this_project_label", attribute: Sprint.human_attribute_name(:goal)) + ) + end + end + + context "when a goal exists" do + before do + create(:sprint_goal, sprint:, project:, text: "Ship dashboard") + end + + it "renders the goal value" do + expect(rendered_component).to have_field(Sprint.human_attribute_name(:goal), with: "Ship dashboard") + end + end + end + + describe "shared sprint" do + let(:sharing_project) { create(:project) } + let(:sprint) { create(:sprint, project: sharing_project) } + let(:role_with_perm) { create(:project_role, permissions: %i[view_sprints create_sprints]) } + let(:role_without_perm) { create(:project_role, permissions: %i[view_sprints]) } + + context "when user has create_sprints in both projects" do + let(:current_user) do + create(:user, + member_with_roles: { project => role_with_perm, sharing_project => role_with_perm }) + end + + it "renders the info banner" do + expect(rendered_component).to have_text( + I18n.t("backlogs.sprint_form_component.shared_sprint_info_banner") + ) + end + + it "renders the goal label with project suffix" do + expect(rendered_component).to have_field( + I18n.t("backlogs.sprint_form.goal_for_this_project_label", attribute: Sprint.human_attribute_name(:goal)) + ) + end + + it "renders the goal caption" do + expect(rendered_component).to have_text(I18n.t("backlogs.sprint_form.goal_caption")) + end + + it "renders the goal section separator" do + expect(rendered_component).to have_css(".border-top.color-border-muted", count: 1) + end + + it "renders all fields as active" do + expect(rendered_component).to have_field(Sprint.human_attribute_name(:name), disabled: false) + expect(rendered_component).to have_field(Sprint.human_attribute_name(:goal), disabled: false) + end + end + + context "when user has create_sprints only in the sharing project" do + let(:current_user) do + create(:user, + member_with_roles: { project => role_without_perm, sharing_project => role_with_perm }) + end + + it "renders the info banner" do + expect(rendered_component).to have_text( + I18n.t("backlogs.sprint_form_component.shared_sprint_info_banner") + ) + end + + it "renders the goal field as disabled" do + expect(rendered_component).to have_field(Sprint.human_attribute_name(:goal), disabled: true) + end + + it "renders the name field as active" do + expect(rendered_component).to have_field(Sprint.human_attribute_name(:name), disabled: false) + end + end + + context "when user has create_sprints only in the current project" do + let(:current_user) do + create(:user, + member_with_roles: { project => role_with_perm, sharing_project => role_without_perm }) + end + + it "renders the warning banner" do + expect(rendered_component).to have_text( + I18n.t("backlogs.sprint_form_component.shared_sprint_warning_banner") + ) + end + + it "renders the name field as disabled" do + expect(rendered_component).to have_field(Sprint.human_attribute_name(:name), disabled: true) + end + + it "does not autofocus the disabled name field" do + expect(rendered_component).to have_no_css('input[name="sprint[name]"][disabled][autofocus]') + end + + it "renders duration as readonly but not disabled" do + expect(rendered_component).to have_field(Sprint.human_attribute_name(:duration), readonly: true, disabled: false) + end + + it "renders the goal field as active" do + expect(rendered_component).to have_field(Sprint.human_attribute_name(:goal), disabled: false) + end + end + end end diff --git a/modules/backlogs/spec/contracts/backlogs/sprints/shared_contract_examples.rb b/modules/backlogs/spec/contracts/backlogs/sprints/shared_contract_examples.rb index 8186fad0a2c4..6162405c45e9 100644 --- a/modules/backlogs/spec/contracts/backlogs/sprints/shared_contract_examples.rb +++ b/modules/backlogs/spec/contracts/backlogs/sprints/shared_contract_examples.rb @@ -64,12 +64,20 @@ context "when user does not have create_sprints permission" do let(:permissions) { [:view_work_packages] } + before do + sprint.name = "Changed sprint name" + end + it_behaves_like "contract is invalid", base: :error_unauthorized end context "when user has no permissions in project" do let(:permissions) { [] } + before do + sprint.name = "Changed sprint name" + end + it_behaves_like "contract is invalid", base: :error_unauthorized context "when user is admin" do diff --git a/modules/backlogs/spec/controllers/backlogs/sprints_controller_spec.rb b/modules/backlogs/spec/controllers/backlogs/sprints_controller_spec.rb index 5fd87400ded2..ff0120c0cd02 100644 --- a/modules/backlogs/spec/controllers/backlogs/sprints_controller_spec.rb +++ b/modules/backlogs/spec/controllers/backlogs/sprints_controller_spec.rb @@ -54,6 +54,15 @@ expect(assigns(:sprints)).not_to be_nil expect(assigns(:work_package_counts)).to be_a(Hash) end + + it "does not load a sprint from a stray sprint id" do + sprint = create(:sprint, project:) + + get :index, params: { project_id: project.id, sprint_id: sprint.id } + + expect(response).to be_successful + expect(assigns(:sprint)).to be_nil + end end describe "GET #new_dialog" do @@ -111,7 +120,7 @@ } end - it "responds with success, creates a sprint, and redirects to backlogs", :aggregate_failures do + it "responds with success and redirects to backlogs", :aggregate_failures do post :create, format: :turbo_stream, params: params expect(response).to be_successful @@ -121,7 +130,6 @@ action: "redirect_to", url: project_backlogs_backlog_path(project) ) - expect(project.reload.sprints.last.name).to eq("My Sprint") expect(flash[:notice]).to eq(I18n.t(:notice_successful_create)) end @@ -133,6 +141,78 @@ end end + context "with a sprint goal" do + let(:params) do + { + project_id: project.id, + sprint: { + name: "My Sprint", + start_date: "2025-10-05", + finish_date: "2025-10-15", + goal: { text: "Ship MVP" } + } + } + end + + it "creates the goal for the route project" do + expect { post :create, format: :turbo_stream, params: params } + .to change(SprintGoal, :count).by(1) + + sprint = Sprint.find_by!(name: "My Sprint") + expect(sprint.goal_text_for(project)).to eq("Ship MVP") + end + + context "with a submitted goal project id" do + let(:other_project) { create(:project) } + + let(:params) do + { + project_id: project.id, + sprint: { + name: "My Sprint", + start_date: "2025-10-05", + finish_date: "2025-10-15", + goal: { text: "Ship MVP", project_id: other_project.id } + } + } + end + + it "ignores the submitted project id" do + post :create, format: :turbo_stream, params: params + + sprint = Sprint.find_by!(name: "My Sprint") + expect(sprint.goal_text_for(project)).to eq("Ship MVP") + expect(sprint.goal_text_for(other_project)).to be_nil + end + end + + context "with a submitted sprint id" do + let!(:other_sprint) { create(:sprint, project:) } + let!(:other_goal) { create(:sprint_goal, sprint: other_sprint, project:, text: "Other goal") } + let(:params) do + { + project_id: project.id, + sprint: { + id: other_sprint.id, + name: "My Sprint", + start_date: "2025-10-05", + finish_date: "2025-10-15", + goal: { text: "Ship MVP" } + } + } + end + + it "ignores the submitted sprint id" do + post :create, format: :turbo_stream, params: params + + sprint = Sprint.find_by!(name: "My Sprint") + expect(sprint).to have_attributes(project_id: project.id) + expect(sprint.goal_text_for(project)).to eq("Ship MVP") + expect(other_goal.reload.text).to eq("Other goal") + end + end + end + context "without the 'create_sprints' permission" do let(:permissions) { all_permissions - [:create_sprints] } @@ -165,11 +245,103 @@ expect(response.body).to have_turbo_stream action: "update", target: "backlogs-sprint-component-#{sprint.id}" assert_select %(turbo-stream[action="update"][target="backlogs-sprint-component-#{sprint.id}"][method="morph"]) expect(response.body).to include("Successful update.") - expect(sprint.reload.name).to eq("Changed sprint name") expect(controller.controller_path).to eq("backlogs/sprints") expect(controller.action_name).to eq("update") end + context "with a sprint goal" do + let(:params) do + { + project_id: project.id, + sprint_id: sprint.id, + sprint: { goal: { text: "Ship MVP" } } + } + end + + it "updates the goal for the route project" do + expect { put :update, format: :turbo_stream, params: params } + .to change(SprintGoal, :count).by(1) + + expect(sprint.reload.goal_text_for(project)).to eq("Ship MVP") + end + + context "with a submitted goal project id" do + let(:other_project) { create(:project) } + let(:params) do + { + project_id: project.id, + sprint_id: sprint.id, + sprint: { goal: { text: "Ship MVP", project_id: other_project.id } } + } + end + + it "ignores the submitted project id" do + put :update, format: :turbo_stream, params: params + + expect(sprint.reload.goal_text_for(project)).to eq("Ship MVP") + expect(sprint.goal_text_for(other_project)).to be_nil + end + end + + context "with a submitted goal id from another project" do + let(:other_project) { create(:project) } + let!(:other_goal) { create(:sprint_goal, sprint:, project: other_project, text: "Other project goal") } + let(:params) do + { + project_id: project.id, + sprint_id: sprint.id, + sprint: { goal: { id: other_goal.id, text: "Ship MVP" } } + } + end + + it "ignores the submitted goal id" do + expect { put :update, format: :turbo_stream, params: params } + .to change(SprintGoal, :count).by(1) + + expect(sprint.reload.goal_text_for(project)).to eq("Ship MVP") + expect(other_goal.reload).to have_attributes(project_id: other_project.id, text: "Other project goal") + end + end + + context "when clearing with a submitted goal id from another project" do + let(:other_project) { create(:project) } + let!(:other_goal) { create(:sprint_goal, sprint:, project: other_project, text: "Other project goal") } + let(:params) do + { + project_id: project.id, + sprint_id: sprint.id, + sprint: { goal: { id: other_goal.id, text: "" } } + } + end + + it "does not destroy the submitted goal id" do + expect { put :update, format: :turbo_stream, params: params } + .not_to change(SprintGoal, :count) + + expect(sprint.reload.goal_text_for(project)).to be_nil + expect(other_goal.reload).to have_attributes(project_id: other_project.id, text: "Other project goal") + end + end + + context "when clearing an existing goal" do + let!(:goal) { create(:sprint_goal, sprint:, project:, text: "Old goal") } + let(:params) do + { + project_id: project.id, + sprint_id: sprint.id, + sprint: { goal: { text: "" } } + } + end + + it "destroys the existing goal" do + expect { put :update, format: :turbo_stream, params: params } + .to change(SprintGoal, :count).by(-1) + + expect(sprint.reload.goal_text_for(project)).to be_nil + end + end + end + context "without the 'create_sprints' permission" do let(:permissions) { all_permissions - [:create_sprints] } @@ -282,14 +454,13 @@ context "when board creation fails" do let(:service_result) { ServiceResult.failure(message: "something went wrong") } - it "redirects back to the backlog and leaves the sprint in planning", :aggregate_failures do + it "redirects back to the backlog", :aggregate_failures do post :start, params: request_params expect(response).to redirect_to(project_backlogs_backlog_path(project)) expect(flash[:alert]).to eq( I18n.t(:notice_unsuccessful_start_with_reason, reason: "something went wrong") ) - expect(sprint.reload).to be_in_planning end end @@ -314,7 +485,7 @@ ) end - it "redirects back to the backlog and leaves the sprint in planning", :aggregate_failures do + it "redirects back to the backlog", :aggregate_failures do post :start, params: request_params expect(response).to redirect_to(project_backlogs_backlog_path(project)) @@ -540,5 +711,173 @@ end end end + + describe "shared sprint authorization" do + let(:source_project) { create(:project, sprint_sharing: "share_all_projects") } + let(:project) { create(:project, sprint_sharing: "receive_shared") } + let!(:sprint) { create(:sprint, project: source_project) } + let(:role_with_perm) { create(:project_role, permissions: %i[view_sprints create_sprints]) } + let(:role_without_perm) { create(:project_role, permissions: %i[view_sprints]) } + let(:role_without_sprint_access) { create(:project_role, permissions: []) } + + describe "GET #edit_dialog" do + context "when user has create_sprints only in the viewing project" do + let(:user) do + create(:user, + member_with_roles: { project => role_with_perm, source_project => role_without_perm }) + end + + it "responds with success", :aggregate_failures do + get :edit_dialog, params: { project_id: project.id, sprint_id: sprint.id }, format: :turbo_stream + + expect(response).to be_successful + end + end + + context "when user has create_sprints only in the defining project" do + let(:user) do + create(:user, + member_with_roles: { project => role_without_perm, source_project => role_with_perm }) + end + + it "responds with success", :aggregate_failures do + get :edit_dialog, params: { project_id: project.id, sprint_id: sprint.id }, format: :turbo_stream + + expect(response).to be_successful + end + end + + context "when user has create_sprints in the defining project but no view_sprints in the viewing project" do + let(:user) do + create(:user, + member_with_roles: { project => role_without_sprint_access, source_project => role_with_perm }) + end + + it "responds with forbidden" do + get :edit_dialog, params: { project_id: project.id, sprint_id: sprint.id }, format: :turbo_stream + + expect(response).to have_http_status(:forbidden) + end + end + + context "when user has no view_sprints in the viewing project and no create_sprints in either project" do + let(:user) do + create(:user, + member_with_roles: { project => role_without_sprint_access, source_project => role_without_perm }) + end + + it "responds with forbidden" do + get :edit_dialog, params: { project_id: project.id, sprint_id: sprint.id }, format: :turbo_stream + + expect(response).to have_http_status(:forbidden) + end + end + + context "when user has create_sprints in neither project" do + let(:user) do + create(:user, + member_with_roles: { project => role_without_perm, source_project => role_without_perm }) + end + + it "responds with forbidden" do + get :edit_dialog, params: { project_id: project.id, sprint_id: sprint.id }, format: :turbo_stream + + expect(response).to have_http_status(:forbidden) + end + end + end + + describe "PUT #update" do + context "when user has create_sprints only in the viewing project" do + let(:user) do + create(:user, + member_with_roles: { project => role_with_perm, source_project => role_without_perm }) + end + + it "allows the request", :aggregate_failures do + put :update, + format: :turbo_stream, + params: { project_id: project.id, sprint_id: sprint.id, sprint: { goal: { text: "Ship MVP" } } } + + expect(response).to be_successful + end + + it "does not update sprint attributes" do + put :update, + format: :turbo_stream, + params: { project_id: project.id, sprint_id: sprint.id, sprint: { name: "Renamed" } } + + expect(response).to have_http_status(:bad_request) + expect(sprint.reload.name).not_to eq("Renamed") + end + end + + context "when user has create_sprints only in the defining project" do + let(:user) do + create(:user, + member_with_roles: { project => role_without_perm, source_project => role_with_perm }) + end + + it "allows the request", :aggregate_failures do + put :update, + format: :turbo_stream, + params: { project_id: project.id, sprint_id: sprint.id, sprint: { name: "Renamed" } } + + expect(response).to be_successful + end + end + + context "when user has create_sprints in the defining project but no view_sprints in the viewing project" do + let(:user) do + create(:user, + member_with_roles: { project => role_without_sprint_access, source_project => role_with_perm }) + end + + it "responds with forbidden" do + put :update, + format: :turbo_stream, + params: { project_id: project.id, sprint_id: sprint.id, sprint: { name: "Renamed" } } + + expect(response).to have_http_status(:forbidden) + end + end + + context "when user has create_sprints in neither project" do + let(:user) do + create(:user, + member_with_roles: { project => role_without_perm, source_project => role_without_perm }) + end + + it "responds with forbidden" do + put :update, + format: :turbo_stream, + params: { project_id: project.id, sprint_id: sprint.id, sprint: { name: "Renamed" } } + + expect(response).to have_http_status(:forbidden) + end + end + end + + describe "GET #refresh_form for shared sprint" do + let(:user) do + create(:user, + member_with_roles: { project => role_with_perm, source_project => role_without_perm }) + end + + it "preserves the sprint's defining project context", :aggregate_failures do + get :refresh_form, + format: :turbo_stream, + params: { + project_id: project.id, + sprint: { id: sprint.id, name: sprint.name } + } + + expect(response).to be_successful + expect(response.body).to include( + I18n.t("backlogs.sprint_form_component.shared_sprint_warning_banner") + ) + end + end + end end end diff --git a/modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb b/modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb new file mode 100644 index 000000000000..9b90bbc1691b --- /dev/null +++ b/modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "rails_helper" + +RSpec.describe Backlogs::Sprints::OwnedGoalForm, type: :forms do + include ViewComponent::TestHelpers + + let(:project) { create(:project) } + let(:sprint) { create(:sprint, project:) } + let(:disabled) { false } + let(:goal) { sprint.goals.find_or_initialize_by(project:) } + let(:form_arguments) { { url: "/foo", model: sprint, scope: :sprint } } + + def render_form + render_in_view_context( + described_class, + form_arguments, + goal, + disabled + ) do |described_class, form_arguments, goal, disabled| + primer_form_with(**form_arguments) do |f| + f.fields_for(:goal, goal) do |goal_fields| + render(described_class.new(goal_fields, disabled:)) + end + end + end + end + + subject(:rendered_form) do + render_form + page + end + + it "renders the goal field" do + expect(rendered_form).to have_field(Sprint.human_attribute_name(:goal), disabled: false) + end + + it "renders the goal text field under the goal command param" do + expect(rendered_form).to have_field("sprint[goal][text]") + end + + it "does not render the shared sprint caption" do + expect(rendered_form).to have_no_text(I18n.t("backlogs.sprint_form.goal_caption")) + end + + context "when a goal exists for the project" do + let!(:goal) { create(:sprint_goal, sprint:, project:, text: "Ship dashboard") } + + it "renders the goal value" do + expect(rendered_form).to have_field(Sprint.human_attribute_name(:goal), with: "Ship dashboard") + end + end + + context "when disabled" do + let(:disabled) { true } + + it "renders the goal field as disabled" do + expect(rendered_form).to have_field(Sprint.human_attribute_name(:goal), disabled: true) + end + end +end diff --git a/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb b/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb new file mode 100644 index 000000000000..967d75cfd5ab --- /dev/null +++ b/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "rails_helper" + +RSpec.describe Backlogs::Sprints::SharedGoalForm, type: :forms do + include ViewComponent::TestHelpers + + let(:project) { create(:project) } + let(:source_project) { create(:project) } + let(:sprint) { create(:sprint, project: source_project) } + let(:disabled) { false } + let(:goal) { sprint.goals.find_or_initialize_by(project:) } + let(:form_arguments) { { url: "/foo", model: sprint, scope: :sprint } } + + def render_form + render_in_view_context( + described_class, + form_arguments, + goal, + disabled + ) do |described_class, form_arguments, goal, disabled| + primer_form_with(**form_arguments) do |f| + f.fields_for(:goal, goal) do |goal_fields| + render(described_class.new(goal_fields, disabled:)) + end + end + end + end + + subject(:rendered_form) do + render_form + page + end + + it "renders the project-specific goal field" do + label = I18n.t("backlogs.sprint_form.goal_for_this_project_label", attribute: Sprint.human_attribute_name(:goal)) + + expect(rendered_form).to have_field(label, disabled: false) + end + + it "renders the goal text field under the goal command param" do + expect(rendered_form).to have_field("sprint[goal][text]") + end + + it "renders the shared sprint caption" do + expect(rendered_form).to have_text(I18n.t("backlogs.sprint_form.goal_caption")) + end + + context "when a goal exists for the project" do + let!(:goal) { create(:sprint_goal, sprint:, project:, text: "Ship dashboard") } + + it "renders the goal value" do + expect(rendered_form).to have_field(with: "Ship dashboard") + end + end + + context "when disabled" do + let(:disabled) { true } + + it "renders the goal field as disabled" do + expect(rendered_form).to have_field(Sprint.human_attribute_name(:goal), disabled: true) + end + end +end diff --git a/modules/backlogs/spec/services/backlogs/sprints/create_service_spec.rb b/modules/backlogs/spec/services/backlogs/sprints/create_service_spec.rb index 032b781a8df5..5a07892707a0 100644 --- a/modules/backlogs/spec/services/backlogs/sprints/create_service_spec.rb +++ b/modules/backlogs/spec/services/backlogs/sprints/create_service_spec.rb @@ -36,4 +36,40 @@ let(:model_class) { Sprint } let(:factory) { :sprint } end + + describe "goal persistence" do + let(:project) { create(:project) } + let(:user) do + create(:user, member_with_permissions: { project => %i[view_sprints create_sprints] }) + end + let(:goals_attributes) { [{ project_id: project.id, text: goal_text }] } + let(:attributes) do + { + project:, + name: "Sprint 1", + start_date: Time.zone.today, + finish_date: Time.zone.today + 2.weeks, + goals_attributes: + } + end + let(:goal_text) { "Ship dashboard" } + + subject(:service_call) do + described_class.new(user:).call(attributes:) + end + + it "creates a goal for the new sprint's project" do + expect { service_call }.to change(SprintGoal, :count).by(1) + + expect(service_call.result.goal_text_for(project)).to eq("Ship dashboard") + end + + context "when the goal is blank" do + let(:goal_text) { "" } + + it "does not create a goal" do + expect { service_call }.not_to change(SprintGoal, :count) + end + end + end end diff --git a/modules/backlogs/spec/services/backlogs/sprints/set_attributes_service_spec.rb b/modules/backlogs/spec/services/backlogs/sprints/set_attributes_service_spec.rb index fed759818c44..83e1cc5eb649 100644 --- a/modules/backlogs/spec/services/backlogs/sprints/set_attributes_service_spec.rb +++ b/modules/backlogs/spec/services/backlogs/sprints/set_attributes_service_spec.rb @@ -129,6 +129,50 @@ end end + context "with goal params" do + let(:other_project) { create(:project) } + let(:params) do + { + project:, + goal: { + id: 123, + project_id: other_project.id, + text: "Ship MVP" + } + } + end + + it "assigns nested goal attributes for the service project" do + service_call + + expect(sprint.goals.first).to have_attributes( + project_id: project.id, + text: "Ship MVP" + ) + end + + context "with a blank new goal" do + let(:params) { { project:, goal: { text: "" } } } + + it "does not build a goal" do + expect { service_call }.not_to change { sprint.goals.length } + end + end + + context "with an existing contextual goal" do + let(:sprint) { create(:sprint, project:) } + let!(:goal) { create(:sprint_goal, sprint:, project:, text: "Old goal") } + let(:params) { { goal: { text: "" } } } + + it "marks the goal for destruction when blanked" do + service_call + + expect(sprint.goals.find { |sprint_goal| sprint_goal.id == goal.id }) + .to be_marked_for_destruction + end + end + end + describe "default attributes" do let(:sprint) { Sprint.new } diff --git a/modules/backlogs/spec/services/backlogs/sprints/update_service_spec.rb b/modules/backlogs/spec/services/backlogs/sprints/update_service_spec.rb new file mode 100644 index 000000000000..3b338c5705f4 --- /dev/null +++ b/modules/backlogs/spec/services/backlogs/sprints/update_service_spec.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe Backlogs::Sprints::UpdateService, type: :model do + let(:project) { create(:project, sprint_sharing: Projects::SprintSharing::RECEIVE_SHARED) } + let(:source_project) { create(:project, sprint_sharing: Projects::SprintSharing::SHARE_ALL_PROJECTS) } + let(:sprint) { create(:sprint, project: source_project, name: "Sprint 1") } + let(:user) do + create(:user, member_with_permissions: { project => project_permissions, source_project => source_project_permissions }) + end + let(:project_permissions) { %i[view_sprints create_sprints] } + let(:source_project_permissions) { %i[view_sprints] } + let(:attributes) { { goals_attributes: [{ project_id: project.id, text: "Ship dashboard" }] } } + + subject(:service_call) do + described_class.new(user:, model: sprint).call(attributes:) + end + + it "persists the goal for the supplied goal project" do + expect { service_call }.to change(SprintGoal, :count).by(1) + + expect(service_call.result.goal_text_for(project)).to eq("Ship dashboard") + end + + context "when a goal already exists" do + let!(:goal) do + create(:sprint_goal, sprint:, project:, text: "Old goal") + end + let(:attributes) { { goals_attributes: [{ id: goal.id, project_id: project.id, text: "Ship dashboard" }] } } + + it "updates the existing goal" do + expect { service_call }.not_to change(SprintGoal, :count) + + expect(sprint.reload.goal_text_for(project)).to eq("Ship dashboard") + end + + context "with a blank goal" do + let(:attributes) { { goals_attributes: [{ id: goal.id, text: "", _destroy: "1" }] } } + + it "removes the existing goal" do + expect { service_call }.to change(SprintGoal, :count).by(-1) + + expect(sprint.goal_text_for(project)).to be_nil + end + end + end + + context "without create_sprints permission in the goal project" do + let(:project_permissions) { %i[view_sprints] } + + it "does not persist the goal" do + expect { service_call }.not_to change(SprintGoal, :count) + + expect(service_call).not_to be_success + end + end + + context "when the sprint is not visible to the goal project" do + let(:unrelated_project) { create(:project) } + let(:user) do + create( + :user, + member_with_permissions: { + unrelated_project => %i[view_sprints create_sprints], + source_project => source_project_permissions + } + ) + end + let(:attributes) { { goals_attributes: [{ project_id: unrelated_project.id, text: "Ship dashboard" }] } } + + it "does not persist the goal" do + expect { service_call }.not_to change(SprintGoal, :count) + + expect(service_call).not_to be_success + end + end + + context "when a duplicate goal would be created" do + before do + create(:sprint_goal, sprint:, project:, text: "Old goal") + end + + it "returns a failed service result" do + expect { service_call }.not_to change(SprintGoal, :count) + + expect(service_call).not_to be_success + expect(service_call.errors).not_to be_empty + end + end + + context "with sprint attributes" do + let(:attributes) { { name: "Renamed" } } + let(:project_permissions) { %i[view_sprints] } + let(:source_project_permissions) { %i[view_sprints create_sprints] } + + it "updates the sprint through the regular update contract" do + expect(service_call).to be_success + + expect(sprint.reload.name).to eq("Renamed") + end + end + + context "with sprint attributes and no source project edit permission" do + let(:attributes) { { name: "Renamed" } } + let(:project_permissions) { %i[view_sprints create_sprints] } + let(:source_project_permissions) { %i[view_sprints] } + + it "does not update the sprint" do + expect(service_call).not_to be_success + + expect(sprint.reload.name).to eq("Sprint 1") + end + end + + context "with sprint attributes and goal attributes" do + let(:attributes) do + { + name: "Renamed", + goals_attributes: [{ project_id: project.id, text: "Ship dashboard" }] + } + end + let(:project_permissions) { %i[view_sprints create_sprints] } + let(:source_project_permissions) { %i[view_sprints create_sprints] } + + it "updates both in one request" do + expect(service_call).to be_success + + expect(sprint.reload.name).to eq("Renamed") + expect(sprint.goal_text_for(project)).to eq("Ship dashboard") + end + end +end From 9ff8b22e333d13e3101445f016ea8ce4ef3f88d8 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Tue, 19 May 2026 17:12:17 +0200 Subject: [PATCH 03/10] [#71059] Render sprint goals in headers Shows the project-specific goal below sprint metadata and exposes it as the accessible description for the sprint heading. https://community.openproject.org/wp/71059 --- .../backlogs/sprint_component.html.erb | 6 ++++ .../backlogs/sprint_component_spec.rb | 30 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/modules/backlogs/app/components/backlogs/sprint_component.html.erb b/modules/backlogs/app/components/backlogs/sprint_component.html.erb index f837ffb245f3..684ef48e394a 100644 --- a/modules/backlogs/app/components/backlogs/sprint_component.html.erb +++ b/modules/backlogs/app/components/backlogs/sprint_component.html.erb @@ -46,6 +46,12 @@ See COPYRIGHT and LICENSE files for more details. %> <% list.with_header(title: sprint.name, title_arguments:) do |header| %> <% header.with_description(display: :flex, direction: :column, classes: "row-gap-2") do %> + <% if goal_text.present? %> + <%= render(Primer::Beta::Text.new(id: sprint_goal_id, color: :muted, mr: 2)) do %> + <%= goal_text %> + <% end %> + <% end %> + <%= render(Primer::Alpha::Stack.new(direction: :horizontal, align: :center)) do %> <%= render(Backlogs::SprintStatusBadgeComponent.new(sprint:)) %> diff --git a/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb b/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb index eee5df1b46f6..be0e13003eb4 100644 --- a/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb @@ -242,6 +242,36 @@ def menu_items end end + describe "sprint goal in header" do + context "when the sprint has a goal for the project" do + before do + create(:sprint_goal, sprint:, project:, text: "Ship the reporting dashboard") + end + + it "renders the goal text" do + expect(rendered_component).to have_text("Ship the reporting dashboard") + end + + it "describes the sprint heading with the goal text" do + expect(rendered_component).to have_heading( + "Sprint 1", + level: 4, + accessible_description: "Ship the reporting dashboard" + ) + end + end + + context "when the sprint has no goal for the project" do + it "does not render goal text" do + expect(rendered_component).to have_no_css("#sprint_#{sprint.id}_goal") + end + + it "does not describe the sprint heading" do + expect(rendered_component).to have_css("h4", text: "Sprint 1", aria: { describedby: nil }) + end + end + end + describe "sprint actions in header" do context "when the sprint is in planning with date range set" do let(:sprint) do From 5d14f5d30e38726943b442efc97c731085678a62 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Tue, 2 Jun 2026 10:27:16 +0200 Subject: [PATCH 04/10] [#71059] Cover sprint goal browser flows Adds browser specs for creating, editing, clearing, and project-specific shared sprint goals through the accessible sprint heading contract. https://community.openproject.org/wp/71059 --- .../spec/features/backlogs/create_spec.rb | 18 +++ .../spec/features/backlogs/edit_spec.rb | 137 ++++++++++++++++++ .../backlogs/spec/support/pages/backlog.rb | 14 ++ 3 files changed, 169 insertions(+) diff --git a/modules/backlogs/spec/features/backlogs/create_spec.rb b/modules/backlogs/spec/features/backlogs/create_spec.rb index 6188b36a779f..8fdadb30f878 100644 --- a/modules/backlogs/spec/features/backlogs/create_spec.rb +++ b/modules/backlogs/spec/features/backlogs/create_spec.rb @@ -96,6 +96,24 @@ expect(sprint.finish_date).to eq finish_date end + it "allows creating a new sprint with a sprint goal" do + planning_page.visit! + + planning_page.open_create_sprint_dialog + + within_dialog "New sprint" do + fill_in "Sprint name", with: "Created sprint" + fill_in "Start date", with: start_date_fmt + fill_in "Finish date", with: finish_date_fmt + fill_in "Sprint goal", with: "Deliver the first MVP scope." + + click_on "Create" + end + + expect_and_dismiss_flash(exact_message: "Successful creation.") + planning_page.expect_sprint_heading_with_goal("Created sprint", "Deliver the first MVP scope.") + end + it "previews the sprint duration when changing the dates" do planning_page.visit! diff --git a/modules/backlogs/spec/features/backlogs/edit_spec.rb b/modules/backlogs/spec/features/backlogs/edit_spec.rb index c2dd66908232..c5d345b8e02b 100644 --- a/modules/backlogs/spec/features/backlogs/edit_spec.rb +++ b/modules/backlogs/spec/features/backlogs/edit_spec.rb @@ -130,6 +130,35 @@ planning_page.expect_sprint_names_in_order("Changed name", second_sprint.name) end + it "edits and clears the sprint goal" do + planning_page.click_in_sprint_menu(first_sprint, "Edit sprint") + planning_page.expect_sprint_dialog + + within_dialog "Edit sprint" do + expect(page).to have_field("Sprint goal", with: "") + expect(page).to have_no_text("This is a shared sprint.") + + fill_in "Sprint goal", with: "Deliver the first MVP scope." + click_on "Save" + end + + expect_and_dismiss_flash(exact_message: "Successful update.") + planning_page.expect_sprint_heading_with_goal(first_sprint.name, "Deliver the first MVP scope.") + + planning_page.click_in_sprint_menu(first_sprint, "Edit sprint") + planning_page.expect_sprint_dialog + + within_dialog "Edit sprint" do + expect(page).to have_field("Sprint goal", with: "Deliver the first MVP scope.") + + fill_in "Sprint goal", with: "" + click_on "Save" + end + + expect_and_dismiss_flash(exact_message: "Successful update.") + planning_page.expect_sprint_heading_without_goal(first_sprint.name) + end + context "when lacking the 'manage_sprint_items' permission" do let(:permissions) { all_permissions - %i[manage_sprint_items] } @@ -168,6 +197,114 @@ end end + context "with a shared sprint" do + let(:project) { create(:project, sprint_sharing: Projects::SprintSharing::RECEIVE_SHARED) } + let(:source_project) { create(:project, sprint_sharing: Projects::SprintSharing::SHARE_ALL_PROJECTS) } + let!(:first_sprint) do + create(:sprint, + name: "Shared Sprint", + project: source_project, + start_date: Date.new(2025, 9, 5), + finish_date: Date.new(2025, 9, 15)) + end + + context "when the user has complete edit rights" do + let(:user) do + create(:user, member_with_permissions: { project => permissions, source_project => permissions }) + end + + it "edits shared sprint details and the project-specific goal" do + planning_page.click_in_sprint_menu(first_sprint, "Edit sprint") + planning_page.expect_sprint_dialog + + within_dialog "Edit sprint" do + expect(page) + .to have_element("x-banner", + text: "This is a shared sprint. Modifications will be reflected in all projects using it.") + expect(page).to have_field("Sprint name", with: "Shared Sprint", disabled: false) + expect(page).to have_field("Start date", disabled: false) + expect(page).to have_field("Finish date", disabled: false) + expect(page).to have_field("Duration", with: "11 days", readonly: true) + expect(page).to have_field( + "Sprint goal (for this project)", + with: "", + disabled: false, + accessible_description: + "The sprint goal is unique to this project and is not shared with other projects also using this sprint." + ) + + fill_in "Sprint name", with: "Renamed Shared Sprint" + fill_in "Sprint goal (for this project)", with: "Deliver the receiving project MVP." + click_on "Save" + end + + expect_and_dismiss_flash(exact_message: "Successful update.") + planning_page.expect_sprint_heading_with_goal("Renamed Shared Sprint", "Deliver the receiving project MVP.") + end + end + + context "when the user can edit only the receiving project goal" do + let(:user) do + create( + :user, + member_with_permissions: { + project => permissions, + source_project => all_permissions - [:create_sprints] + } + ) + end + + it "edits only the project-specific goal" do + planning_page.click_in_sprint_menu(first_sprint, "Edit sprint") + planning_page.expect_sprint_dialog + + within_dialog "Edit sprint" do + expect(page) + .to have_element("x-banner", + text: "This is a shared sprint. You do not have the necessary permissions to edit it.") + expect(page).to have_field("Sprint name", with: "Shared Sprint", disabled: true) + expect(page).to have_field("Start date", disabled: true) + expect(page).to have_field("Finish date", disabled: true) + expect(page).to have_field("Duration", with: "11 days", readonly: true) + expect(page).to have_field( + "Sprint goal (for this project)", + with: "", + disabled: false, + accessible_description: + "The sprint goal is unique to this project and is not shared with other projects also using this sprint." + ) + + fill_in "Sprint goal (for this project)", with: "Deliver the local rollout." + click_on "Save" + end + + expect_and_dismiss_flash(exact_message: "Successful update.") + planning_page.expect_sprint_heading_with_goal("Shared Sprint", "Deliver the local rollout.") + end + end + end + + context "when moving work packages from sprints" do + describe "moving to a different sprint" do + it "moves a work package to a different sprint" do + planning_page.expect_work_package_in_sprint(work_package, first_sprint) + + planning_page.click_in_work_package_menu(work_package, "Move to sprint", wait: false) + + within("#move-to-sprint-dialog") do + expect(page).to have_no_select("target_id", with_options: [first_sprint.name]) + expect(page).to have_select("target_id", with_options: [second_sprint.name]) + + select second_sprint.name, from: "target_id" + click_on "Move" + end + + planning_page.expect_work_package_not_in_sprint(work_package, first_sprint) + planning_page.expect_work_package_in_sprint(work_package, second_sprint) + end + end + end + context "without the necessary permissions" do let(:permissions) { all_permissions - %i[create_sprints start_complete_sprint] } diff --git a/modules/backlogs/spec/support/pages/backlog.rb b/modules/backlogs/spec/support/pages/backlog.rb index 1ba2304a19f4..0c682fb99090 100644 --- a/modules/backlogs/spec/support/pages/backlog.rb +++ b/modules/backlogs/spec/support/pages/backlog.rb @@ -206,6 +206,20 @@ def expect_sprint_names_in_order(*sprint_names) expect(sprint_names_in_order).to eq(sprint_names) end + def expect_sprint_heading_with_goal(sprint_name, goal_text) + within(:section, sprint_name) do + expect(page) + .to have_heading(sprint_name, level: 4, accessible_description: goal_text, exact: true) + end + end + + def expect_sprint_heading_without_goal(sprint_name) + within(:section, sprint_name) do + expect(page) + .to have_heading(sprint_name, level: 4, accessible_description: "", exact: true) + end + end + def expect_sprint_story_points(sprint, points) within(sprint_selector(sprint)) do expect(page).to have_css(".velocity", text: points.to_s) From d9e5ae7bfee9161add0092d6f1ca1626a9a26857 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Tue, 2 Jun 2026 10:27:32 +0200 Subject: [PATCH 05/10] Tighten backlog heading matchers Use exact heading matches in existing backlog feature specs so nearby content cannot satisfy heading assertions. --- modules/backlogs/spec/features/burndown/show_spec.rb | 2 +- modules/backlogs/spec/features/empty_backlogs_spec.rb | 8 ++++---- .../projects/settings/backlog_sharing_settings_spec.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/backlogs/spec/features/burndown/show_spec.rb b/modules/backlogs/spec/features/burndown/show_spec.rb index 2d565699bceb..1604e72fd259 100644 --- a/modules/backlogs/spec/features/burndown/show_spec.rb +++ b/modules/backlogs/spec/features/burndown/show_spec.rb @@ -51,7 +51,7 @@ planning_page.click_in_sprint_menu(sprint, "Burndown chart") expect(page) - .to have_heading(sprint.name, level: 2) + .to have_heading(sprint.name, level: 2, exact: true) expect(page) .to have_text "#{sprint.start_date.strftime('%m/%d/%Y')} – #{sprint.finish_date.strftime('%m/%d/%Y')}" expect(page) diff --git a/modules/backlogs/spec/features/empty_backlogs_spec.rb b/modules/backlogs/spec/features/empty_backlogs_spec.rb index 9d5dfdf6d0bd..030ee250b244 100644 --- a/modules/backlogs/spec/features/empty_backlogs_spec.rb +++ b/modules/backlogs/spec/features/empty_backlogs_spec.rb @@ -49,12 +49,12 @@ it "shows blankslate with description" do within "#owner_backlogs_container .blankslate" do - expect(page).to have_heading("Backlog inbox is empty") + expect(page).to have_heading("Backlog inbox is empty", exact: true) expect(page).to have_text("All open work packages in this project will automatically appear here.") end within "#sprint_backlogs_container .blankslate" do - expect(page).to have_heading("No sprints present yet") + expect(page).to have_heading("No sprints present yet", exact: true) expect(page).to have_text("To start planning your sprint, create one here") expect(page).to have_link("project settings") end @@ -67,12 +67,12 @@ it "shows a blankslate without description" do within "#owner_backlogs_container .blankslate" do - expect(page).to have_heading("Backlog inbox is empty") + expect(page).to have_heading("Backlog inbox is empty", exact: true) expect(page).to have_text("All open work packages in this project will automatically appear here.") end within "#sprint_backlogs_container .blankslate" do - expect(page).to have_heading("No sprints present yet") + expect(page).to have_heading("No sprints present yet", exact: true) expect(page).to have_text("No sprints are available for this project yet.") end end diff --git a/modules/backlogs/spec/features/projects/settings/backlog_sharing_settings_spec.rb b/modules/backlogs/spec/features/projects/settings/backlog_sharing_settings_spec.rb index ca98d363b7f3..2dd9a27e97e1 100644 --- a/modules/backlogs/spec/features/projects/settings/backlog_sharing_settings_spec.rb +++ b/modules/backlogs/spec/features/projects/settings/backlog_sharing_settings_spec.rb @@ -178,7 +178,7 @@ it "does not show the sharing tab and forbids direct route access" do visit project_settings_backlogs_path(project) - expect(page).to have_heading(I18n.t(:label_backlogs)) + expect(page).to have_heading(I18n.t(:label_backlogs), exact: true) expect(page).to have_no_link(I18n.t("backlogs.sharing")) visit project_settings_backlog_sharing_path(project) From 6d50d02a02ed3160809974b46181a1b9e6b07c1f Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Mon, 1 Jun 2026 22:12:02 +0200 Subject: [PATCH 06/10] [#71059] Limit sprint goal text length Keep sprint goals short enough for dialog entry and header display while leaving the column type unchanged. https://community.openproject.org/wp/71059 --- .../backlogs/app/forms/backlogs/sprints/owned_goal_form.rb | 3 ++- .../app/forms/backlogs/sprints/shared_goal_form.rb | 3 ++- modules/backlogs/app/models/sprint_goal.rb | 4 +++- .../spec/forms/backlogs/sprints/owned_goal_form_spec.rb | 6 ++++++ .../spec/forms/backlogs/sprints/shared_goal_form_spec.rb | 6 ++++++ modules/backlogs/spec/models/sprint_goal_spec.rb | 7 +++++++ 6 files changed, 26 insertions(+), 3 deletions(-) diff --git a/modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb b/modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb index a577a4686270..4536a51ff7b8 100644 --- a/modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb +++ b/modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb @@ -39,7 +39,8 @@ class OwnedGoalForm < ApplicationForm f.text_field( name: :text, label: Sprint.human_attribute_name(:goal), - disabled: + disabled:, + maxlength: SprintGoal::TEXT_MAX_LENGTH ) end end diff --git a/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb b/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb index 2e24d8db5e2c..298b2e908ff2 100644 --- a/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb +++ b/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb @@ -40,7 +40,8 @@ class SharedGoalForm < ApplicationForm name: :text, label: goal_label, caption: I18n.t("backlogs.sprint_form.goal_caption"), - disabled: + disabled:, + maxlength: SprintGoal::TEXT_MAX_LENGTH ) end diff --git a/modules/backlogs/app/models/sprint_goal.rb b/modules/backlogs/app/models/sprint_goal.rb index 9fe1b8aa720a..0805636d42f7 100644 --- a/modules/backlogs/app/models/sprint_goal.rb +++ b/modules/backlogs/app/models/sprint_goal.rb @@ -29,12 +29,14 @@ #++ class SprintGoal < ApplicationRecord + TEXT_MAX_LENGTH = 500 + belongs_to :sprint, inverse_of: :goals belongs_to :project normalizes :text, with: ->(text) { text.strip.presence } - validates :text, presence: true + validates :text, presence: true, length: { maximum: TEXT_MAX_LENGTH } validates :project_id, uniqueness: { scope: :sprint_id, message: :project_already_has_goal } diff --git a/modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb b/modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb index 9b90bbc1691b..8f082453c33a 100644 --- a/modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb +++ b/modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb @@ -67,6 +67,12 @@ def render_form expect(rendered_form).to have_field("sprint[goal][text]") end + it "limits the goal text field length" do + expect(rendered_form).to have_css( + "input[name='sprint[goal][text]'][maxlength='#{SprintGoal::TEXT_MAX_LENGTH}']" + ) + end + it "does not render the shared sprint caption" do expect(rendered_form).to have_no_text(I18n.t("backlogs.sprint_form.goal_caption")) end diff --git a/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb b/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb index 967d75cfd5ab..92b20631eef9 100644 --- a/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb +++ b/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb @@ -70,6 +70,12 @@ def render_form expect(rendered_form).to have_field("sprint[goal][text]") end + it "limits the goal text field length" do + expect(rendered_form).to have_css( + "input[name='sprint[goal][text]'][maxlength='#{SprintGoal::TEXT_MAX_LENGTH}']" + ) + end + it "renders the shared sprint caption" do expect(rendered_form).to have_text(I18n.t("backlogs.sprint_form.goal_caption")) end diff --git a/modules/backlogs/spec/models/sprint_goal_spec.rb b/modules/backlogs/spec/models/sprint_goal_spec.rb index 6750e7faef82..a387539a2edf 100644 --- a/modules/backlogs/spec/models/sprint_goal_spec.rb +++ b/modules/backlogs/spec/models/sprint_goal_spec.rb @@ -59,6 +59,13 @@ expect(sprint_goal).not_to be_valid end + it "limits text to 500 characters" do + sprint_goal.text = "a" * 501 + + expect(sprint_goal).not_to be_valid + expect(sprint_goal.errors).to be_added(:text, :too_long, count: 500) + end + it "validates uniqueness of project_id scoped to sprint_id" do sprint_goal.save! expect(sprint_goal).to validate_uniqueness_of(:project_id) From 4cdc94acb2b85ba8be0b76ea0ea845c7fe3fcb38 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Mon, 1 Jun 2026 22:12:24 +0200 Subject: [PATCH 07/10] [#71059] Adjust sprint form error spacing Let the flex row own the vertical spacing so the danger banner aligns with the other sprint form rows. https://community.openproject.org/wp/71059 --- .../app/components/backlogs/sprint_form_component.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb b/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb index b7c5b696e22f..ee779cf05e8d 100644 --- a/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb +++ b/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb @@ -41,7 +41,7 @@ See COPYRIGHT and LICENSE files for more details. flex_layout(mb: 2) do |flex| if base_errors&.any? flex.with_row(mb: 3) do - render(Primer::Alpha::Banner.new(mb: 3, scheme: :danger)) { base_errors.join("\n") } + render(Primer::Alpha::Banner.new(scheme: :danger)) { base_errors.join("\n") } end end if shared_sprint? From 60798b5365a02dbf3ab9f27cd7538bfadba2af72 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Wed, 17 Jun 2026 13:18:44 +0200 Subject: [PATCH 08/10] [#71059] Merge sprint goal forms Use one configurable form for owned and shared sprint goal fields. --- .../backlogs/sprint_form_component.html.erb | 7 +- .../backlogs/sprint_form_component.rb | 10 +- .../{owned_goal_form.rb => goal_form.rb} | 7 +- .../backlogs/sprints/shared_goal_form.rb | 53 ---------- ...ed_goal_form_spec.rb => goal_form_spec.rb} | 34 +++++-- .../backlogs/sprints/shared_goal_form_spec.rb | 98 ------------------- 6 files changed, 43 insertions(+), 166 deletions(-) rename modules/backlogs/app/forms/backlogs/sprints/{owned_goal_form.rb => goal_form.rb} (91%) delete mode 100644 modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb rename modules/backlogs/spec/forms/backlogs/sprints/{owned_goal_form_spec.rb => goal_form_spec.rb} (71%) delete mode 100644 modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb diff --git a/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb b/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb index ee779cf05e8d..914428cdc19a 100644 --- a/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb +++ b/modules/backlogs/app/components/backlogs/sprint_form_component.html.erb @@ -62,7 +62,12 @@ See COPYRIGHT and LICENSE files for more details. end flex.with_row do f.fields_for(:goal, goal) do |goal_fields| - render goal_form_class.new(goal_fields, disabled: !can_edit_goal?) + render Backlogs::Sprints::GoalForm.new( + goal_fields, + label: goal_label, + caption: goal_caption, + disabled: !can_edit_goal? + ) end end end diff --git a/modules/backlogs/app/components/backlogs/sprint_form_component.rb b/modules/backlogs/app/components/backlogs/sprint_form_component.rb index a538f40e219f..2bed573eda4f 100644 --- a/modules/backlogs/app/components/backlogs/sprint_form_component.rb +++ b/modules/backlogs/app/components/backlogs/sprint_form_component.rb @@ -80,14 +80,18 @@ def goal sprint.goals.find_or_initialize_by(project:) end - def goal_form_class + def goal_label if shared_sprint? - Backlogs::Sprints::SharedGoalForm + I18n.t("backlogs.sprint_form.goal_for_this_project_label", attribute: Sprint.human_attribute_name(:goal)) else - Backlogs::Sprints::OwnedGoalForm + Sprint.human_attribute_name(:goal) end end + def goal_caption + I18n.t("backlogs.sprint_form.goal_caption") if shared_sprint? + end + private def http_verb diff --git a/modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb b/modules/backlogs/app/forms/backlogs/sprints/goal_form.rb similarity index 91% rename from modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb rename to modules/backlogs/app/forms/backlogs/sprints/goal_form.rb index 4536a51ff7b8..da88ce36fdd9 100644 --- a/modules/backlogs/app/forms/backlogs/sprints/owned_goal_form.rb +++ b/modules/backlogs/app/forms/backlogs/sprints/goal_form.rb @@ -30,15 +30,18 @@ module Backlogs module Sprints - class OwnedGoalForm < ApplicationForm + class GoalForm < ApplicationForm extend Dry::Initializer + option :label + option :caption, optional: true option :disabled, default: -> { false } form do |f| f.text_field( name: :text, - label: Sprint.human_attribute_name(:goal), + label:, + caption:, disabled:, maxlength: SprintGoal::TEXT_MAX_LENGTH ) diff --git a/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb b/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb deleted file mode 100644 index 298b2e908ff2..000000000000 --- a/modules/backlogs/app/forms/backlogs/sprints/shared_goal_form.rb +++ /dev/null @@ -1,53 +0,0 @@ -# frozen_string_literal: true - -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) the OpenProject GmbH -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License version 3. -# -# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -# Copyright (C) 2006-2013 Jean-Philippe Lang -# Copyright (C) 2010-2013 the ChiliProject Team -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -# -# See COPYRIGHT and LICENSE files for more details. -#++ - -module Backlogs - module Sprints - class SharedGoalForm < ApplicationForm - extend Dry::Initializer - - option :disabled, default: -> { false } - - form do |f| - f.text_field( - name: :text, - label: goal_label, - caption: I18n.t("backlogs.sprint_form.goal_caption"), - disabled:, - maxlength: SprintGoal::TEXT_MAX_LENGTH - ) - end - - def goal_label - I18n.t("backlogs.sprint_form.goal_for_this_project_label", attribute: Sprint.human_attribute_name(:goal)) - end - end - end -end diff --git a/modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb b/modules/backlogs/spec/forms/backlogs/sprints/goal_form_spec.rb similarity index 71% rename from modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb rename to modules/backlogs/spec/forms/backlogs/sprints/goal_form_spec.rb index 8f082453c33a..4d7a1c9c39f0 100644 --- a/modules/backlogs/spec/forms/backlogs/sprints/owned_goal_form_spec.rb +++ b/modules/backlogs/spec/forms/backlogs/sprints/goal_form_spec.rb @@ -30,13 +30,15 @@ require "rails_helper" -RSpec.describe Backlogs::Sprints::OwnedGoalForm, type: :forms do +RSpec.describe Backlogs::Sprints::GoalForm, type: :forms do include ViewComponent::TestHelpers let(:project) { create(:project) } let(:sprint) { create(:sprint, project:) } - let(:disabled) { false } let(:goal) { sprint.goals.find_or_initialize_by(project:) } + let(:label) { Sprint.human_attribute_name(:goal) } + let(:caption) { nil } + let(:disabled) { false } let(:form_arguments) { { url: "/foo", model: sprint, scope: :sprint } } def render_form @@ -44,11 +46,13 @@ def render_form described_class, form_arguments, goal, + label, + caption, disabled - ) do |described_class, form_arguments, goal, disabled| + ) do |described_class, form_arguments, goal, label, caption, disabled| primer_form_with(**form_arguments) do |f| f.fields_for(:goal, goal) do |goal_fields| - render(described_class.new(goal_fields, disabled:)) + render(described_class.new(goal_fields, label:, caption:, disabled:)) end end end @@ -59,8 +63,8 @@ def render_form page end - it "renders the goal field" do - expect(rendered_form).to have_field(Sprint.human_attribute_name(:goal), disabled: false) + it "renders the goal field with the provided label" do + expect(rendered_form).to have_field(label, disabled: false) end it "renders the goal text field under the goal command param" do @@ -73,15 +77,27 @@ def render_form ) end - it "does not render the shared sprint caption" do + it "does not render a blank caption" do expect(rendered_form).to have_no_text(I18n.t("backlogs.sprint_form.goal_caption")) end + context "with a caption" do + let(:label) do + I18n.t("backlogs.sprint_form.goal_for_this_project_label", attribute: Sprint.human_attribute_name(:goal)) + end + let(:caption) { I18n.t("backlogs.sprint_form.goal_caption") } + + it "renders the project-specific label and caption" do + expect(rendered_form).to have_field(label, disabled: false) + expect(rendered_form).to have_text(caption) + end + end + context "when a goal exists for the project" do let!(:goal) { create(:sprint_goal, sprint:, project:, text: "Ship dashboard") } it "renders the goal value" do - expect(rendered_form).to have_field(Sprint.human_attribute_name(:goal), with: "Ship dashboard") + expect(rendered_form).to have_field(label, with: "Ship dashboard") end end @@ -89,7 +105,7 @@ def render_form let(:disabled) { true } it "renders the goal field as disabled" do - expect(rendered_form).to have_field(Sprint.human_attribute_name(:goal), disabled: true) + expect(rendered_form).to have_field(label, disabled: true) end end end diff --git a/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb b/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb deleted file mode 100644 index 92b20631eef9..000000000000 --- a/modules/backlogs/spec/forms/backlogs/sprints/shared_goal_form_spec.rb +++ /dev/null @@ -1,98 +0,0 @@ -# frozen_string_literal: true - -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) the OpenProject GmbH -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License version 3. -# -# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -# Copyright (C) 2006-2013 Jean-Philippe Lang -# Copyright (C) 2010-2013 the ChiliProject Team -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -# -# See COPYRIGHT and LICENSE files for more details. -#++ - -require "rails_helper" - -RSpec.describe Backlogs::Sprints::SharedGoalForm, type: :forms do - include ViewComponent::TestHelpers - - let(:project) { create(:project) } - let(:source_project) { create(:project) } - let(:sprint) { create(:sprint, project: source_project) } - let(:disabled) { false } - let(:goal) { sprint.goals.find_or_initialize_by(project:) } - let(:form_arguments) { { url: "/foo", model: sprint, scope: :sprint } } - - def render_form - render_in_view_context( - described_class, - form_arguments, - goal, - disabled - ) do |described_class, form_arguments, goal, disabled| - primer_form_with(**form_arguments) do |f| - f.fields_for(:goal, goal) do |goal_fields| - render(described_class.new(goal_fields, disabled:)) - end - end - end - end - - subject(:rendered_form) do - render_form - page - end - - it "renders the project-specific goal field" do - label = I18n.t("backlogs.sprint_form.goal_for_this_project_label", attribute: Sprint.human_attribute_name(:goal)) - - expect(rendered_form).to have_field(label, disabled: false) - end - - it "renders the goal text field under the goal command param" do - expect(rendered_form).to have_field("sprint[goal][text]") - end - - it "limits the goal text field length" do - expect(rendered_form).to have_css( - "input[name='sprint[goal][text]'][maxlength='#{SprintGoal::TEXT_MAX_LENGTH}']" - ) - end - - it "renders the shared sprint caption" do - expect(rendered_form).to have_text(I18n.t("backlogs.sprint_form.goal_caption")) - end - - context "when a goal exists for the project" do - let!(:goal) { create(:sprint_goal, sprint:, project:, text: "Ship dashboard") } - - it "renders the goal value" do - expect(rendered_form).to have_field(with: "Ship dashboard") - end - end - - context "when disabled" do - let(:disabled) { true } - - it "renders the goal field as disabled" do - expect(rendered_form).to have_field(Sprint.human_attribute_name(:goal), disabled: true) - end - end -end From 7c6939f83b79d5aac334fc572905d3d3252aba8d Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Wed, 17 Jun 2026 13:41:47 +0200 Subject: [PATCH 09/10] Allow project-specific permission checks Let backlogs helpers check permissions against an explicit project when shared sprint UI needs source-project access. --- .../components/backlogs/sprint_component.rb | 4 ---- .../app/helpers/backlogs/common_helper.rb | 4 ++-- .../helpers/backlogs/common_helper_spec.rb | 24 +++++++++++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/modules/backlogs/app/components/backlogs/sprint_component.rb b/modules/backlogs/app/components/backlogs/sprint_component.rb index b40ce46129b7..3bc4cfe935a6 100644 --- a/modules/backlogs/app/components/backlogs/sprint_component.rb +++ b/modules/backlogs/app/components/backlogs/sprint_component.rb @@ -144,9 +144,5 @@ def can_open_edit_dialog? user_allowed?(:create_sprints) || user_allowed?(:create_sprints, project: sprint.project) end end - - def user_allowed?(permission, project: self.project) - current_user.allowed_in_project?(permission, project) - end end end diff --git a/modules/backlogs/app/helpers/backlogs/common_helper.rb b/modules/backlogs/app/helpers/backlogs/common_helper.rb index 4fceff480dbf..0b30aeba766f 100644 --- a/modules/backlogs/app/helpers/backlogs/common_helper.rb +++ b/modules/backlogs/app/helpers/backlogs/common_helper.rb @@ -30,8 +30,8 @@ module Backlogs module CommonHelper - def user_allowed?(permission) - current_user.allowed_in_project?(permission, project) + def user_allowed?(permission, project: nil) + current_user.allowed_in_project?(permission, project || self.project) end def backlog_bucket_creation_allowed? diff --git a/modules/backlogs/spec/helpers/backlogs/common_helper_spec.rb b/modules/backlogs/spec/helpers/backlogs/common_helper_spec.rb index 642b78319bfb..2e1a5b64916b 100644 --- a/modules/backlogs/spec/helpers/backlogs/common_helper_spec.rb +++ b/modules/backlogs/spec/helpers/backlogs/common_helper_spec.rb @@ -31,6 +31,30 @@ require "rails_helper" RSpec.describe Backlogs::CommonHelper do + describe "#user_allowed?" do + let(:user) { build_stubbed(:user) } + let(:default_project) { build_stubbed(:project) } + let(:explicit_project) { build_stubbed(:project) } + + before do + without_partial_double_verification do + allow(helper).to receive_messages(current_user: user, project: default_project) + end + + mock_permissions_for(user) do |mock| + mock.allow_in_project(:create_sprints, project: explicit_project) + end + end + + it "checks permissions in the provided project when given" do + expect(helper.user_allowed?(:create_sprints, project: explicit_project)).to be true + end + + it "checks permissions in the default project when none is given" do + expect(helper.user_allowed?(:create_sprints)).to be false + end + end + describe "#show_all_backlog" do before do allow(helper).to receive(:params).and_return(params) From c8a72977992640477d10af1033f5368799024954 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Wed, 17 Jun 2026 20:58:23 +0200 Subject: [PATCH 10/10] Truncate long goals with ExpandableTextComponent --- .../app/components/backlogs/sprint_component.html.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/backlogs/app/components/backlogs/sprint_component.html.erb b/modules/backlogs/app/components/backlogs/sprint_component.html.erb index 684ef48e394a..6837e3180806 100644 --- a/modules/backlogs/app/components/backlogs/sprint_component.html.erb +++ b/modules/backlogs/app/components/backlogs/sprint_component.html.erb @@ -48,7 +48,9 @@ See COPYRIGHT and LICENSE files for more details. <% header.with_description(display: :flex, direction: :column, classes: "row-gap-2") do %> <% if goal_text.present? %> <%= render(Primer::Beta::Text.new(id: sprint_goal_id, color: :muted, mr: 2)) do %> - <%= goal_text %> + <%= render(OpPrimer::ExpandableTextComponent.new(truncate: :multi_line, lines: 3)) do %> + <%= goal_text %> + <% end %> <% end %> <% end %>