422 deactivate services when parent resource is deactivated #431
Conversation
cliffcrosland
left a comment
cliffcrosland
left a comment
There was a problem hiding this comment.
Some small comments, but otherwise LGTM
|
|
||
| module Services | ||
| module Resources | ||
| include Errors |
There was a problem hiding this comment.
Rails might auto-load this module, so I think it might be possible to delete this include Errors line here. In development mode, Rails lazily loads modules, so when it encounters raise Errors::PreconditionFailed, it will go look for the appropriate file and load it into scope.
| remove_from_algolia(service) | ||
| end | ||
|
|
||
| def self.remove_from_algolia(service) |
There was a problem hiding this comment.
Maybe this can be factored out as a common utility since it is used in both services? Might be able to put it in app/services/utils.rb? Perhaps the error log message can be generalized as follows:
def self.remove_from_algolia(record)
record.remove_from_index!
rescue StandardError
puts "failed to remove #{record.class} #{record.id} from algolia index"
end| # frozen_string_literal: true | ||
|
|
||
| module Services | ||
| module Resources |
There was a problem hiding this comment.
Might be cool to add some rspec tests for these services, but if their functionality is already covered by the Postman tests, there is probably no need.
|
Haha, looks like Rubocop wants you to add empty newlines after the |
3 participants
No description provided.