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

Add tenant: option to tagged_with #1102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

flickgradley
Copy link

@flickgradley flickgradley commented Mar 10, 2023

To improve multi tenancy support, I thought it would be good to add a tenant: option to the tagged_with query. We have 1.2M tagged objects in our database, and adding this tenant filter to that query should make it significantly faster to find objects tagged within a certain tenant.

@flickgradley flickgradley marked this pull request as ready for review March 10, 2023 01:52
@courtsimas
Copy link

@mbleigh can we get this added in? Would be a huge help for people.

@tvongaza
Copy link

tvongaza commented Oct 10, 2023

This is a great change. We are heavy users of the amazing ActsAsTenant gem and created the following rails initializer to scope our tags & taggings to the set tenant. This PRs change is a lot more flexible, though will require being a bit more explicit on the tenant setting (vs using ActsAsTenant & the require_tenant = true flag).

module ActsAsTaggableOnWithActsAsTenant
  def self.included(base)
    base.class_eval do
      acts_as_tenant(:account)
    end
  end
end

ActsAsTaggableOn::Tag.include(ActsAsTaggableOnWithActsAsTenant)
ActsAsTaggableOn::Tagging.include(ActsAsTaggableOnWithActsAsTenant)

module ActsAsTaggableOnTagNameValidationWithActsAsTenant
  def self.included(base)
    base.class_eval do
      # https://github.com/mbleigh/acts-as-taggable-on/blob/master/lib/acts_as_taggable_on/tag.rb#L17
      def validates_name_uniqueness?
        false
      end
      validates_uniqueness_to_tenant :name
    end
  end
end

ActsAsTaggableOn::Tag.include(ActsAsTaggableOnTagNameValidationWithActsAsTenant)

Related spec (likely needs tweaks for others to use):

require "rails_helper"

RSpec.describe ActsAsTaggableOn::Tag, type: :model do
  it("acts as tenant with account") { expect(subject.account).to eq(ActsAsTenant.current_tenant) }
  context "with different tenant set" do
    let!(:second_account) { AccountCreatorService.call(name: "Example Org") }

    describe "validates_uniqueness_to_tenant" do
      let!(:tag) { ActsAsTenant.with_tenant(second_account) { ActsAsTaggableOn::Tag.create(name: "test") } }

      it "is valid" do
        ActsAsTenant.with_tenant(ActsAsTenant.current_tenant) do
          expect(ActsAsTaggableOn::Tag.create(name: "test")).to be_valid
        end
      end

      it "is invalid" do
        ActsAsTenant.with_tenant(second_account) do
          expect(ActsAsTaggableOn::Tag.create(name: "test")).to be_invalid
        end
      end
    end
  end
end

@ragingdave
Copy link

@mbleigh any progress on getting this into a new release? It would be hugely helpful

@mbleigh
Copy link
Owner

mbleigh commented Oct 12, 2023

Hey folks, apologies I haven't been the maintainer of this gem for close to a decade so it wouldn't be appropriate for me to decide if it's a good fit.

@seuros I believe you were the last contributor to put a release together, are you still actively working on related things?

@seuros
Copy link
Collaborator

seuros commented Oct 12, 2023

I will try to get some time soon to update this gem.

I avoided adding new features unless they are opt-in.

sigra added a commit to Salesap/acts-as-taggable-on that referenced this pull request Dec 6, 2023
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.

6 participants