Skip to content

Commit

Permalink
add multi-field scoping for slugs (#274)
Browse files Browse the repository at this point in the history
* add multiple scopes support

* upd changelog and readme

* fix some comments

* refactor array iterations

* fix compound indexes

* rename for clarity

* fix readme

* Update README.md

* Update slug.rb

---------

Co-authored-by: Johnny Shields <27655+johnnyshields@users.noreply.github.com>
  • Loading branch information
mikekosulin and johnnyshields authored Apr 21, 2024
1 parent 6b6ad2f commit c52831e
Show file tree
Hide file tree
Showing 10 changed files with 184 additions and 28 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 19 additions & 9 deletions lib/mongoid/slug.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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<Symbol>] 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.
#
Expand Down Expand Up @@ -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
Expand All @@ -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>, 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.
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions lib/mongoid/slug/index_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Symbol> ] 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
#
Expand All @@ -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
Expand Down
65 changes: 50 additions & 15 deletions lib/mongoid/slug/unique_slug.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/mongoid/slug/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

module Mongoid # :nodoc:
module Slug
VERSION = '7.0.0'
VERSION = '7.1.0'
end
end
15 changes: 15 additions & 0 deletions spec/models/page_with_categories.rb
Original file line number Diff line number Diff line change
@@ -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
29 changes: 29 additions & 0 deletions spec/mongoid/index_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
48 changes: 48 additions & 0 deletions spec/mongoid/slug_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
1 change: 1 addition & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def database_id
Book.create_indexes
AuthorPolymorphic.create_indexes
BookPolymorphic.create_indexes
PageWithCategories.create_indexes
end

c.after(:each) do
Expand Down

0 comments on commit c52831e

Please sign in to comment.