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

feat: mysql support #43

Merged
merged 8 commits into from
May 8, 2024
Merged

feat: mysql support #43

merged 8 commits into from
May 8, 2024

Conversation

danielrudn
Copy link
Contributor

@danielrudn danielrudn commented Apr 25, 2024

This PR adds support for mysql with the mysql2 adapter.

It is a major version bump 2.0.0 -> 3.0.0 because the pg and pg_search dependencies are now development dependencies. See the Breaking Change section for more details.

The rspec test suite now runs on both Postgres and MySQL databases during the test github action.

Breaking Change

The gem no longer depends directly on the pg_search gem. This means
models using pg_search_scope must be updated to explicitly include PgSearch::Model.

When upgrading from 2.x to 3.x, follow these steps:

  1. Ensure pg and pg_search gems are present if pg_search_scope is used
  2. Search for usage of include SnFilterable::Filterable that also uses pg_search_scope
  3. Add a line to include PgSearch::Model for results from step 2

@danielrudn danielrudn requested a review from a team as a code owner April 25, 2024 22:20
Comment on lines -196 to -204
it "can search ignoring diacritics" do
all_items = BasicFilterableTestModel.where(nil)
expected_items = [all_items.first.name]

filtered = BasicFilterableTestModel.filter(params: ActionController::Parameters.new({ filter: { search: target_user.name.split(//).join("").tr("aeiouylszcn", "àèîôûÿłšżçñ") } }))

expect(filtered.items.map(&:name)).to eq(expected_items)
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this test had a dependency on pg_search and seems pretty specific to pg_search so I removed it

Comment on lines +520 to +536
if ActiveRecord::Base.connection_db_config.adapter == "mysql2"
"LEFT OUTER JOIN `#{first_model_table}` ON `#{first_model_table}`.id = `#{base_model_table}`.`#{polymorphic_association_id}` AND `#{base_model_table}`.`#{polymorphic_association_type}` = '#{first_model_name}'"
else
"LEFT OUTER JOIN \"#{first_model_table}\" ON \"#{first_model_table}\".id = \"#{base_model_table}\".\"#{polymorphic_association_id}\" AND \"#{base_model_table}\".\"#{polymorphic_association_type}\" = '#{first_model_name}'"
end
end
end

describe ".coalesce" do
it "creates a valid function" do
actual_output = described_class.coalesce([DummyModel], "some_column")

expect(actual_output).to eq("coalesce(\"dummy_models\".\"some_column\")")
if ActiveRecord::Base.connection_db_config.adapter == "mysql2"
expect(actual_output).to eq("coalesce(`dummy_models`.`some_column`)")
else
expect(actual_output).to eq("coalesce(\"dummy_models\".\"some_column\")")
end
Copy link
Contributor Author

@danielrudn danielrudn Apr 25, 2024

Choose a reason for hiding this comment

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

there were only two tests that were expecting postgres syntax

Copy link
Member

@michaelroudnitski michaelroudnitski left a comment

Choose a reason for hiding this comment

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

LGTM

@danielrudn danielrudn merged commit f670e21 into main May 8, 2024
4 of 5 checks passed
@danielrudn danielrudn deleted the feat/mysql branch May 8, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants