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

Use the writing role for increment_usage_count #170

Merged
merged 4 commits into from
Mar 4, 2024

Conversation

hughkelsey
Copy link
Contributor

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
Copy link
Collaborator

fschwahn commented Mar 1, 2024

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
Copy link
Collaborator

fschwahn commented Mar 1, 2024

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

@hughkelsey
Copy link
Contributor Author

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
Copy link
Collaborator

fschwahn commented Mar 1, 2024

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
Copy link
Contributor Author

Done!

@fschwahn fschwahn merged commit c323af3 into jpmcgrath:develop Mar 4, 2024
12 checks passed
@fschwahn
Copy link
Collaborator

fschwahn commented Mar 4, 2024

Thank you!

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.

2 participants