search

Casecommons / pg_search

pg_search builds ActiveRecord named scopes that take advantage of PostgreSQL’s full text search
http://www.casebook.net
MIT License
1.3k stars 369 forks source link

Readme updates for delete_by and update_if option #496

Closed raspygold closed 1 year ago

raspygold commented 1 year ago

454 made some changes to the readme to replace delete_all with delete_by, but one instance remains and this PR updates that

The current readme has an example for multisearch's update_if using the ActiveRecord *_changed? method to determine if the pg search document should be updated. However, because update_if is evaluated in an after_save callback the dirty flags have already been cleared which may result in pg search documents not being updated as expected. Instead, I'd suggest people use *_previously_changed? as it returns whether the attribute was changed before the model was saved is more useful for more cases.

There are some whitespace changes included in this PR which I can remove if you'd prefer.

nertzy commented 1 year ago

I think this is an improvement, although I would argue that both the delete_all and delete_by examples are wrong if you look at the actual implementation:

https://github.com/Casecommons/pg_search/blob/892eec7fb656ad9d78fb96380e34fe0686bf6439/lib/pg_search/multisearch.rb#L27

raspygold commented 1 year ago

@nertzy thanks for looking at my PR and apologies for the long round trip on the response

I've update the readme to remove the incorrect rebuild strategy when the :against option is present on the multisearchable. I've added a text description instead.

I've left the original delete_by example though as it isn't trying to describe the implemented behaviour so much as offer a way to delete indexed search records for a given model which I believe to be accurate.

Let me know what you think :)