jpmcgrath / shortener

Shortener makes it easy to create shortened URLs for your rails application.
MIT License
539 stars 223 forks source link

Use the writing role for increment_usage_count #170

Closed hughkelsey closed 7 months ago

hughkelsey commented 7 months ago

Within my app I'm having an issue where shortener is writing during a GET request and we need to specifically use the writing role to increment_usage_count.

All of the spec pass and the test application does not have any special setup for multi-db so I suspect this will have no issues unless there is some way to name the writing role differently.

I am open to other suggestions, like using an initializer but I'm unsure the best approach.

This is in regards to my issue #169 .

fschwahn commented 7 months ago

This is probably incompatible with the old rails versions this gem still supports. So would at least need to check if that method exists.

fschwahn commented 7 months ago

Maybe this is even more a documentation issue. See eg. https://github.com/heartcombo/devise/issues/5264 where this is also handled by overwrites. So adding documentation how to appease multi-db setups may be the way to go.

hughkelsey commented 7 months ago

Thanks @fschwahn . I've updated the PR to check if ActiveRecord::Base.respond_to?(:connected_to). I'm open to the documentation approach but devise is quite different being included in a base class. How about something like this? I think it would only require ActiveSupport.run_load_hooks :shortener_shortened_url, Shortener::ShortenedUrl to be added to Shortener::ShortenedUrl.

# config/initializers/monkey_patch_shortener_writer
module MonkeyPatchShortenerWriter
  def increment_usage_count
    ActiveRecord::Base.connected_to(role: :writing) do
      self.class.increment_counter(:use_count, id)
    end
  end
end

ActiveSupport.on_load(:shortener_shortened_url) do
  Shortener::ShortenedUrl.prepend(MonkeyPatchShortenerWriter)
end
fschwahn commented 7 months ago

I think it would only require ActiveSupport.run_load_hooks :shortener_shortened_url, Shortener::ShortenedUrl to be added to Shortener::ShortenedUrl.

That sounds great! Can you make that change? And also add a section in the README which explains this?

hughkelsey commented 7 months ago

Done!

fschwahn commented 7 months ago

Thank you!