Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add multi-field scoping for slugs #274

Merged
merged 9 commits into from
Apr 21, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)

mikekosulin marked this conversation as resolved.
Show resolved Hide resolved
* [#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
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,26 @@ class Employee
end
```

Sometimes, system constraints prevent using relation-based scoping. When this happens, you can scope slugs using multiple fields, addressing needs like database structure or performance issues.

Here's a quick setup for multi-field scoping:

```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
```

Note: this approach creates multiple indexes, differing from single-field scoping, and impacting database performance and storage.
Copy link
Member

@johnnyshields johnnyshields Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... shouldn't this still create only one compound index? For example:

slug :name, scope: %i[company_id department_id]

# Should create
index company_id: 1, department_id: 1, _slugs: 1

# NOT
index company_id: 1, _slugs: 1
index department_id: 1, _slugs: 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late response. You're right, it creates only a compound index in this case. I've fixed it here and forgot to remove the note.


### Slug Max Length

MongoDB [featureCompatibilityVersion](https://docs.mongodb.com/manual/reference/command/setFeatureCompatibilityVersion/#std-label-view-fcv)
Expand Down
22 changes: 16 additions & 6 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_key, 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

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 key for indexing, considering associations
#
# @return [ Array<Document>, Document ]
def slug_scope_key
mikekosulin marked this conversation as resolved.
Show resolved Hide resolved
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
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
Loading