diff --git a/CHANGELOG.md b/CHANGELOG.md index 1edb779..3afffd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ -## 7.0.1 (Next) +## 7.1.0 (Next) +* [#274](https://github.com/mongoid/mongoid-slug/pull/274): Added support for scoping slugs by multiple fields - [@mikekosulin](https://github.com/mikekosulin) * Your contribution here. ## 7.0.0 (2023/09/18) diff --git a/README.md b/README.md index 358c613..41f880e 100644 --- a/README.md +++ b/README.md @@ -193,6 +193,22 @@ class Employee end ``` +You may scope slugs using multiple fields as per the following example: + +```ruby +class Employee + include Mongoid::Document + include Mongoid::Slug + + field :name + field :company_id + field :department_id + + # Scope slug uniqueness by a combination of company and department + slug :name, scope: %i[company_id department_id] +end +``` + ### Slug Max Length MongoDB [featureCompatibilityVersion](https://docs.mongodb.com/manual/reference/command/setFeatureCompatibilityVersion/#std-label-view-fcv) diff --git a/lib/mongoid/slug.rb b/lib/mongoid/slug.rb index 93e853c..1f34b49 100644 --- a/lib/mongoid/slug.rb +++ b/lib/mongoid/slug.rb @@ -57,9 +57,10 @@ module ClassMethods # @param options [Boolean] :permanent Whether the slug should be # immutable. Defaults to `false`. # @param options [Array] :reserve` A list of reserved slugs - # @param options :scope [Symbol] a reference association or field to - # scope the slug by. Embedded documents are, by default, scoped by - # their parent. + # @param options :scope [Symbol, Array] a reference association, field, + # or array of fields to scope the slug by. + # Embedded documents are, by default, scoped by their parent. Now it supports not only + # a single association or field but also an array of them. # @param options :max_length [Integer] the maximum length of the text portion of the slug # @yield If given, a block is used to build a slug. # @@ -90,8 +91,7 @@ def slug(*fields, &block) # Set indexes if slug_index && !embedded? - Mongoid::Slug::IndexBuilder.build_indexes(self, slug_scope_key, slug_by_model_type, - options[:localize]) + Mongoid::Slug::IndexBuilder.build_indexes(self, slug_scope_keys, slug_by_model_type, options[:localize]) end self.slug_url_builder = block_given? ? block : default_slug_url_builder @@ -113,13 +113,23 @@ def look_like_slugs?(*args) with_default_scope.look_like_slugs?(*args) end - # Returns the scope key for indexing, considering associations + def slug_scopes + # If slug_scope is set (i.e., not nil), we convert it to an array to ensure we can handle it consistently. + # If it's not set, we use an array with a single nil element, signifying no specific scope. + slug_scope ? Array(slug_scope) : [nil] + end + + # Returns the scope keys for indexing, considering associations # # @return [ Array, Document ] - def slug_scope_key + def slug_scope_keys return nil unless slug_scope - reflect_on_association(slug_scope).try(:key) || slug_scope + # If slug_scope is an array, we map over its elements to get each individual scope's key. + slug_scopes.map do |individual_scope| + # Attempt to find the association and get its key. If no association is found, use the scope as-is. + reflect_on_association(individual_scope).try(:key) || individual_scope + end end # Find documents by slugs. @@ -297,7 +307,7 @@ def new_with_slugs? def persisted_with_slug_changes? if localized? changes = _slugs_change - return (persisted? && false) if changes.nil? + return false if changes.nil? # ensure we check for changes only between the same locale original = changes.first.try(:fetch, I18n.locale.to_s, nil) diff --git a/lib/mongoid/slug/index_builder.rb b/lib/mongoid/slug/index_builder.rb index 8158f8d..c6e000d 100644 --- a/lib/mongoid/slug/index_builder.rb +++ b/lib/mongoid/slug/index_builder.rb @@ -8,7 +8,7 @@ module IndexBuilder # Creates indexes on a document for a given slug scope # # @param [ Mongoid::Document ] doc The document on which to create the index(es) - # @param [ String or Symbol ] scope_key The optional scope key for the index(es) + # @param [ String or Symbol or Array ] scope_key The optional scope key for the index(es) # @param [ Boolean ] by_model_type Whether or not to use single table inheritance # @param [ Boolean or Array ] localize The locale for localized index field # @@ -28,7 +28,8 @@ def build_index(doc, scope_key = nil, by_model_type = false, locale = nil) # See: http://docs.mongodb.org/manual/core/index-compound/ fields = {} fields[:_type] = 1 if by_model_type - fields[scope_key] = 1 if scope_key + + Array(scope_key).each { |key| fields[key] = 1 } locale = ::I18n.default_locale if locale.is_a?(TrueClass) if locale diff --git a/lib/mongoid/slug/unique_slug.rb b/lib/mongoid/slug/unique_slug.rb index 4596a29..3132129 100644 --- a/lib/mongoid/slug/unique_slug.rb +++ b/lib/mongoid/slug/unique_slug.rb @@ -100,10 +100,12 @@ def find_unique(attempt = nil) where_hash[:_slugs.all] = [regex_for_slug] where_hash[:_id.ne] = model._id - if (scope = slug_scope) && reflect_on_association(scope).nil? + Array(slug_scope).each do |individual_scope| + next unless reflect_on_association(individual_scope).nil? + # scope is not an association, so it's scoped to a local field # (e.g. an association id in a denormalized db design) - where_hash[scope] = model.try(:read_attribute, scope) + where_hash[individual_scope] = model.try(:read_attribute, individual_scope) end where_hash[:_type] = model.try(:read_attribute, :_type) if slug_by_model_type @@ -143,26 +145,59 @@ def regex_for_slug end def uniqueness_scope - if slug_scope && (metadata = reflect_on_association(slug_scope)) - - parent = model.send(metadata.name) - - # Make sure doc is actually associated with something, and that - # some referenced docs have been persisted to the parent - # - # TODO: we need better reflection for reference associations, - # like association_name instead of forcing collection_name here - # -- maybe in the forthcoming Mongoid refactorings? - inverse = metadata.inverse_of || collection_name - return parent.respond_to?(inverse) ? parent.send(inverse) : model.class + # If slug_scope is present, we need to handle whether it's a single scope or multiple scopes. + if slug_scope + # We'll track individual scope results in an array. + scope_results = [] + + Array(slug_scope).each do |individual_scope| + next unless (metadata = reflect_on_association(individual_scope)) + + # For each scope, we identify its association metadata and fetch the parent record. + parent = model.send(metadata.name) + + # It's important to handle nil cases if the parent record doesn't exist. + if parent.nil? + # You might want to handle this scenario differently based on your application's logic. + next + end + + # Make sure doc is actually associated with something, and that + # some referenced docs have been persisted to the parent + # + # TODO: we need better reflection for reference associations, + # like association_name instead of forcing collection_name here + # -- maybe in the forthcoming Mongoid refactorings? + inverse = metadata.inverse_of || collection_name + next unless parent.respond_to?(inverse) + + # Add the associated records of the parent (based on the inverse) to our results. + scope_results << parent.send(inverse) + end + + # After iterating through all scopes, we need to decide how to combine the results (if there are multiple). + # This part depends on how your application should treat multiple scopes. + # Here, we'll simply return the first non-empty scope result as an example. + scope_results.each do |result| + return result if result.present? # or any other logic for selecting among multiple scope results + end + + # If we reach this point, it means no valid parent scope was found (all were nil or didn't match the + # conditions). + # You might want to raise an error, return a default scope, or handle this scenario based on your + # application's logic. + # For this example, we're returning the model's class as a default. + return model.class end + # The rest of your method remains unchanged, handling cases where slug_scope isn't defined. + # This is your existing logic for embedded models or deeper superclass retrieval. if embedded? parent_metadata = reflect_on_all_association(:embedded_in)[0] return model._parent.send(parent_metadata.inverse_of || self.metadata.name) end - # unless embedded or slug scope, return the deepest document superclass + # Unless embedded or slug scope, return the deepest document superclass. appropriate_class = model.class appropriate_class = appropriate_class.superclass while appropriate_class.superclass.include?(Mongoid::Document) appropriate_class diff --git a/lib/mongoid/slug/version.rb b/lib/mongoid/slug/version.rb index e4eac64..6ccf1bb 100644 --- a/lib/mongoid/slug/version.rb +++ b/lib/mongoid/slug/version.rb @@ -2,6 +2,6 @@ module Mongoid # :nodoc: module Slug - VERSION = '7.0.0' + VERSION = '7.1.0' end end diff --git a/spec/models/page_with_categories.rb b/spec/models/page_with_categories.rb new file mode 100644 index 0000000..12e38e4 --- /dev/null +++ b/spec/models/page_with_categories.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class PageWithCategories + include Mongoid::Document + include Mongoid::Slug + field :title + field :content + + field :page_category + field :page_sub_category + + field :order, type: Integer + slug :title, scope: %i[page_category page_sub_category] + default_scope -> { asc(:order) } +end diff --git a/spec/mongoid/index_builder_spec.rb b/spec/mongoid/index_builder_spec.rb index 3ec3aef..c782767 100644 --- a/spec/mongoid/index_builder_spec.rb +++ b/spec/mongoid/index_builder_spec.rb @@ -51,6 +51,35 @@ end end + context 'when scope_key is set and is array' do + let(:doc) do + Class.new do + include Mongoid::Document + field :title, type: String + field :page_category, type: String + field :page_sub_category, type: String + end + end + let(:scope_key) { %i[page_category page_sub_category] } + + before do + doc.field :page_category, type: String + doc.field :page_sub_category, type: String + + Mongoid::Slug::IndexBuilder.build_indexes(doc, scope_key, by_model_type, locales) + end + + context 'when by_model_type is true' do + let(:by_model_type) { true } + + it { is_expected.to eq [[{ _slugs: 1, page_category: 1, page_sub_category: 1, _type: 1 }, {}]] } + end + + context 'when by_model_type is false' do + it { is_expected.to eq [[{ _slugs: 1, page_category: 1, page_sub_category: 1 }, {}]] } + end + end + context 'when scope_key is not set' do context 'when by_model_type is true' do let(:by_model_type) { true } diff --git a/spec/mongoid/slug_spec.rb b/spec/mongoid/slug_spec.rb index 46c46f0..a54ce32 100644 --- a/spec/mongoid/slug_spec.rb +++ b/spec/mongoid/slug_spec.rb @@ -163,6 +163,54 @@ module Mongoid end end + context 'when the object has multiple scopes' do + let(:category1) { 'category1' } + let(:category2) { 'category2' } + let(:sub_category1) { 'sub_category1' } + let(:sub_category2) { 'sub_category2' } + let(:common_title) { 'Common Title' } + + context 'when pages have the same title and different categories' do + it 'creates pages with the same slug' do + page1 = PageWithCategories.create!(title: common_title, page_category: category1) + page2 = PageWithCategories.create!(title: common_title, page_category: category2) + + expect(page1.slug).to eq(page2.slug) + end + end + + context 'when pages have the same title and same category but different sub-categories' do + it 'creates pages with the same slug' do + page1 = PageWithCategories.create!(title: common_title, page_category: category1, + page_sub_category: sub_category1) + page2 = PageWithCategories.create!(title: common_title, page_category: category1, + page_sub_category: sub_category2) + + expect(page1.slug).to eq(page2.slug) + end + end + + context 'when pages have the same title, same category, and same sub-category' do + it 'creates pages with different slugs' do + page1 = PageWithCategories.create!(title: common_title, page_category: category1, + page_sub_category: sub_category1) + page2 = PageWithCategories.create!(title: common_title, page_category: category1, + page_sub_category: sub_category1) + + expect(page1.slug).not_to eq(page2.slug) + end + end + + context 'when pages have the same title and same category, without sub-categories' do + it 'creates pages with different slugs' do + page1 = PageWithCategories.create!(title: common_title, page_category: category1) + page2 = PageWithCategories.create!(title: common_title, page_category: category1) + + expect(page1.slug).not_to eq(page2.slug) + end + end + end + context 'when the object is embedded' do let(:subject) do book.subjects.create(name: 'Psychoanalysis') diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ce43a00..e2e5fb1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -48,6 +48,7 @@ def database_id Book.create_indexes AuthorPolymorphic.create_indexes BookPolymorphic.create_indexes + PageWithCategories.create_indexes end c.after(:each) do