Merge pull request #23629 from opf/fix/meeting-backlog-section

Expose the backlog section visibly through the meetings API, ensuring it appears

docs/api/apiv3/components/schemas/meeting_section_model.yml

@@ -23,6 +23,10 @@ properties:
   position:
     type: integer
     description: The position of the section within the meeting.
+  backlog:
+    type: boolean
+    description: |-
+      Whether this section is the meeting's backlog. Backlog sections are read-only via the API.
   createdAt:
     type: string
     format: date-time

docs/api/apiv3/paths/meeting_section.yml

@@ -5,7 +5,12 @@ get:
   operationId: get_meeting_section
   tags:
     - Meetings
-  description: Retrieve an individual meeting section.
+  description: |-
+    Retrieve an individual meeting section.
+
+    The backlog section can be retrieved via this endpoint even though it is not part of the regular
+    `meeting.sections` association. Its `_links.meeting` points to the owning meeting (the series
+    template for recurring meeting occurrences).
   parameters:
     - description: Section identifier
       example: 1
@@ -34,7 +39,10 @@ patch:
   operationId: update_meeting_section
   tags:
     - Meetings
-  description: Updates the given meeting section.
+  description: |-
+    Updates the given meeting section.
+
+    The backlog section cannot be updated and will return `422 Unprocessable Entity`.
   parameters:
     - description: Section identifier
       example: 1
@@ -88,7 +96,10 @@ delete:
   operationId: delete_meeting_section
   tags:
     - Meetings
-  description: Deletes the meeting section.
+  description: |-
+    Deletes the meeting section and all its agenda items.
+
+    The backlog section cannot be deleted and will return `422 Unprocessable Entity`.
   parameters:
     - description: Section identifier
       example: 1

docs/api/apiv3/paths/meeting_section_by_meeting.yml

@@ -5,7 +5,13 @@ get:
   operationId: get_meeting_section_by_meeting
   tags:
     - Meetings
-  description: Retrieve an individual section of a meeting.
+  description: |-
+    Retrieve an individual section of a meeting.
+
+    The backlog section can be retrieved via this nested endpoint even though it is not part of the
+    regular `meeting.sections` association. Its `_links.self` and `_links.meeting` point to the
+    canonical flat section resource and the owning meeting (the series template for recurring
+    meeting occurrences).
   parameters:
     - description: Meeting identifier
       example: 1

docs/api/apiv3/paths/meeting_sections_by_meeting.yml

@@ -5,7 +5,11 @@ get:
   operationId: list_meeting_sections
   tags:
     - Meetings
-  description: Lists all sections for the given meeting.
+  description: |-
+    Lists all sections for the given meeting, including the backlog section as the last element.
+
+    The backlog section is read-only and its `_links.meeting` points to the owning meeting (the
+    series template for recurring meeting occurrences, otherwise the meeting itself).
   parameters:
     - description: Meeting identifier
       example: 1

lib/api/decorators/linked_resource.rb

@@ -120,7 +120,7 @@ module API
                    getter:,
                    setter:,
                    if: show_if,
-                   skip_render: ->(*) { !embed_links || (skip_render && instance_exec(&skip_render)) },
+                   skip_render: ->(*) { !embed_link?(name) || (skip_render && instance_exec(&skip_render)) },
                    linked_resource: true,
                    embedded:,
                    uncacheable: true
@@ -142,7 +142,7 @@ module API
                    getter:,
                    setter:,
                    if: show_if,
-                   skip_render: ->(*) { !embed_links || (skip_render && instance_exec(&skip_render)) },
+                   skip_render: ->(*) { !embed_link?(name) || (skip_render && instance_exec(&skip_render)) },
                    linked_resource: true,
                    embedded:,
                    uncacheable: true
@@ -177,7 +177,7 @@ module API
                                 link_getter: :"#{name}_id",
                                 link_property_name: nil,
                                 uncacheable_link: false,
