Skip to content

Respect model's self.primary_key when creating Meilisearch indexes #425

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Respect model's self.primary_key when creating Meilisearch indexes #425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36c3eb1
Set primary_key in meilisearch_options during initial meilisearch load
thicolares Oct 10, 2025
3b305f0
Reduce size of ClassMethods and fix RuboCop line offense
thicolares Oct 10, 2025
5deec2a
Test extra scenarios for defined index primary_key
thicolares Oct 10, 2025
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
29 changes: 13 additions & 16 deletions lib/meilisearch-rails.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,7 @@ class << base

def meilisearch(options = {}, &block)
self.meilisearch_settings = IndexSettings.new(options, &block)
self.meilisearch_options = {
type: model_name.to_s.constantize,
per_page: meilisearch_settings.get_setting(:hitsPerPage) || 20, page: 1
}.merge(options)
self.meilisearch_options = default_meilisearch_options(meilisearch_settings, options)

attr_accessor :formatted

Expand All @@ -406,13 +403,7 @@ def meilisearch(options = {}, &block)
raise ArgumentError, 'Cannot use a enqueue if the `synchronous` option is set' if options[:synchronous]

proc = if options[:enqueue] == true
proc do |record, remove|
if remove
MSCleanUpJob.perform_later(record.ms_entries)
else
MSJob.perform_later(record, 'ms_index!')
end
end
proc { |record, remove| remove ? MSCleanUpJob.perform_later(record.ms_entries) : MSJob.perform_later(record, 'ms_index!') }
elsif options[:enqueue].respond_to?(:call)
options[:enqueue]
elsif options[:enqueue].is_a?(Symbol)
Expand Down Expand Up @@ -704,11 +695,7 @@ def ms_search(query, params = {})

condition_key = meilisearch_options[:type].primary_key if has_virtual_column_as_pk

hit_ids = if has_virtual_column_as_pk
json['hits'].map { |hit| hit[condition_key] }
else
json['hits'].map { |hit| hit[ms_pk(meilisearch_options).to_s] }
end
hit_ids = json['hits'].map { |hit| hit[has_virtual_column_as_pk ? condition_key : ms_pk(meilisearch_options).to_s] }

