FEATURE: ability to reorder links in custom sidebar sections (#20626)

Drag and drop to reorder custom sidebar sections
2024-11-22 15:25:35 +08:00 · 2023-03-21 12:23:28 +11:00 · 2023-03-21 12:23:28 +11:00 · db74e9484b
commit db74e9484b
parent 53cadac4b8
17 changed files with 343 additions and 46 deletions
--- a/app/assets/javascripts/discourse/app/components/sidebar/section.hbs
+++ b/app/assets/javascripts/discourse/app/components/sidebar/section.hbs
@ -1,5 +1,12 @@
 {{#if this.displaySection}}
-  <div class={{concat "sidebar-section-wrapper sidebar-section-" @sectionName}}>
+  <div
+    class={{concat
+      "sidebar-section-wrapper sidebar-section-"
+      @sectionName
+      " "
+      @class
+    }}
+  >
    <div class="sidebar-section-header-wrapper sidebar-row">
      <Sidebar::SectionHeader
        @collapsable={{@collapsable}}
--- a/app/assets/javascripts/discourse/app/components/sidebar/user/custom-sections.hbs
+++ b/app/assets/javascripts/discourse/app/components/sidebar/user/custom-sections.hbs
@ -6,6 +6,7 @@
      @collapsable={{true}}
      @headerActions={{section.headerActions}}
      @headerActionsIcon="pencil-alt"
+      @class={{section.dragCss}}
    >
      {{#each section.links as |link|}}
        {{#if link.external}}
@ -15,6 +16,12 @@
            @prefixType="icon"
            @prefixValue={{link.icon}}
            @href={{link.value}}
+            @class={{link.linkDragCss}}
+            {{draggable
+              didStartDrag=link.didStartDrag
+              didEndDrag=link.didEndDrag
+              dragMove=link.dragMove
+            }}
          />
        {{else}}
          <Sidebar::SectionLink
@ -25,6 +32,12 @@
            @content={{replace-emoji link.name}}
            @prefixType="icon"
            @prefixValue={{link.icon}}
+            @class={{link.linkDragCss}}
+            {{draggable
+              didStartDrag=link.didStartDrag
+              didEndDrag=link.didEndDrag
+              dragMove=link.dragMove
+            }}
          />
        {{/if}}
      {{/each}}
--- a/app/assets/javascripts/discourse/app/components/sidebar/user/custom-sections.js
+++ b/app/assets/javascripts/discourse/app/components/sidebar/user/custom-sections.js
@ -1,12 +1,8 @@
 import Component from "@glimmer/component";
-import showModal from "discourse/lib/show-modal";
 import { inject as service } from "@ember/service";
-import RouteInfoHelper from "discourse/lib/sidebar/route-info-helper";
-import I18n from "I18n";
 import { ajax } from "discourse/lib/ajax";
-import { iconHTML } from "discourse-common/lib/icon-library";
-import { htmlSafe } from "@ember/template";
 import { bind } from "discourse-common/utils/decorators";
+import Section from "discourse/components/sidebar/user/section";

 export default class SidebarUserCustomSections extends Component {
  @service currentUser;
@ -23,31 +19,13 @@ export default class SidebarUserCustomSections extends Component {
  }

  get sections() {
-    this.currentUser.sidebarSections.forEach((section) => {
-      if (!section.public || this.currentUser.staff) {
-        section.headerActions = [
-          {
-            action: () => {
-              return showModal("sidebar-section-form", { model: section });
-            },
-            title: I18n.t("sidebar.sections.custom.edit"),
-          },
-        ];
-      }
-      section.decoratedTitle =
-        section.public && this.currentUser.staff
-          ? htmlSafe(`${iconHTML("globe")} ${section.title}`)
-          : section.title;
-      section.links.forEach((link) => {
-        if (!link.external) {
-          const routeInfoHelper = new RouteInfoHelper(this.router, link.value);
-          link.route = routeInfoHelper.route;
-          link.models = routeInfoHelper.models;
-          link.query = routeInfoHelper.query;
-        }
+    return this.currentUser.sidebarSections.map((section) => {
+      return new Section({
+        section,
+        currentUser: this.currentUser,
+        router: this.router,
      });
    });
-    return this.currentUser.sidebarSections;
  }

  @bind
--- a/app/assets/javascripts/discourse/app/components/sidebar/user/section-link.js
+++ b/app/assets/javascripts/discourse/app/components/sidebar/user/section-link.js
@ -0,0 +1,63 @@
+import { tracked } from "@glimmer/tracking";
+import { bind } from "discourse-common/utils/decorators";
+import RouteInfoHelper from "discourse/lib/sidebar/route-info-helper";
+
+export default class SectionLink {
+  @tracked linkDragCss;
+
+  constructor({ external, icon, id, name, value }, section, router) {
+    this.external = external;
+    this.icon = icon;
+    this.id = id;
+    this.name = name;
+    this.value = value;
+    this.section = section;
+
+    if (!this.external) {
+      const routeInfoHelper = new RouteInfoHelper(router, value);
+      this.route = routeInfoHelper.route;
+      this.models = routeInfoHelper.models;
+      this.query = routeInfoHelper.query;
+    }
+  }
+
+  @bind
+  didStartDrag(e) {
+    this.mouseY = e.targetTouches ? e.targetTouches[0].screenY : e.screenY;
+  }
+
+  @bind
+  didEndDrag() {
+    this.linkDragCss = null;
+    this.mouseY = null;
+    this.section.enable();
+    this.section.reorder();
+  }
+
+  @bind
+  dragMove(e) {
+    const currentMouseY = e.targetTouches
+      ? e.targetTouches[0].screenY
+      : e.screenY;
+    const distance = currentMouseY - this.mouseY;
+    if (!this.linkHeight) {
+      this.linkHeight = document.getElementsByClassName(
+        "sidebar-section-link-wrapper"
+      )[0].clientHeight;
+    }
+    if (distance >= this.linkHeight) {
+      if (this.section.links.indexOf(this) !== this.section.links.length - 1) {
+        this.section.moveLinkDown(this);
+        this.mouseY = currentMouseY;
+      }
+    }
+    if (distance <= -this.linkHeight) {
+      if (this.section.links.indexOf(this) !== 0) {
+        this.section.moveLinkUp(this);
+        this.mouseY = currentMouseY;
+      }
+    }
+    this.linkDragCss = "drag";
+    this.section.disable();
+  }
+}
--- a/app/assets/javascripts/discourse/app/components/sidebar/user/section.js
+++ b/app/assets/javascripts/discourse/app/components/sidebar/user/section.js
@ -0,0 +1,79 @@
+import I18n from "I18n";
+import showModal from "discourse/lib/show-modal";
+import { iconHTML } from "discourse-common/lib/icon-library";
+import { htmlSafe } from "@ember/template";
+import SectionLink from "discourse/components/sidebar/user/section-link";
+import { tracked } from "@glimmer/tracking";
+import { bind } from "discourse-common/utils/decorators";
+import { ajax } from "discourse/lib/ajax";
+
+export default class Section {
+  @tracked dragCss;
+  @tracked links;
+
+  constructor({ section, currentUser, router }) {
+    this.section = section;
+    this.router = router;
+    this.currentUser = currentUser;
+    this.slug = section.slug;
+
+    this.links = this.section.links.map((link) => {
+      return new SectionLink(link, this, this.router);
+    });
+  }
+
+  get decoratedTitle() {
+    return this.section.public && this.currentUser.staff
+      ? htmlSafe(`${iconHTML("globe")} ${this.section.title}`)
+      : this.section.title;
+  }
+
+  get headerActions() {
+    if (!this.section.public || this.currentUser.staff) {
+      return [
+        {
+          action: () => {
+            return showModal("sidebar-section-form", { model: this.section });
+          },
+          title: I18n.t("sidebar.sections.custom.edit"),
+        },
+      ];
+    }
+  }
+
+  @bind
+  disable() {
+    this.dragCss = "disabled";
+  }
+
+  @bind
+  enable() {
+    this.dragCss = null;
+  }
+
+  @bind
+  moveLinkDown(link) {
+    const position = this.links.indexOf(link) + 1;
+    this.links = this.links.removeObject(link);
+    this.links.splice(position, 0, link);
+  }
+
+  @bind
+  moveLinkUp(link) {
+    const position = this.links.indexOf(link) - 1;
+    this.links = this.links.removeObject(link);
+    this.links.splice(position, 0, link);
+  }
+  @bind
+  reorder() {
+    return ajax(`/sidebar_sections/reorder`, {
+      type: "POST",
+      contentType: "application/json",
+      dataType: "json",
+      data: JSON.stringify({
+        sidebar_section_id: this.section.id,
+        links_order: this.links.map((link) => link.id),
+      }),
+    });
+  }
+}
--- a/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js
+++ b/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js
@ -172,7 +172,10 @@ export default Controller.extend(ModalFunctionality, {
      }),
    })
      .then((data) => {
-        this.currentUser.sidebar_sections.pushObject(data.sidebar_section);
+        this.currentUser.set(
+          "sidebar_sections",
+          this.currentUser.sidebar_sections.concat(data.sidebar_section)
+        );
        this.send("closeModal");
      })
      .catch((e) =>
--- a/app/assets/javascripts/discourse/app/modifiers/draggable.js
+++ b/app/assets/javascripts/discourse/app/modifiers/draggable.js
@ -32,7 +32,7 @@ export default class DraggableModifier extends Modifier {
      this.hasStarted = true;

      if (this.didStartDragCallback) {
-        this.didStartDragCallback();
+        this.didStartDragCallback(e);
      }

      // Register a global event to capture mouse moves when element 'clicked'.
--- a/app/assets/stylesheets/common/base/_index.scss
+++ b/app/assets/stylesheets/common/base/_index.scss
@ -47,6 +47,7 @@
@import "share_link";
@import "shared-drafts";
@import "sidebar";
+@import "sidebar-custom-section";
@import "sidebar-footer";
@import "sidebar-section";
@import "sidebar-more-section-links";
--- a/app/assets/stylesheets/common/base/sidebar-custom-section.scss
+++ b/app/assets/stylesheets/common/base/sidebar-custom-section.scss
@ -0,0 +1,34 @@
+.sidebar-custom-sections {
+  .sidebar-section-wrapper {
+    padding-bottom: 0;
+  }
+  .d-icon-globe {
+    position: absolute;
+    left: 0.5em;
+    height: 0.75em;
+    width: 0.75em;
+    margin-top: 0.15em;
+    align-items: center;
+  }
+
+  .sidebar-section-link-prefix.icon {
+    cursor: move;
+  }
+
+  .sidebar-section-wrapper.disabled {
+    .sidebar-section-link-wrapper {
+      .sidebar-section-link-prefix.icon,
+      .sidebar-section-link {
+        background: none;
+        color: var(--primary-low);
+      }
+      .sidebar-section-link.drag {
+        font-weight: bold;
+        color: var(--primary-high);
+        .sidebar-section-link-prefix.icon {
+          color: var(--primary-high);
+        }
+      }
+    }
+  }
+}
--- a/app/assets/stylesheets/common/base/sidebar.scss
+++ b/app/assets/stylesheets/common/base/sidebar.scss
@ -128,19 +128,6 @@
  }
 }

-.sidebar-custom-sections {
-  .sidebar-section-wrapper {
-    padding-bottom: 0;
-  }
-  .d-icon-globe {
-    position: absolute;
-    left: 0.5em;
-    height: 0.75em;
-    width: 0.75em;
-    margin-top: 0.15em;
-    align-items: center;
-  }
-}
 .sidebar-section-form-modal {
  .modal-inner-container {
    width: var(--modal-max-width);
--- a/app/controllers/sidebar_sections_controller.rb
+++ b/app/controllers/sidebar_sections_controller.rb
@ -56,6 +56,26 @@ class SidebarSectionsController < ApplicationController
    render json: failed_json, status: 403
  end

+  def reorder
+    sidebar_section = SidebarSection.find_by(id: reorder_params["sidebar_section_id"])
+    @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])
+    render json: sidebar_section
+  rescue Discourse::InvalidAccess
+    render json: failed_json, status: 403
+  end
+
  def destroy
    sidebar_section = SidebarSection.find_by(id: section_params["id"])
    @guardian.ensure_can_delete!(sidebar_section)
@ -82,6 +102,10 @@ class SidebarSectionsController < ApplicationController
    params.permit(links: %i[icon name value id _destroy])["links"]
  end

+  def reorder_params
+    params.permit(:sidebar_section_id, links_order: [])
+  end
+
  def check_if_member_of_group
    ### TODO remove when enable_custom_sidebar_sections SiteSetting is removed
    if !SiteSetting.enable_custom_sidebar_sections.present? ||
--- a/app/models/sidebar_section.rb
+++ b/app/models/sidebar_section.rb
@ -2,7 +2,7 @@

 class SidebarSection < ActiveRecord::Base
  belongs_to :user
-  has_many :sidebar_section_links, dependent: :destroy
+  has_many :sidebar_section_links, -> { order("position") }, dependent: :destroy
  has_many :sidebar_urls,
           through: :sidebar_section_links,
           source: :linkable,
--- a/app/models/sidebar_section_link.rb
+++ b/app/models/sidebar_section_link.rb
@ -13,6 +13,12 @@ class SidebarSectionLink < ActiveRecord::Base
  SUPPORTED_LINKABLE_TYPES = %w[Category Tag SidebarUrl]

  before_validation { self.user_id ||= self.sidebar_section&.user_id }
+  before_create do
+    if self.user_id && self.sidebar_section
+      self.position = self.sidebar_section.sidebar_section_links.maximum(:position).to_i + 1
+    end
+  end
+
  after_destroy { self.linkable.destroy! if self.linkable_type == "SidebarUrl" }

  private def ensure_supported_linkable_type
@ -37,9 +43,11 @@ end
 #  created_at         :datetime         not null
 #  updated_at         :datetime         not null
 #  sidebar_section_id :integer
+#  position           :integer          default(0), not null
 #
 # Indexes
 #
 #  idx_unique_sidebar_section_links                              (user_id,linkable_type,linkable_id) UNIQUE
 #  index_sidebar_section_links_on_linkable_type_and_linkable_id  (linkable_type,linkable_id)
+#  links_user_id_section_id_position                             (user_id,sidebar_section_id,position) UNIQUE
 #
--- a/config/routes.rb
+++ b/config/routes.rb
@ -1594,6 +1594,7 @@ Discourse::Application.routes.draw do
    delete "user-status" => "user_status#clear"

    resources :sidebar_sections, only: %i[index create update destroy]
+    post "/sidebar_sections/reorder" => "sidebar_sections#reorder"

    get "*url", to: "permalinks#show", constraints: PermalinkConstraint.new
  end
--- a/db/migrate/20230307051200_add_position_to_sidebar_section_links.rb
+++ b/db/migrate/20230307051200_add_position_to_sidebar_section_links.rb
@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class AddPositionToSidebarSectionLinks < ActiveRecord::Migration[7.0]
+  def change
+    add_column :sidebar_section_links, :position, :integer, default: 0, null: false
+    execute "UPDATE sidebar_section_links SET position = id"
+    add_index :sidebar_section_links,
+              %i[user_id sidebar_section_id position],
+              unique: true,
+              name: "links_user_id_section_id_position"
+  end
+end
--- a/spec/requests/sidebar_sections_controller_spec.rb
+++ b/spec/requests/sidebar_sections_controller_spec.rb
@ -167,6 +167,7 @@ RSpec.describe SidebarSectionsController do
            links: [
              { 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" },
            ],
          }

@ -178,10 +179,18 @@ 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",
+      )
+
      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")
+      expect(user_history.details).to eq(
+        "links: latest - /latest, homepage - https://discourse.org",
+      )
    end

    it "doesn't allow to edit other's sections" do
@ -232,6 +241,57 @@ RSpec.describe SidebarSectionsController do
    end
  end

+  describe "#reorder" do
+    fab!(:sidebar_section) { Fabricate(:sidebar_section, user: user) }
+    fab!(:sidebar_url_1) { Fabricate(:sidebar_url, name: "tags", value: "/tags") }
+    fab!(:sidebar_url_2) { Fabricate(:sidebar_url, name: "categories", value: "/categories") }
+    fab!(:sidebar_url_3) { Fabricate(:sidebar_url, name: "topic", value: "/t/1") }
+    fab!(:section_link_1) do
+      Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_1)
+    end
+    fab!(:section_link_2) do
+      Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_2)
+    end
+    fab!(:section_link_3) do
+      Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_3)
+    end
+
+    it "sorts links" do
+      serializer = SidebarSectionSerializer.new(sidebar_section, root: false).as_json
+      expect(serializer[:links].map(&:id)).to eq(
+        [sidebar_url_1.id, sidebar_url_2.id, sidebar_url_3.id],
+      )
+
+      sign_in(user)
+      post "/sidebar_sections/reorder.json",
+           params: {
+             sidebar_section_id: sidebar_section.id,
+             links_order: [sidebar_url_2.id, sidebar_url_3.id, sidebar_url_1.id],
+           }
+
+      serializer = SidebarSectionSerializer.new(sidebar_section.reload, root: false).as_json
+      expect(serializer[:links].map(&:id)).to eq(
+        [sidebar_url_2.id, sidebar_url_3.id, sidebar_url_1.id],
+      )
+    end
+
+    it "is not allowed for not own sections" do
+      sidebar_section_2 = Fabricate(:sidebar_section)
+      post "/sidebar_sections/reorder.json",
+           params: {
+             sidebar_section_id: sidebar_section_2.id,
+             links_order: [sidebar_url_2.id, sidebar_url_3.id, sidebar_url_1.id],
+           }
+
+      expect(response.status).to eq(403)
+
+      serializer = SidebarSectionSerializer.new(sidebar_section, root: false).as_json
+      expect(serializer[:links].map(&:id)).to eq(
+        [sidebar_url_1.id, sidebar_url_2.id, sidebar_url_3.id],
+      )
+    end
+  end
+
  describe "#destroy" do
    fab!(:sidebar_section) { Fabricate(:sidebar_section, user: user) }

--- a/spec/system/custom_sidebar_sections_spec.rb
+++ b/spec/system/custom_sidebar_sections_spec.rb
@ -80,6 +80,33 @@ describe "Custom sidebar sections", type: :system, js: true do
    expect(page).not_to have_link("Sidebar Categories")
  end

+  it "allows the user to reorder links in custom section" do
+    sidebar_section = Fabricate(:sidebar_section, title: "My section", user: user)
+    sidebar_url_1 = Fabricate(:sidebar_url, name: "Sidebar Tags", value: "/tags")
+    Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_1)
+    sidebar_url_2 = Fabricate(:sidebar_url, name: "Sidebar Categories", value: "/categories")
+    Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_2)
+
+    visit("/latest")
+    within(".sidebar-custom-sections .sidebar-section-link-wrapper:nth-child(1)") do
+      expect(page).to have_css(".sidebar-section-link-sidebar-tags")
+    end
+    within(".sidebar-custom-sections .sidebar-section-link-wrapper:nth-child(2)") do
+      expect(page).to have_css(".sidebar-section-link-sidebar-categories")
+    end
+
+    tags_link = find(".sidebar-section-link-sidebar-tags")
+    categories_link = find(".sidebar-section-link-sidebar-categories")
+    tags_link.drag_to(categories_link)
+
+    within(".sidebar-custom-sections .sidebar-section-link-wrapper:nth-child(1)") do
+      expect(page).to have_css(".sidebar-section-link-sidebar-categories")
+    end
+    within(".sidebar-custom-sections .sidebar-section-link-wrapper:nth-child(2)") do
+      expect(page).to have_css(".sidebar-section-link-sidebar-tags")
+    end
+  end
+
  it "does not allow the user to edit public section" do
    sidebar_section = Fabricate(:sidebar_section, title: "Public section", user: user, public: true)
    sidebar_url_1 = Fabricate(:sidebar_url, name: "Sidebar Tags", value: "/tags")