-                                getter: associated_resource_default_getter(name, representer),
+                                getter: associated_resource_default_getter(name, representer, as || name),
                                 setter: associated_resource_default_setter(name, as, v3_path),
                                 link: associated_resource_default_link_lambda(link_property_name || name,
                                                                               v3_path:,
@@ -243,11 +243,12 @@ module API
         end
 
         def associated_resource_default_getter(name,
-                                               representer)
+                                               representer,
+                                               embed_name)
           representer ||= default_representer(name)
 
           ->(*) do
-            if embed_links && represented.send(name)
+            if embed_link?(embed_name) && represented.send(name)
               representer.create(represented.send(name), current_user:)
             end
           end
@@ -290,7 +291,7 @@ module API
                                  skip_link: skip_render,
                                  link_title_attribute: :name,
                                  uncacheable_link: false,
-                                 getter: associated_resources_default_getter(name, representer),
+                                 getter: associated_resources_default_getter(name, representer, as),
                                  setter: associated_resources_default_setter(name, v3_path),
                                  link: associated_resources_default_link(name,
                                                                          v3_path:,
@@ -305,10 +306,13 @@ module API
         end
 
         def associated_resources_default_getter(name,
-                                                representer)
+                                                representer,
+                                                embed_name)
           representer ||= default_representer(name.to_s.singularize)
 
           ->(*) do
+            next unless embed_link?(embed_name)
+
             represented.send(name)&.map do |associated|
               representer.create(associated, current_user:)
             end

lib/api/decorators/single.rb

@@ -94,6 +94,17 @@ module API
         end
       end
 
+      def embed_link?(name)
+        case embed_links
+        when true
+          true
+        when false, nil
+          false
+        else
+          embed_links.include?(name)
+        end
+      end
+
       # If a subclass does not depend on a model being passed to this class, it can override
       # this method and return false. Otherwise it will be enforced that the model of each
       # representer is non-nil.

modules/meeting/app/contracts/meeting_sections/delete_contract.rb

@@ -32,5 +32,11 @@ module MeetingSections
     include EditableItem
 
     delete_permission :manage_agendas
+
+    validate :backlog_not_deletable
+
+    def backlog_not_deletable
+      errors.add :base, :error_readonly if model.backlog?
+    end
   end
 end

modules/meeting/app/contracts/meeting_sections/update_contract.rb

@@ -30,6 +30,7 @@
 module MeetingSections
   class UpdateContract < BaseContract
     validate :user_allowed_to_edit
+    validate :backlog_not_editable
 
     # We allow an empty title internally via create to mark an untitled/implicit section
     # but users should not be able to update it with an empty title through this contract
@@ -44,5 +45,9 @@ module MeetingSections
         errors.add :base, :error_unauthorized
       end
     end
+
+    def backlog_not_editable
+      errors.add :base, :error_readonly if model.backlog?
+    end
   end
 end

modules/meeting/lib/api/v3/meeting_agenda_items/meeting_agenda_item_collection_representer.rb

@@ -32,14 +32,12 @@ module API
   module V3
     module MeetingAgendaItems
       class MeetingAgendaItemCollectionRepresenter < ::API::Decorators::UnpaginatedCollection
-        # Force `embed_links` on the elements so that their associated resources
-        # (most notably the outcomes) are embedded by default, mirroring the
-        # single agenda item endpoint. All of these associations are already
-        # eager loaded by the element representer, so this adds no extra queries.
+        # Embed outcomes and sections by default without repeating already known
+        # resources such as the parent meeting.
         collection :elements,
                    getter: ->(*) {
                      represented.map do |model|
-                       element_decorator.create(model, current_user:, embed_links: true)
+                       element_decorator.create(model, current_user:, embed_links: %i[section outcomes])
                      end
                    },
                    exec_context: :decorator,

modules/meeting/lib/api/v3/meeting_sections/meeting_section_representer.rb

