From 80ac2757f01eb6cf250ac378bd4448f306683176 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 28 Apr 2020 21:54:43 +0200 Subject: [PATCH] Remove acts_as_list from Content Closes #1793 --- app/models/alchemy/content.rb | 48 ++++++------- app/models/alchemy/element.rb | 70 +++++++++---------- app/serializers/alchemy/content_serializer.rb | 1 - .../20200226213334_alchemy_four_point_four.rb | 3 +- spec/dummy/db/schema.rb | 3 +- spec/models/alchemy/element_spec.rb | 17 ----- 6 files changed, 60 insertions(+), 82 deletions(-) diff --git a/app/models/alchemy/content.rb b/app/models/alchemy/content.rb index 25621726c7..fcb6e332e5 100644 --- a/app/models/alchemy/content.rb +++ b/app/models/alchemy/content.rb @@ -30,27 +30,25 @@ class Content < BaseRecord stampable stamper_class_name: Alchemy.user_class_name - acts_as_list scope: [:element_id] - # Essence scopes - scope :essence_booleans, -> { where(essence_type: "Alchemy::EssenceBoolean") } - scope :essence_dates, -> { where(essence_type: "Alchemy::EssenceDate") } - scope :essence_files, -> { where(essence_type: "Alchemy::EssenceFile") } - scope :essence_htmls, -> { where(essence_type: "Alchemy::EssenceHtml") } - scope :essence_links, -> { where(essence_type: "Alchemy::EssenceLink") } - scope :essence_pictures, -> { where(essence_type: "Alchemy::EssencePicture") } + scope :essence_booleans, -> { where(essence_type: "Alchemy::EssenceBoolean") } + scope :essence_dates, -> { where(essence_type: "Alchemy::EssenceDate") } + scope :essence_files, -> { where(essence_type: "Alchemy::EssenceFile") } + scope :essence_htmls, -> { where(essence_type: "Alchemy::EssenceHtml") } + scope :essence_links, -> { where(essence_type: "Alchemy::EssenceLink") } + scope :essence_pictures, -> { where(essence_type: "Alchemy::EssencePicture") } scope :essence_richtexts, -> { where(essence_type: "Alchemy::EssenceRichtext") } - scope :essence_selects, -> { where(essence_type: "Alchemy::EssenceSelect") } - scope :essence_texts, -> { where(essence_type: "Alchemy::EssenceText") } - scope :named, ->(name) { where(name: name) } - scope :available, -> { published.not_trashed } - scope :published, -> { joins(:element).merge(Element.published) } - scope :not_trashed, -> { joins(:element).merge(Element.not_trashed) } - scope :not_restricted, -> { joins(:element).merge(Element.not_restricted) } - - delegate :restricted?, to: :page, allow_nil: true - delegate :trashed?, to: :element, allow_nil: true - delegate :public?, to: :element, allow_nil: true + scope :essence_selects, -> { where(essence_type: "Alchemy::EssenceSelect") } + scope :essence_texts, -> { where(essence_type: "Alchemy::EssenceText") } + scope :named, ->(name) { where(name: name) } + scope :available, -> { published.not_trashed } + scope :published, -> { joins(:element).merge(Element.published) } + scope :not_trashed, -> { joins(:element).merge(Element.not_trashed) } + scope :not_restricted, -> { joins(:element).merge(Element.not_restricted) } + + delegate :restricted?, to: :page, allow_nil: true + delegate :trashed?, to: :element, allow_nil: true + delegate :public?, to: :element, allow_nil: true class << self # Returns the translated label for a content name. @@ -72,7 +70,7 @@ def translated_label_for(content_name, element_name = nil) Alchemy.t( content_name, scope: "content_names.#{element_name}", - default: Alchemy.t("content_names.#{content_name}", default: content_name.humanize) + default: Alchemy.t("content_names.#{content_name}", default: content_name.humanize), ) end end @@ -132,7 +130,7 @@ def serialize { name: name, value: serialized_ingredient, - link: essence.try(:link) + link: essence.try(:link), }.delete_if { |_k, v| v.blank? } end @@ -174,12 +172,12 @@ def essence_validation_failed? end def has_validations? - definition['validate'].present? + definition["validate"].present? end # Returns a string used as dom id on html elements. def dom_id - return '' if essence.nil? + return "" if essence.nil? "#{essence_partial_name}_#{id}" end @@ -195,7 +193,7 @@ def linked? # Returns true if this content should be taken for element preview. def preview_content? - !!definition['as_element_title'] + !!definition["as_element_title"] end # Proxy method that returns the preview text from essence. @@ -205,7 +203,7 @@ def preview_text(maxlength = 30) end def essence_partial_name - return '' if essence.nil? + return "" if essence.nil? essence.partial_name end diff --git a/app/models/alchemy/element.rb b/app/models/alchemy/element.rb index 2385a3269c..ebcfb0fffb 100644 --- a/app/models/alchemy/element.rb +++ b/app/models/alchemy/element.rb @@ -34,7 +34,7 @@ class Element < BaseRecord "hint", "taggable", "compact", - "message" + "message", ].freeze SKIPPED_ATTRIBUTES_ON_COPY = [ @@ -45,7 +45,7 @@ class Element < BaseRecord "folded", "position", "updated_at", - "updater_id" + "updater_id", ].freeze # All Elements that share the same page id and parent element id and are fixed or not are considered a list. @@ -64,13 +64,13 @@ class Element < BaseRecord has_many :all_nested_elements, -> { order(:position).not_trashed }, - class_name: 'Alchemy::Element', + class_name: "Alchemy::Element", foreign_key: :parent_element_id, dependent: :destroy has_many :nested_elements, -> { order(:position).available }, - class_name: 'Alchemy::Element', + class_name: "Alchemy::Element", foreign_key: :parent_element_id, dependent: :destroy, inverse_of: :parent_element @@ -79,13 +79,13 @@ class Element < BaseRecord # A nested element belongs to a parent element. belongs_to :parent_element, - class_name: 'Alchemy::Element', + class_name: "Alchemy::Element", optional: true, touch: true, inverse_of: :nested_elements has_and_belongs_to_many :touchable_pages, -> { distinct }, - class_name: 'Alchemy::Page', + class_name: "Alchemy::Page", join_table: ElementToPage.table_name validates_presence_of :name, on: :create @@ -98,19 +98,19 @@ class Element < BaseRecord after_update :touch_touchable_pages - scope :trashed, -> { where(position: nil).order('updated_at DESC') } - scope :not_trashed, -> { where.not(position: nil) } - scope :published, -> { where(public: true) } - scope :not_restricted, -> { joins(:page).merge(Page.not_restricted) } - scope :available, -> { published.not_trashed } - scope :named, ->(names) { where(name: names) } - scope :excluded, ->(names) { where.not(name: names) } - scope :fixed, -> { where(fixed: true) } - scope :unfixed, -> { where(fixed: false) } - scope :from_current_site, -> { where(Language.table_name => {site_id: Site.current || Site.default}).joins(page: 'language') } - scope :folded, -> { where(folded: true) } - scope :expanded, -> { where(folded: false) } - scope :not_nested, -> { where(parent_element_id: nil) } + scope :trashed, -> { where(position: nil).order("updated_at DESC") } + scope :not_trashed, -> { where.not(position: nil) } + scope :published, -> { where(public: true) } + scope :not_restricted, -> { joins(:page).merge(Page.not_restricted) } + scope :available, -> { published.not_trashed } + scope :named, ->(names) { where(name: names) } + scope :excluded, ->(names) { where.not(name: names) } + scope :fixed, -> { where(fixed: true) } + scope :unfixed, -> { where(fixed: false) } + scope :from_current_site, -> { where(Language.table_name => { site_id: Site.current || Site.default }).joins(page: "language") } + scope :folded, -> { where(folded: true) } + scope :expanded, -> { where(folded: false) } + scope :not_nested, -> { where(parent_element_id: nil) } delegate :restricted?, to: :page, allow_nil: true @@ -132,7 +132,7 @@ class << self def new(attributes = {}) return super if attributes[:name].blank? - element_attributes = attributes.to_h.merge(name: attributes[:name].split('#').first) + element_attributes = attributes.to_h.merge(name: attributes[:name].split("#").first) element_definition = Element.definition_by_name(element_attributes[:name]) if element_definition.nil? raise(ElementDefinitionError, attributes) @@ -154,13 +154,13 @@ def new(attributes = {}) # def copy(source_element, differences = {}) attributes = source_element.attributes.with_indifferent_access - .except(*SKIPPED_ATTRIBUTES_ON_COPY) - .merge(differences) - .merge({ - autogenerate_contents: false, - autogenerate_nested_elements: false, - tag_list: source_element.tag_list - }) + .except(*SKIPPED_ATTRIBUTES_ON_COPY) + .merge(differences) + .merge({ + autogenerate_contents: false, + autogenerate_nested_elements: false, + tag_list: source_element.tag_list, + }) new_element = create!(attributes) @@ -178,7 +178,7 @@ def copy(source_element, differences = {}) def all_from_clipboard(clipboard) return [] if clipboard.nil? - where(id: clipboard.collect { |e| e['id'] }) + where(id: clipboard.collect { |e| e["id"] }) end # All elements in clipboard that could be placed on page @@ -197,7 +197,7 @@ def all_from_clipboard_for_page(clipboard, page) # Pass an element name to get next of this kind. # def next(name = nil) - elements = page.elements.published.where('position > ?', position) + elements = page.elements.published.where("position > ?", position) select_element(elements, name, :asc) end @@ -206,7 +206,7 @@ def next(name = nil) # Pass an element name to get previous of this kind. # def prev(name = nil) - elements = page.elements.published.where('position < ?', position) + elements = page.elements.published.where("position < ?", position) select_element(elements, name, :desc) end @@ -232,7 +232,7 @@ def trashed? # Returns true if the definition of this element has a taggable true value. def taggable? - definition['taggable'] == true + definition["taggable"] == true end # The opposite of folded? @@ -242,7 +242,7 @@ def expanded? # Defined as compact element? def compact? - definition['compact'] == true + definition["compact"] == true end # The element's view partial is dependent from its name @@ -279,7 +279,7 @@ def cache_key # A collection of element names that can be nested inside this element. def nestable_elements - definition.fetch('nestable_elements', []) + definition.fetch("nestable_elements", []) end # Copy all nested elements from current element to given target element. @@ -287,7 +287,7 @@ def copy_nested_elements_to(target_element) nested_elements.map do |nested_element| Element.copy(nested_element, { parent_element_id: target_element.id, - page_id: target_element.page_id + page_id: target_element.page_id, }) end end @@ -295,7 +295,7 @@ def copy_nested_elements_to(target_element) private def generate_nested_elements - definition.fetch('autogenerate', []).each do |nestable_element| + definition.fetch("autogenerate", []).each do |nestable_element| if nestable_elements.include?(nestable_element) Element.create(page: page, parent_element_id: id, name: nestable_element) else diff --git a/app/serializers/alchemy/content_serializer.rb b/app/serializers/alchemy/content_serializer.rb index e9b39494ec..a082e97c1e 100644 --- a/app/serializers/alchemy/content_serializer.rb +++ b/app/serializers/alchemy/content_serializer.rb @@ -6,7 +6,6 @@ class ContentSerializer < ActiveModel::Serializer :name, :ingredient, :element_id, - :position, :created_at, :updated_at, :settings diff --git a/db/migrate/20200226213334_alchemy_four_point_four.rb b/db/migrate/20200226213334_alchemy_four_point_four.rb index 3de012be7a..047d03cad1 100644 --- a/db/migrate/20200226213334_alchemy_four_point_four.rb +++ b/db/migrate/20200226213334_alchemy_four_point_four.rb @@ -23,12 +23,11 @@ def up t.string "essence_type", null: false t.integer "essence_id", null: false t.integer "element_id", null: false - t.integer "position" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.integer "creator_id" t.integer "updater_id" - t.index ["element_id", "position"], name: "index_contents_on_element_id_and_position" + t.index ["element_id"], name: "index_contents_on_element_id" t.index ["essence_id", "essence_type"], name: "index_alchemy_contents_on_essence_id_and_essence_type", unique: true end end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 74697e085d..f58810d234 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -30,12 +30,11 @@ t.string "essence_type", null: false t.integer "essence_id", null: false t.integer "element_id", null: false - t.integer "position" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.integer "creator_id" t.integer "updater_id" - t.index ["element_id", "position"], name: "index_contents_on_element_id_and_position" + t.index ["element_id"], name: "index_contents_on_element_id" t.index ["essence_id", "essence_type"], name: "index_alchemy_contents_on_essence_id_and_essence_type", unique: true end diff --git a/spec/models/alchemy/element_spec.rb b/spec/models/alchemy/element_spec.rb index 861a9629bb..88be7dfa45 100644 --- a/spec/models/alchemy/element_spec.rb +++ b/spec/models/alchemy/element_spec.rb @@ -398,23 +398,6 @@ module Alchemy end end - describe '#contents' do - let(:element) { create(:alchemy_element) } - let!(:content1) { create(:alchemy_content, element: element) } - let!(:content2) { create(:alchemy_content, element: element) } - - subject { element.contents } - - before do - content1.update_column(:position, 2) - content2.update_column(:position, 1) - end - - it 'are ordered by position' do - is_expected.to eq([content2, content1]) - end - end - describe '#content_by_type' do before(:each) do @element = create(:alchemy_element, name: 'headline')