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

Handle composite primary keys in scopes #537

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

causztic
Copy link

@causztic causztic commented Jul 24, 2024

Fixes #531

  • Make search scope work with composite primary keys
  • Test associations with composite primary keys

@causztic causztic changed the title Handle composite primary keys Handle composite primary keys in scopes Jul 24, 2024
@causztic causztic marked this pull request as draft July 25, 2024 09:31
@nertzy
Copy link
Collaborator

nertzy commented Aug 13, 2024

A good place to start would be to set up something in the spec/integration directory following the general pattern there. I think making a new file such as spec/integration/composite_primary_key_spec.rb may be a good approach.

The test should set up a model / DB table using with_model and with a pg_search_scope applied. It should perform some queries against the model and assert that the correct results are returned.

If we have this level of testing, then I think that the more specific "unit"-level specs are more optional and only really needed if there is some complex or surprising behavior in one of the specific implementation classes.

@nertzy
Copy link
Collaborator

nertzy commented Aug 13, 2024

You can also run rake standard:fix to fix the code formatting issues that are being triggered.

Good luck, and thanks for this contribution.

@nertzy nertzy self-assigned this Aug 13, 2024
include PgSearch::Model
belongs_to :parent, foreign_key: [:parent_first_name, :parent_last_name]

pg_search_scope :search_parent_hobby, associated_against: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's great that you are testing associated_against, good catch!

@causztic causztic marked this pull request as ready for review August 18, 2024 01:44
@causztic
Copy link
Author

causztic commented Aug 18, 2024

@nertzy before e1fd67d I attempted to test when both the model AND the association have composite primary keys, and quickly realised that we need some more changes on association.rb to make it work

Could we leave that for a future PR?

@causztic
Copy link
Author

@nertzy checking in ICYMI!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when searching a table with a composite primary key
2 participants