Fix ActiveJob::DeserializationError issue #422
Conversation
|
Hi @dsgh , Thank you for opening this PR! 🙇♂️ If you have some time, would it be possible to add a test for this change in functionality please? Also, this seems to be a BC-breaking change, so we can only release this in a new major 🙂 |
|
@DevinCodes I don't see any existing tests for |
|
Anyway to move this forward? |
There was a problem hiding this comment.
This looks good to me, thank you again for the contribution and apologies for the delay on this. Since this is a breaking change, we can only release this in the next major version. To be fully transparent, I don't know when this will happen, but I hope we can get a few other contributions in before we release a new major.
Thank you for your understanding 🙇♂️
DevinCodes
added
the
next major
| @@ -399,7 +399,8 @@ def algoliasearch(options = {}, &block) | |||
| raise ArgumentError.new("Cannot use a enqueue if the `synchronous` option if set") if options[:synchronous] | |||
| proc = if options[:enqueue] == true | |||
| Proc.new do |record, remove| | |||
| AlgoliaJob.perform_later(record, remove ? 'algolia_remove_from_index!' : 'algolia_index!') | |||
| record_or_object_id = remove ? algolia_object_id_of(record) : record | |||
There was a problem hiding this comment.
@dsgh @DevinCodes I think this line needs to be more general. If a single model instance is included in two indexes, and has a unique objectID in each of those indexes, this will not remove the record from each Algolia index in which it's included.
To remove the instance from each index in which it's included, we need to specify the Algolia index options for each relevant index in this call to algolia_object_id_of. i.e. this should probably be an array of objectID values, one per relevant index
There was a problem hiding this comment.
For what it's worth, we have a pretty complex setup in which several models are in several indices, and many of those indices include multiple models. Some of the models also use custom objectID values in indexes (so that e.g. Message=3 and User=3 will not have colliding objectIDs in the indices in which both are present).
When a record needs to be deleted, we find each of the indexes in which the record is included, and find the name of each of those indexes. Then we send that 2D array of [[object_id, index_name], ...] to our ActiveJob, from which we can retrieve the relevant index with index = Algolia::Index.new(index_name) and then remove the object with index.delete_object(object_id). This ensures that objects with custom object ids that are included in several indices are properly removed from each when the model instance is destroyed
Describe your change
This is a different attempt at fixing the issue, following #369 being closed.
Change arguments passed to AlgoliaJob, so that we can run
algolia_remove_from_index!correctly.Change
algolia_remove_from_index!so that if the object is a string or numeric, it's considered to be the object_id.What problem is this fixing?
When a record is removed and jobs are set to run async, an error happens:
ActiveJob::DeserializationError: Error while trying to deserialize arguments- see #359