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

Fail to upgrade to 5.0.2 #549

Closed
leonardow-unep-wcmc opened this issue Feb 3, 2024 · 3 comments
Closed

Fail to upgrade to 5.0.2 #549

leonardow-unep-wcmc opened this issue Feb 3, 2024 · 3 comments

Comments

@leonardow-unep-wcmc
Copy link

Hi,

I upgrading an application which was using ahoy_matey 1.0.1.
I follow the links below, to upgrade from 1.6.1 > 2.2.1 > 3.3.0 > 4.2.1 > 5.0.2

https://github.com/ankane/ahoy/tree/v1.6.1?tab=readme-ov-file#upgrading
https://github.com/ankane/ahoy/blob/v2.2.1/docs/Ahoy-2-Upgrade.md
https://github.com/ankane/ahoy/tree/v2.2.1?tab=readme-ov-file#upgrading
https://github.com/ankane/ahoy/tree/v3.3.0?tab=readme-ov-file#upgrading
https://github.com/ankane/ahoy/tree/v4.2.1?tab=readme-ov-file#upgrading
https://github.com/ankane/ahoy/tree/v5.0.2?tab=readme-ov-file#upgrading

All good to 4.2.1, however when I upgrade to 5.0.2, it raise SystemStackError: stack level too deep

     # ./config/initializers/ahoy.rb:19:in `visit'
     # /usr/local/bundle/gems/ahoy_matey-5.0.2/lib/ahoy/tracker.rb:94:in `visit'
     # /usr/local/bundle/gems/ahoy_matey-5.0.2/lib/ahoy/tracker.rb:210:in `visit_token_helper'
     # /usr/local/bundle/gems/ahoy_matey-5.0.2/lib/ahoy/tracker.rb:128:in `visit_token'
     # ./config/initializers/ahoy.rb:19:in `visit'
     # /usr/local/bundle/gems/ahoy_matey-5.0.2/lib/ahoy/tracker.rb:94:in `visit'
     # /usr/local/bundle/gems/ahoy_matey-5.0.2/lib/ahoy/tracker.rb:210:in `visit_token_helper'
     # /usr/local/bundle/gems/ahoy_matey-5.0.2/lib/ahoy/tracker.rb:128:in `visit_token'
     # /usr/local/bundle/gems/ahoy_matey-5.0.2/lib/ahoy/tracker.rb:83:in `authenticate'
     # /usr/local/bundle/gems/ahoy_matey-5.0.2/lib/ahoy/warden.rb:4:in `block in <top (required)>'

I traced the code, and found that a method in config/initializers/ahoy.rb, which 2.0 upgrade guide asked me to add, break.

def visit
    @visit ||= visit_model.find_by(id: ensure_uuid(ahoy.visit_token)) if ahoy.visit_token
end

I also changed the index from 5.0 upgrade, which ask me to create an index:

add_index :ahoy_visits, [:visitor_token, :started_at]

However my database column is named visitor_id not visitor_token, so I changed it to:

add_index :ahoy_visits, [:visitor_id, :started_at]

But I don't think this is related to the error.

Should I just remove the visit method in initializers? What is the right way to fix this?
Any input will be appreciated, thanks.

@ankane
Copy link
Owner

ankane commented Feb 3, 2024

Hi @leonardow-unep-wcmc, you'll need to copy the latest version of that method and adapt it to your model. I believe you'll want to replace:

  1. where(visit_token: ahoy.visit_token) with where(id: ensure_uuid(ahoy.visit_token))
  2. where(visitor_token: ahoy.visitor_token) with where(visitor_id: ensure_uuid(ahoy.visitor_token))

@leonardow-unep-wcmc
Copy link
Author

Thanks for your reply. Now I understand that the reason for the overriding is due to column names changed in v1.4.0.

After apply your suggestion, ahoy.track "something", {} in controller raise error:

NoMethodError (undefined method `visit_token' for #<Ahoy::Visit ...>

I solve it by using the following:

# config/initializers/ahoy.rb

class Ahoy::Store < Ahoy::DatabaseStore
  def authenticate(data)
    # https://github.com/ankane/ahoy/tree/v5.0.2?tab=readme-ov-file#gdpr-compliance-1
    # disables automatic linking of visits and users (for GDPR compliance)
  end

  def track_visit(data)
    # Map the new column names (since 1.4.0), to old column name (< 1.4.0).
    data[:id] = ensure_uuid(data.delete(:visit_token))
    data[:visitor_id] = ensure_uuid(data.delete(:visitor_token))
    super(data)
  end

  def track_event(data)
    # Map the new column names (since 1.4.0), to old column name (< 1.4.0).
    data[:id] = ensure_uuid(data.delete(:event_id))
    super(data)
  end

  def ensure_uuid(id)
    UUIDTools::UUID.parse(id).to_s
  rescue
    UUIDTools::UUID.sha1_create(UUIDTools::UUID.parse(Ahoy::Tracker::UUID_NAMESPACE), id).to_s
  end
end
# app/models/ahoy/visit.rb
module Ahoy
  class Visit < ApplicationRecord
    # Map the new column names (since 1.4.0), to old column name (< 1.4.0).
    alias_attribute :visit_token, :id
    alias_attribute :visitor_token, :visitor_id
  end
end

Could you tell me if you think the above approach is feasible? Would it raise any other issues?
Thanks.

@ankane
Copy link
Owner

ankane commented Feb 5, 2024

That looks reasonable to me (along with the visit method change), but you'll need to try it out to see if there are other issues.

@ankane ankane closed this as completed Feb 5, 2024
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

No branches or pull requests

2 participants