Merge pull request #2770 from tvdeyen/remove-order-pages-route

Remove order admin pages route

app/controllers/alchemy/admin/pages_controller.rb

@@ -210,23 +210,6 @@ def copy_language_tree
         redirect_to admin_pages_path
       end
 
-      # Receives a JSON object representing a language tree to be ordered
-      # and updates all pages in that language structure to their correct indexes
-      def order
-        neworder = JSON.parse(params[:set])
-        tree = create_tree(neworder, @page_root)
-
-        Alchemy::Page.transaction do
-          tree.each do |key, node|
-            dbitem = Page.find(key)
-            dbitem.update_node!(node)
-          end
-        end
-
-        flash[:notice] = Alchemy.t("Pages order saved")
-        do_redirect_to admin_pages_path
-      end
-
       def flush
         @current_language.pages.flushables.update_all(published_at: Time.current)
         # We need to ensure, that also all layoutpages get the +published_at+ timestamp set,
@@ -259,60 +242,6 @@ def language_root_to_copy_from
         Page.language_root_for(params[:languages][:old_lang_id])
       end
 
-      # Returns the current left index and the aggregated hash of tree nodes indexed by page id visited so far
-      #
-      # Visits a batch of children nodes, assigns them the correct ordering indexes and spuns recursively the same
-      # procedure on their children, if any
-      #
-      # @param [Array]
-      #   An array of children nodes to be visited
-      # @param [Integer]
-      #   The lft attribute that should be given to the first node in the array
-      # @param [Integer]
-      #   The page id of the parent of this batch of children nodes
-      # @param [Integer]
-      #   The depth at which these children reside
-      # @param [Hash]
-      #   A Hash of TreeNode's indexed by their page ids
-      # @param [String]
-      #   The url for the parent node of these children
-      # @param [Boolean]
-      #   Whether these children reside in a restricted branch according to their ancestors
-      #
-      def visit_nodes(nodes, my_left, parent, depth, tree, url, restricted)
-        nodes.each do |item|
-          my_right = my_left + 1
-          my_restricted = item["restricted"] || restricted
-          urls = process_url(url, item)
-
-          if item["children"]
-            my_right, tree = visit_nodes(item["children"], my_left + 1, item["id"], depth + 1, tree, urls[:children_path], my_restricted)
-          end
-
-          tree[item["id"]] = TreeNode.new(my_left, my_right, parent, depth, urls[:my_urlname], my_restricted)
-          my_left = my_right + 1
-        end
-
-        [my_left, tree]
-      end
-
-      # Returns a Hash of TreeNode's indexed by their page ids
-      #
-      # Grabs the array representing a tree structure of pages passed as a parameter,
-      # visits it and creates a map of TreeNodes indexed by page id featuring Nested Set
-      # ordering information consisting of the left, right, depth and parent_id indexes as
-      # well as a node's url and restricted status
-      #
-      # @param [Array]
-      #   An Array representing a tree of Alchemy::Page's
-      # @param [Alchemy::Page]
-      #   The root page for the language being ordered
-      #
-      def create_tree(items, rootpage)
-        _, tree = visit_nodes(items, rootpage.lft + 1, rootpage.id, rootpage.depth + 1, {}, "", rootpage.restricted)
-        tree
-      end
-
       # Returns a pair, the path that a given tree node should take, and the path its children should take
       #
       # This function will add a node's own slug into their ancestor's path

app/models/alchemy/page.rb

@@ -444,25 +444,6 @@ def public_on=(time)
 
     delegate :public_until=, to: :public_version, allow_nil: true
 
-    # Updates an Alchemy::Page based on a new ordering to be applied to it
-    #
-    # Note: Page's urls should not be updated (and a legacy URL created) if nesting is OFF
-    # or if the URL is the same
-    #
-    # @param [TreeNode]
-    #   A tree node with new lft, rgt, depth, url, parent_id and restricted indexes to be updated
-    #
-    def update_node!(node)
-      hash = {lft: node.left, rgt: node.right, parent_id: node.parent, depth: node.depth, restricted: node.restricted}
-
-      if urlname != node.url
-        LegacyPageUrl.create(page_id: id, urlname: urlname)
-        hash[:urlname] = node.url
-      end
-
-      update_columns(hash)
-    end
-
     # Holds an instance of +FixedAttributes+
     def fixed_attributes
       @_fixed_attributes ||= FixedAttributes.new(self)

config/locales/alchemy.en.yml

@@ -302,7 +302,6 @@ en:
     "Page deleted": "%{name} deleted"
     "Page saved": "%{name} saved"
     "Page cache flushed": "Page cache flushed"
-    "Pages order saved": "Pages order saved"
     "Password": "Password"
     "Password reset": "Password reset"
     "Paste from clipboard": "Paste from clipboard"

