diff --git a/app/controllers/sidebar_sections_controller.rb b/app/controllers/sidebar_sections_controller.rb index 6990d20ca82..9f72bf06fd4 100644 --- a/app/controllers/sidebar_sections_controller.rb +++ b/app/controllers/sidebar_sections_controller.rb @@ -35,6 +35,18 @@ class SidebarSectionsController < ApplicationController ActiveRecord::Base.transaction do sidebar_section.update!(section_params.merge(sidebar_urls_attributes: links_params)) sidebar_section.sidebar_section_links.update_all(user_id: sidebar_section.user_id) + order = + sidebar_section + .sidebar_urls + .sort_by do |url| + links_params.index { |link| link["name"] == url.name && link["value"] == url.value } || + -1 + end + .each_with_index + .map { |url, index| [url.id, index] } + .to_h + + set_order(sidebar_section, order) end if sidebar_section.public? @@ -43,7 +55,7 @@ class SidebarSectionsController < ApplicationController Site.clear_anon_cache! end - render json: SidebarSectionSerializer.new(sidebar_section) + render_serialized(sidebar_section.reload, SidebarSectionSerializer) rescue ActiveRecord::RecordInvalid => e render_json_error(e.record.errors.full_messages.first) rescue Discourse::InvalidAccess @@ -67,16 +79,8 @@ class SidebarSectionsController < ApplicationController @guardian.ensure_can_edit!(sidebar_section) order = reorder_params["links_order"].map(&:to_i).each_with_index.to_h - position_generator = - (0..sidebar_section.sidebar_section_links.count * 2).excluding( - sidebar_section.sidebar_section_links.map(&:position), - ).each - links = - sidebar_section - .sidebar_section_links - .sort_by { |link| order[link.linkable_id] } - .map { |link| link.attributes.merge(position: position_generator.next) } - sidebar_section.sidebar_section_links.upsert_all(links, update_only: [:position]) + + set_order(sidebar_section, order) render json: sidebar_section rescue Discourse::InvalidAccess render json: failed_json, status: 403 @@ -103,7 +107,7 @@ class SidebarSectionsController < ApplicationController end def links_params - params.permit(links: %i[icon name value id _destroy])["links"] + params.permit(links: %i[icon name value id _destroy segment])["links"] end def reorder_params @@ -112,6 +116,20 @@ class SidebarSectionsController < ApplicationController private + def set_order(sidebar_section, order) + position_generator = + (0..sidebar_section.sidebar_section_links.count * 2).excluding( + sidebar_section.sidebar_section_links.map(&:position), + ).each + + links = + sidebar_section + .sidebar_section_links + .sort_by { |link| order[link.linkable_id] } + .map { |link| link.attributes.merge(position: position_generator.next) } + sidebar_section.sidebar_section_links.upsert_all(links, update_only: [:position]) + end + def check_access_if_public return true if !params[:public] raise Discourse::InvalidAccess.new if !guardian.can_create_public_sidebar_section? diff --git a/spec/requests/sidebar_sections_controller_spec.rb b/spec/requests/sidebar_sections_controller_spec.rb index 87f8d3ad6b4..f08027a1b4f 100644 --- a/spec/requests/sidebar_sections_controller_spec.rb +++ b/spec/requests/sidebar_sections_controller_spec.rb @@ -179,9 +179,20 @@ RSpec.describe SidebarSectionsController do params: { title: "custom section edited", links: [ + { + icon: "link", + name: "meta", + value: "https://meta.discourse.org", + segment: "primary", + }, { icon: "link", id: sidebar_url_1.id, name: "latest", value: "/latest" }, { icon: "link", id: sidebar_url_2.id, name: "tags", value: "/tags", _destroy: "1" }, - { icon: "link", name: "homepage", value: "https://discourse.org" }, + { + icon: "link", + name: "homepage", + value: "https://discourse.org", + segment: "secondary", + }, ], } @@ -193,17 +204,21 @@ RSpec.describe SidebarSectionsController do expect { section_link_2.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { sidebar_url_2.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect(sidebar_section.sidebar_section_links.last.position).to eq(2) - expect(sidebar_section.sidebar_section_links.last.linkable.name).to eq("homepage") - expect(sidebar_section.sidebar_section_links.last.linkable.value).to eq( - "https://discourse.org", - ) + urls = sidebar_section.sidebar_urls + expect(urls[0].name).to eq("meta") + expect(urls[0].value).to eq("https://meta.discourse.org") + expect(urls[0].segment).to eq("primary") + expect(urls[1].name).to eq("latest") + expect(urls[1].value).to eq("/latest") + expect(urls[2].name).to eq("homepage") + expect(urls[2].value).to eq("https://discourse.org") + expect(urls[2].segment).to eq("secondary") user_history = UserHistory.last expect(user_history.action).to eq(UserHistory.actions[:update_public_sidebar_section]) expect(user_history.subject).to eq("custom section edited") expect(user_history.details).to eq( - "links: latest - /latest, homepage - https://discourse.org", + "links: latest - /latest, meta - https://meta.discourse.org, homepage - https://discourse.org", ) end