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

Issue when trying to skip set_current_tenant_account from ApplicationControlller #20

Open
matedemorphy opened this issue Mar 29, 2023 · 4 comments
Assignees

Comments

@matedemorphy
Copy link

Hi, i'm having an issue:

skip_before_action does not actually execute or perform any action. Something I find odd is that when I enter to a url with a valid subdomain in the database, even though the tenant is assigned correctly, no job is executed in Sidekiq. In any case, without Sidekiq enabled the behavior is the same, I have already given a thousand turns and I do not find the reason.

application_controller.rb

class ApplicationController < ActionController::Base
  rescue_from MultiTenantSupport::NilTenantError, with: :not_found  
  skip_before_action :set_current_tenant_account, unless: -> { storefront_action? }
  before_action :authenticate_seller!, unless: -> { storefront_action? || controller_name == "errors" }
  
  private

  def find_current_tenant_account
    raise MultiTenantSupport::NilTenantError unless store_tenant = Store.find_by("subdomain = ? OR domain = ?", request.subdomain, request.domain)
    store_tenant
  end

  def storefront_action?
    controller_name == "stores" && action_name.start_with?("storefront_")
  end

  def not_found(exception)
    redirect_to :not_found
  end
end

multi_tenant_support.rb

MultiTenantSupport.configure do
  model do |config|
    config.tenant_account_class_name = 'Store'
    config.tenant_account_primary_key = :id
  end

  controller do |config|
    config.current_tenant_account_method = :current_tenant_account
  end

  app do |config|
    config.excluded_subdomains = ['www']
    config.host = 'lvh.me'
  end

  console do |config|
    config.allow_read_across_tenant_by_default = true
  end
end

import.rb

class Import < ApplicationRecord
  belongs_to_tenant :store

  # == Schema Information
  #
  # Table name: imports
  #
  #  id                  :bigint           not null, primary key
  #  created_at          :datetime         not null
  #  updated_at          :datetime         not null
  #  store_id            :bigint
  #
  # Indexes
  #
  #  index_imports_on_store_id    (store_id)
  #
  # Foreign Keys
  #
  #  fk_rails_...  (store_id => stores.id)
end

store.rb

class Store < ApplicationRecord
  has_many :imports

  # == Schema Information
  #
  # Table name: stores
  #
  #  id         :bigint           not null, primary key
  #  domain     :string
  #  subdomain  :string
  #  created_at :datetime         not null
  #  updated_at :datetime         not null
end

Thanks in advance.

@hoppergee
Copy link
Owner

hoppergee commented Mar 29, 2023

Sorry, I'm not quite get the point. It seems you met two issues:

  • Can't skip set_current_tenant_account with skip_before_action?
  • Have some problems with sidekiq?

Please help to give more details. Currently, I can't figure out what's the problem you're facing.

And please also help to provide tech details like

  • Ruby version
  • Rails version
  • Sidekiq version

Then I can create demo for debugging. Thanks

@hoppergee hoppergee self-assigned this Mar 29, 2023
@matedemorphy
Copy link
Author

matedemorphy commented Mar 29, 2023

Indeed, Can't skip set_current_tenant_account with skip_before_action and uncomment require 'multi_tenant_support/sidekiq' does not enables Sidekiq, but i just found a workarround for this especial case:

set_tenant_job.rb

class SetTenantJob < ApplicationJob
  queue_as :default

  def perform(storefront_action, subdomain, domain, store_id, controller_name, action_name)
    store_tenant = find_store(storefront_action, subdomain, domain, store_id, controller_name, action_name)
    store_tenant
  end

  private

  def find_store(storefront_action, subdomain, domain, store_id, controller_name, action_name)
    if storefront_action && controller_name != "errors"
      raise MultiTenantSupport::NilTenantError unless store_tenant = Store.find_by("subdomain = ? OR domain = ?", subdomain, domain)
    else
      return store_id.nil? ? Store.new : Store.find(store_id)
    end
    store_tenant
  end
end

application_controller.rb

class ApplicationController < ActionController::Base
  rescue_from ActiveRecord::RecordNotFound, with: :not_found
  rescue_from MultiTenantSupport::NilTenantError, with: :not_found
  rescue_from MultiTenantSupport::MissingTenantError, with: :not_found  
  before_action :authenticate_seller!, unless: -> { storefront_action? || controller_name == "errors" }

  private

  def find_current_tenant_account
    store = SetTenantJob.perform_now(storefront_action?, request.subdomain, request.domain, current_seller&.store&.id, controller_name, action_name)
    store
  end

  def storefront_action?
    controller_name == "stores" && action_name.start_with?("storefront_")
  end

  def not_found(exception)
    redirect_to :not_found
  end
end

Tech details:

Ruby version: 3.2.0
Rails version: 7.0.4
Sidekiq version: 7.0.7

@hoppergee
Copy link
Owner

Hi @matedemorphy , thanks for those details. I did some tests. Here is what I found:

  • The skip_before_action :set_current_tenant_account, unless: -> { storefront_action? } is working. Maybe you can try to set some debugger point in your codebase to find the real reason.
  • Please comment out require 'multi_tenant_support/active_job' instead of the require 'multi_tenant_support/sidekiq' even you're using sidekiq as the ActiveJob's adapter.
    • And in each job's perform method, you can get the tenant through current_tenant. (It seems I forgot to write this in README, will add it later)

If there's anything new, please put them here. I would like to help.

@hoppergee
Copy link
Owner

hoppergee commented Mar 30, 2023

Currently, our full test cases haven't run against Ruby 3.2, Rails 7 and Sidekiq 7. I did a manual test, it seems work well. However, I will take some times to finish the whole compatible works to make sure I cover all the edge cases in the weekend maybe.

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