config/routes.rb

@@ -22,7 +22,6 @@
     resources :pages do
       resources :elements
       collection do
-        post :order
         post :flush
         post :copy_language_tree
         get :create_language

spec/models/alchemy/page_spec.rb

@@ -1585,41 +1585,6 @@ module Alchemy
       end
     end
 
-    describe "#update_node!" do
-      let(:original_url) { "sample-url" }
-      let(:page) { create(:alchemy_page, language: language, parent: language_root, urlname: original_url, restricted: false) }
-      let(:node) { TreeNode.new(10, 11, 12, 13, "another-url", true) }
-
-      it "should update all attributes" do
-        page.update_node!(node)
-        page.reload
-        expect(page.lft).to eq(node.left)
-        expect(page.rgt).to eq(node.right)
-        expect(page.parent_id).to eq(node.parent)
-        expect(page.depth).to eq(node.depth)
-        expect(page.urlname).to eq(node.url)
-        expect(page.restricted).to eq(node.restricted)
-      end
-
-      context "when url is the same" do
-        let(:node) { TreeNode.new(10, 11, 12, 13, original_url, true) }
-
-        it "should not create a legacy url" do
-          page.update_node!(node)
-          page.reload
-          expect(page.legacy_urls.size).to eq(0)
-        end
-      end
-
-      context "when url is not the same" do
-        it "should create a legacy url" do
-          page.update_node!(node)
-          page.reload
-          expect(page.legacy_urls.size).to eq(1)
-        end
-      end
-    end
-
     describe "#cache_page?" do
       let(:page) { Page.new(page_layout: "news") }
       subject { page.cache_page? }

spec/requests/alchemy/admin/pages_controller_spec.rb

@@ -308,77 +308,6 @@ module Alchemy
         end
       end
 
-      describe "#order" do
-        let(:page_1) { create(:alchemy_page) }
-        let(:page_2) { create(:alchemy_page) }
-        let(:page_3) { create(:alchemy_page) }
-        let(:page_item_1) { {id: page_1.id, slug: page_1.slug, restricted: false, children: [page_item_2]} }
-        let(:page_item_2) { {id: page_2.id, slug: page_2.slug, restricted: false, children: [page_item_3]} }
-        let(:page_item_3) { {id: page_3.id, slug: page_3.slug, restricted: false} }
-        let(:set_of_pages) { [page_item_1] }
-
-        it "stores the new order" do
-          post order_admin_pages_path(set: set_of_pages.to_json), xhr: true
-          page_1.reload
-          expect(page_1.descendants).to eq([page_2, page_3])
-        end
-
-        it "updates the pages urlnames" do
-          post order_admin_pages_path(set: set_of_pages.to_json), xhr: true
-          [page_1, page_2, page_3].map(&:reload)
-          expect(page_1.urlname).to eq(page_1.slug.to_s)
-          expect(page_2.urlname).to eq("#{page_1.slug}/#{page_2.slug}")
-          expect(page_3.urlname).to eq("#{page_1.slug}/#{page_2.slug}/#{page_3.slug}")
-        end
-
-        context "with restricted page in tree" do
-          let(:page_2) { create(:alchemy_page, restricted: true) }
-          let(:page_item_2) do
-            {
-              id: page_2.id,
-              slug: page_2.slug,
-              children: [page_item_3],
-              restricted: true
-            }
-          end
-
-          it "updates restricted status of descendants" do
-            post order_admin_pages_path(set: set_of_pages.to_json), xhr: true
-            page_3.reload
-            expect(page_3.restricted).to be_truthy
-          end
-        end
-
-        context "with page having number as slug" do
-          let(:page_item_2) do
-            {
-              id: page_2.id,
-              slug: 42,
-              children: [page_item_3]
-            }
-          end
-
-          it "does not raise error" do
-            expect {
-              post order_admin_pages_path(set: set_of_pages.to_json), xhr: true
-            }.not_to raise_error
-          end
-
-          it "still generates the correct urlname on page_3" do
-            post order_admin_pages_path(set: set_of_pages.to_json), xhr: true
-            [page_1, page_2, page_3].map(&:reload)
-            expect(page_3.urlname).to eq("#{page_1.slug}/#{page_2.slug}/#{page_3.slug}")
-          end
-        end
-
-        it "creates legacy urls" do
-          post order_admin_pages_path(set: set_of_pages.to_json), xhr: true
-          [page_2, page_3].map(&:reload)
-          expect(page_2.legacy_urls.size).to eq(1)
-          expect(page_3.legacy_urls.size).to eq(1)
-        end
-      end
-
       describe "#configure" do
         context "with page having nested urlname" do
           let(:page) { create(:alchemy_page, name: "Foobar", urlname: "foobar") }