DEV: edited links are set in specific order (#21665)

Update section endpoint already exists. However, it has to also respect order of links.

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(
+    set_order(sidebar_section, order)
-        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])
     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?

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)
+      urls = sidebar_section.sidebar_urls
-      expect(sidebar_section.sidebar_section_links.last.linkable.name).to eq("homepage")
+      expect(urls[0].name).to eq("meta")
-      expect(sidebar_section.sidebar_section_links.last.linkable.value).to eq(
+      expect(urls[0].value).to eq("https://meta.discourse.org")
-        "https://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