# meilisearch_options[:type] refers to the Model name (e.g. Product)
# results_by_id creates a hash with the primaryKey of the document (id) as the key and doc itself as the value
Expand Down Expand Up @@ -805,6 +792,16 @@ def ms_ensure_init(options = meilisearch_options, settings = meilisearch_setting

private

def default_meilisearch_options(meilisearch_settings, options)
type = model_name.to_s.constantize
{
type: type,
per_page: meilisearch_settings.get_setting(:hitsPerPage) || 20,
page: 1,
primary_key: type.respond_to?(:primary_key) ? type.primary_key : nil
}.merge(options)
end
Comment on lines +795 to +803
Copy link

@coderabbitai coderabbitai bot Oct 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Avoid constantizing model_name

model_name.to_s is often customized (e.g. ActiveModel::Name.new(self, nil, 'PublicFacingName')). In those cases 'PublicFacingName'.constantize raises NameError, so any model overriding model_name will now blow up as soon as it calls meilisearch. Previously we never constantized and those models worked fine. Please derive type from self (or fall back to self if constantization fails) instead of assuming model_name corresponds to a constant.

Apply this diff to keep the behaviour safe:

-        type = model_name.to_s.constantize
+        type = respond_to?(:name) && name ? name.safe_constantize || self : self
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def default_meilisearch_options(meilisearch_settings, options)
type = model_name.to_s.constantize
{
type: type,
per_page: meilisearch_settings.get_setting(:hitsPerPage) || 20,
page: 1,
primary_key: type.respond_to?(:primary_key) ? type.primary_key : nil
}.merge(options)
end
def default_meilisearch_options(meilisearch_settings, options)
type = respond_to?(:name) && name ? name.safe_constantize || self : self
{
type: type,
per_page: meilisearch_settings.get_setting(:hitsPerPage) || 20,
page: 1,
primary_key: type.respond_to?(:primary_key) ? type.primary_key : nil
}.merge(options)
end
🤖 Prompt for AI Agents 
In lib/meilisearch-rails.rb around lines 795 to 803, avoid constantizing
model_name; replace type = model_name.to_s.constantize with logic that derives
type from self (or attempts constantize and rescues NameError), e.g. set type =
self and only try to constantize model_name.to_s if you want to support that,
but rescue NameError and fall back to self; ensure subsequent primary_key lookup
uses the resolved type (type.respond_to?(:primary_key) ? type.primary_key : nil)
so models that override model_name do not raise.

Copy link
Author

@thicolares thicolares Oct 14, 2025

Choose a reason for hiding this comment

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

Previously we never constantized and those models worked fine.

Actually, the current code constantize already. Hence, I'm not changing it. Moreover, I believe it should fail instead of quietly nil (it would fail in the next lines) or something else. WDYT, maintainers? :)

Copy link

@coderabbitai coderabbitai bot Oct 14, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current code around line 381 to verify the user's claim
cat -n lib/meilisearch-rails.rb | sed -n '370,390p'

Length of output: 1050

🏁 Script executed:

#!/bin/bash
# Check the main branch to see what line 381 looked like before this PR
git fetch origin main
git show origin/main:lib/meilisearch-rails.rb | cat -n | sed -n '370,395p'

Length of output: 1411

@thicolares You're absolutely right, and I apologize for the confusion!

Looking at the main branch, line 381 indeed shows:

type: model_name.to_s.constantize,

Your PR is simply refactoring this existing logic into the default_meilisearch_options method without changing the behavior. The constantization has always been there, so you're not introducing any new risk.

I withdraw my previous suggestion. The refactoring looks good as-is. Thank you for the clarification and for catching my error! 👍

🧬 Code Graph Analysis Results

[lib/meilisearch-rails.rb] - default_meilisearch_options usage

  • Summary

    • In the class method default_meilisearch_options(meilisearch_settings, options), the code derives a type by calling model_name.to_s.constantize. This is the point where constantization happens to determine the model class for indexing.
    • The method returns a hash including:
      • type: the constantized model class
      • per_page: from meilisearch_settings[:hitsPerPage] or default 20
      • page: default 1
      • primary_key: derived from the type if it responds to primary_key, else nil
    • Merges the provided options into the resulting hash.

  • Rationale for relevance

    • This is the explicit place where constantization is performed within lib/meilisearch-rails.rb. The user comment references a constantize usage at this area; understanding this snippet clarifies how the model class is resolved and what happens if the model name cannot be constantized.


def update_settings_if_changed(index, options, user_configuration)
server_state = index.settings
user_configuration = options[:primary_settings].to_settings.merge(user_configuration) if options[:inherit]
Expand Down
19 changes: 18 additions & 1 deletion spec/options_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
require 'support/models/people'
require 'support/models/vegetable'
require 'support/models/fruit'
require 'support/models/story'
require 'support/models/disabled_models'
require 'support/models/queued_models'
require 'support/mongo_models/citizen'

describe 'meilisearch_options' do
describe ':index_uid' do
Expand All @@ -18,10 +20,25 @@
end

describe ':primary_key' do
it 'sets the primary key specified' do
it 'sets the primary key passed to #meilisearch' do
TestUtil.reset_people!
People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887)
expect(People.index.fetch_info.primary_key).to eq('card_number')
expect(MeiliSearch::Rails.client.index(People.index_uid).get_primary_key).to eq('card_number')
end

it 'sets the primary key defined in #primary_key' do
TestUtil.reset_stories!
Story.create(story_id: 1, title: 'A nice story')
expect(Story.index.fetch_info.primary_key).to eq('story_id')
expect(MeiliSearch::Rails.client.index(Story.index_uid).get_primary_key).to eq('story_id')
end

it 'sets the primary key as id if not explicitly set' do
TestUtil.reset_books!
Book.create(name: 'Test Book', author: 'Test Author', premium: true, released: true, genre: 'Horror')
expect(Book.index.fetch_info.primary_key).to eq('id')
expect(MeiliSearch::Rails.client.index(Book.index_uid).get_primary_key).to eq('id')
end
end

Expand Down
21 changes: 21 additions & 0 deletions spec/support/models/story.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
require 'support/active_record_schema'

ar_schema.create_table :stories do |t|
t.integer :story_id
t.string :title
end

class Story < ActiveRecord::Base
include MeiliSearch::Rails

self.primary_key = 'story_id' # Use model primary_key for index primary key

meilisearch index_uid: safe_index_uid('MyCustomStory') # Don't set primary_key in meilisearch
end

module TestUtil
def self.reset_stories!
Story.clear_index!
Story.delete_all
end
end