@@ -46,6 +46,8 @@ module API
 
         property :position
 
+        property :backlog
+
         associated_resource :meeting,
                             link: ->(*) {
                               {

modules/meeting/lib/api/v3/meeting_sections/sections_by_meeting_api.rb

@@ -32,9 +32,17 @@ module API
   module V3
     module MeetingSections
       class SectionsByMeetingAPI < ::API::OpenProjectAPI
+        helpers do
+          def find_backlog_section(id)
+            backlog = @meeting.backlog
+            backlog if backlog&.id == id
+          end
+        end
+
         resources :sections do
           get do
-            sections = @meeting.sections
+            sections = @meeting.sections.to_a
+            sections << @meeting.backlog if @meeting.backlog.present?
             MeetingSectionCollectionRepresenter.new(sections,
                                                     self_link: api_v3_paths.meeting_sections(meeting_id: @meeting.id),
                                                     current_user:)
@@ -42,7 +50,9 @@ module API
 
           route_param :section_id, type: Integer, desc: "Section ID" do
             after_validation do
-              @meeting_section = @meeting.sections.find(declared_params[:section_id])
+              @meeting_section = @meeting.sections.find_by(id: declared_params[:section_id]) ||
+                                 find_backlog_section(declared_params[:section_id])
+              raise API::Errors::NotFound.new unless @meeting_section
             end
 
             get &::API::V3::Utilities::Endpoints::Show.new(model: MeetingSection).mount

modules/meeting/spec/contracts/meeting_sections/delete_contract_spec.rb

@@ -62,6 +62,12 @@ RSpec.describe MeetingSections::DeleteContract do
 
       it_behaves_like "contract is valid"
     end
+
+    context "when the section is the backlog" do
+      let(:section) { meeting.backlog }
+
+      it_behaves_like "contract is invalid", base: :error_readonly
+    end
   end
 
   context "without permission" do

modules/meeting/spec/contracts/meeting_sections/update_contract_spec.rb

@@ -54,6 +54,12 @@ RSpec.describe MeetingSections::UpdateContract do
 
       it_behaves_like "contract is invalid", base: I18n.t(:text_agenda_item_not_editable_anymore)
     end
+
+    context "when the section is the backlog" do
+      let(:section) { meeting.backlog }
+
+      it_behaves_like "contract is invalid", base: :error_readonly
+    end
   end
 
   context "without permission" do

modules/meeting/spec/requests/api/v3/meeting_agenda_items/agenda_items_by_meeting_resource_spec.rb

@@ -83,6 +83,64 @@ RSpec.describe "API v3 Meeting Agenda Items sub-resource", content_type: :json d
         .at_path("_embedded/elements/0/_links/section/href")
     end
 
+    it "only embeds outcomes and sections for the agenda items" do
+      expect(last_response.body)
+        .to have_json_path("_embedded/elements/0/_embedded/outcomes")
+
+      expect(last_response.body)
+        .to be_json_eql(section.id.to_json)
+        .at_path("_embedded/elements/0/_embedded/section/id")
+
+      expect(last_response.body)
+        .not_to have_json_path("_embedded/elements/0/_embedded/meeting")
+
+      expect(last_response.body)
+        .not_to have_json_path("_embedded/elements/0/_embedded/author")
+    end
+
+    context "with an agenda item in the meeting backlog" do
+      let(:backlog) { meeting.backlog }
+      let!(:backlog_agenda_item) do
+        create(:meeting_agenda_item,
+               meeting:,
+               meeting_section: backlog,
+               author: current_user,
+               title: "Backlog agenda item")
+      end
+
+      before do
+        get path
+      end
+
+      it "embeds and links the backlog section" do
+        elements = JSON.parse(last_response.body).dig("_embedded", "elements")
+        element = elements.find { |item| item["id"] == backlog_agenda_item.id }
+        embedded_section = element.dig("_embedded", "section")
+
+        expect(element.dig("_links", "section", "href"))
+          .to eq(api_v3_paths.meeting_section(backlog.id))
+        expect(element.dig("_links", "section", "title")).to eq(backlog.title)
+        expect(embedded_section["id"]).to eq(backlog.id)
+        expect(embedded_section["backlog"]).to be(true)
+      end
+
+      it "uses a retrievable backlog section link with canonical meeting ownership" do
+        elements = JSON.parse(last_response.body).dig("_embedded", "elements")
+        backlog_section_href = elements
+          .find { |item| item["id"] == backlog_agenda_item.id }
+          .dig("_links", "section", "href")
+
+        get backlog_section_href
+
+        expect(last_response).to have_http_status(:ok)
+        expect(last_response.body).to be_json_eql(backlog.id.to_json).at_path("id")
+        expect(last_response.body).to be_json_eql(true.to_json).at_path("backlog")
+        expect(last_response.body)
+          .to be_json_eql(api_v3_paths.meeting(meeting.id).to_json)
+          .at_path("_links/meeting/href")
+      end
+    end
+
     context "without view_meetings permission" do
       let(:permissions) { [] }
 
@@ -203,6 +261,42 @@ RSpec.describe "API v3 Meeting Agenda Items sub-resource", content_type: :json d
       end
     end
 
+    context "when the agenda item is part of the backlog" do
+      let(:backlog) { meeting.backlog }
+      let!(:backlog_agenda_item) do
+        create(:meeting_agenda_item,
+               meeting:,
+               meeting_section: backlog,
+               author: current_user,
+               title: "Backlog agenda item")
+      end
+      let(:path) { api_v3_paths.meeting_agenda_item(backlog_agenda_item.id, meeting_id: meeting.id) }
+
+      it "renders the backlog section correctly" do
+        expect(last_response).to have_http_status(:ok)
+
+        expect(last_response.body)
+          .to be_json_eql(api_v3_paths.meeting_section(backlog.id).to_json)
+          .at_path("_links/section/href")
+
+        expect(last_response.body)
+          .to be_json_eql(backlog.title.to_json)
+          .at_path("_links/section/title")
+
+        expect(last_response.body)
+          .to be_json_eql(backlog.id.to_json)
+          .at_path("_embedded/section/id")
+
+        expect(last_response.body)
+          .to be_json_eql(true.to_json)
+          .at_path("_embedded/section/backlog")
+
+        expect(last_response.body)
+          .to be_json_eql(api_v3_paths.meeting(meeting.id).to_json)
+          .at_path("_embedded/section/_links/meeting/href")
+      end
+    end
+
     context "when the agenda item is linked to a work package in an inaccessible project" do
       let(:private_project) { create(:project, public: false) }
       let(:private_work_package) { create(:work_package, project: private_project) }

modules/meeting/spec/requests/api/v3/meeting_sections/sections_by_meeting_resource_spec.rb

@@ -250,6 +250,138 @@ RSpec.describe "API v3 Meeting Sections sub-resource", content_type: :json do
     end
   end
 
+  describe "backlog section" do
+    let(:backlog) { meeting.backlog }
+
+    describe "GET /api/v3/meetings/:meeting_id/sections" do
+      let(:path) { api_v3_paths.meeting_sections(meeting_id: meeting.id) }
+
+      before { get path }
+
+      it "includes the backlog as the last section" do
+        elements = JSON.parse(last_response.body).dig("_embedded", "elements")
+
+        expect(elements.pluck("id")).to eq([section.id, backlog.id])
+        expect(elements.last).to include("backlog" => true)
+        expect(elements.first).to include("backlog" => false)
+      end
+    end
+
+    describe "GET /api/v3/meetings/:meeting_id/sections/:id" do
+      let(:path) { api_v3_paths.meeting_section(backlog.id, meeting_id: meeting.id) }
+
+      before { get path }
+
+      it "returns the backlog with canonical links" do
+        expect(last_response).to have_http_status(:ok)
+
+        expect(last_response.body)
+          .to be_json_eql(backlog.id.to_json)
+          .at_path("id")
+
+        expect(last_response.body)
+          .to be_json_eql(true.to_json)
+          .at_path("backlog")
+
+        expect(last_response.body)
+          .to be_json_eql(api_v3_paths.meeting_section(backlog.id).to_json)
+          .at_path("_links/self/href")
+
+        expect(last_response.body)
+          .to be_json_eql(api_v3_paths.meeting(meeting.id).to_json)
+          .at_path("_links/meeting/href")
+      end
+    end
+
+    describe "PATCH /api/v3/meeting_sections/:id" do
+      let(:path) { api_v3_paths.meeting_section(backlog.id) }
+      let(:original_title) { backlog.title }
+      let(:body) { { title: "Updated Backlog Title" }.to_json }
+
+      subject(:response) { patch path, body }
+
+      it "responds with 422 and does not change the title" do
+        expect(response).to have_http_status(:unprocessable_entity)
+        expect(backlog.reload.title).to eq(original_title)
+      end
+    end
+
+    describe "DELETE /api/v3/meeting_sections/:id" do
+      let(:path) { api_v3_paths.meeting_section(backlog.id) }
+
+      before { delete path }
+
+      it "responds with 422 and keeps the backlog" do
+        expect(last_response).to have_http_status(:unprocessable_entity)
+        expect(MeetingSection).to exist(backlog.id)
+      end
+    end
+  end
+
+  describe "backlog section for a recurring meeting occurrence" do
+    let(:recurring_meeting) { create(:recurring_meeting, project:, author: current_user) }
+    let(:template) { recurring_meeting.template }
+    let(:occurrence) do
+      create(:recurring_meeting_occurrence, recurring_meeting:, project:, author: current_user)
+    end
+    let!(:occurrence_section) { create(:meeting_section, meeting: occurrence, title: "Occurrence Section") }
+    let(:backlog) { template.backlog }
+
+    describe "GET /api/v3/meetings/:meeting_id/sections" do
+      let(:path) { api_v3_paths.meeting_sections(meeting_id: occurrence.id) }
+
+      before { get path }
+
+      it "includes the backlog with canonical links to the template meeting" do
+        elements = JSON.parse(last_response.body).dig("_embedded", "elements")
+        backlog_element = elements.find { |element| element["id"] == backlog.id }
+
+        expect(elements.pluck("id")).to eq([occurrence_section.id, backlog.id])
+        expect(backlog_element).to include("backlog" => true)
+        expect(backlog_element.dig("_links", "self", "href"))
+          .to eq(api_v3_paths.meeting_section(backlog.id))
+        expect(backlog_element.dig("_links", "meeting", "href"))
+          .to eq(api_v3_paths.meeting(template.id))
+      end
+    end
+
+    describe "GET /api/v3/meetings/:meeting_id/sections/:id" do
+      let(:path) { api_v3_paths.meeting_section(backlog.id, meeting_id: occurrence.id) }
+
+      before { get path }
+
+      it "returns the backlog with canonical links to the template meeting" do
+        expect(last_response).to have_http_status(:ok)
+
+        expect(last_response.body)
+          .to be_json_eql(api_v3_paths.meeting_section(backlog.id).to_json)
+          .at_path("_links/self/href")
+
+        expect(last_response.body)
+          .to be_json_eql(api_v3_paths.meeting(template.id).to_json)
+          .at_path("_links/meeting/href")
+      end
+    end
+
+    describe "agenda item section link consistency" do
+      let!(:agenda_item) do
+        create(:meeting_agenda_item,
+               meeting: occurrence,
+               meeting_section: backlog,
+               author: current_user,
+               title: "Backlog item")
+      end
+      let(:path) { api_v3_paths.meeting_section(backlog.id) }
+
+      before { get path }
+
+      it "resolves the section referenced by the agenda item" do
+        expect(last_response).to have_http_status(:ok)
+        expect(last_response.body).to be_json_eql(backlog.id.to_json).at_path("id")
+      end
+    end
+  end
+
   describe "DELETE /api/v3/meeting_sections/:id" do
     let(:path) { api_v3_paths.meeting_section(section.id) }
 

spec/lib/api/decorators/linked_resource_spec.rb

@@ -43,6 +43,112 @@ RSpec.describe API::Decorators::LinkedResource do
   let(:represented) { {} }
   let(:current_user) { create(:user) }
 
+  describe "embedded links" do
+    let(:thing_representer_class) do
+      Class.new(API::Decorators::Single) do
+        property :id
+        def _type = "Thing"
+      end
+    end
+
+    let(:representer_class) do
+      klass = thing_representer_class
+      Class.new(API::Decorators::Single) do
+        include API::Decorators::LinkedResource
+
+        associated_resource :included_thing, v3_path: :user, representer: klass
+        associated_resource :excluded_thing, v3_path: :user, representer: klass
+      end
+    end
+
+    let(:included_thing) { Struct.new(:id, :name).new(1, "Included thing") }
+    let(:excluded_thing) { Struct.new(:id, :name).new(2, "Excluded thing") }
+    let(:model) do
+      Struct
+        .new(:included_thing_id, :included_thing, :excluded_thing_id, :excluded_thing)
+        .new(included_thing.id, included_thing, excluded_thing.id, excluded_thing)
+    end
+
+    subject(:json) do
+      representer_class
+        .new(model, current_user:, embed_links: %i[included_thing])
+        .to_json
+    end
+
+    it "embeds only the selected links" do
+      expect(json).to have_json_path("_embedded/includedThing")
+      expect(json).not_to have_json_path("_embedded/excludedThing")
+    end
+
+    context "when an excluded embedded resource has no readable association" do
+      let(:representer_class) do
+        klass = thing_representer_class
+        Class.new(API::Decorators::Single) do
+          include API::Decorators::LinkedResource
+
+          associated_resource :included_thing, v3_path: :user, representer: klass
+          associated_resource :excluded_thing,
+                              v3_path: :user,
+                              representer: klass,
+                              link: ->(*) {
+                                {
+                                  href: "/api/v3/users/#{represented.excluded_thing_id}",
+                                  title: "Excluded thing"
+                                }
+                              }
+        end
+      end
+
+      let(:model) do
+        Class.new do
+          attr_reader :included_thing_id, :included_thing, :excluded_thing_id
+
+          def initialize(included_thing, excluded_thing)
+            @included_thing_id = included_thing.id
+            @included_thing = included_thing
+            @excluded_thing_id = excluded_thing.id
+          end
+
+          def excluded_thing
+            raise "excluded association should not be read"
+          end
+        end.new(included_thing, excluded_thing)
+      end
+
+      it "does not call the excluded association getter" do
+        expect { json }.not_to raise_error
+      end
+    end
+
+    context "with plural associated resources" do
+      let(:representer_class) do
+        klass = thing_representer_class
+        Class.new(API::Decorators::Single) do
+          include API::Decorators::LinkedResource
+
+          associated_resources :included_things, v3_path: :user, representer: klass
+          associated_resources :excluded_things, v3_path: :user, representer: klass
+        end
+      end
+      let(:model) do
+        Struct
+          .new(:included_things, :excluded_things)
+          .new([included_thing], [excluded_thing])
+      end
+
+      subject(:json) do
+        representer_class
+          .new(model, current_user:, embed_links: %i[included_things])
+          .to_json
+      end
+
+      it "embeds only the selected collection links" do
+        expect(json).to have_json_path("_embedded/includedThings")
+        expect(json).not_to have_json_path("_embedded/excludedThings")
+      end
+    end
+  end
+
   describe ".associated_visible_resource" do
     include API::V3::Utilities::PathHelper