activerecord-multi-tenant icon indicating copy to clipboard operation
activerecord-multi-tenant copied to clipboard

Adding a tenant to a record persisted with tenant nil

Open stevenjonescgm opened this issue 5 years ago • 0 comments

I have a case where I persist a record of request input (email processing) before evaluating which tenant it should be assigned to.

When trying to add a tenant id later, persistence silently fails due to https://github.com/citusdata/activerecord-multi-tenant/blob/4512aeeb72504d401f4ed3a869039ee075a2208e/lib/activerecord-multi-tenant/query_rewriter.rb#L227-L233

seeing MultiTenant.with_write_only_mode_enabled? false and a tenant (temporarily) set, so the update has a WHERE clause for the target id when the database record currently has nil.

The tenant is temporarily set by https://github.com/citusdata/activerecord-multi-tenant/blob/4512aeeb72504d401f4ed3a869039ee075a2208e/lib/activerecord-multi-tenant/model_extensions.rb#L96-L110

I considered whether such code should generally use attribute_was like

        around_update -> (record, block) {
          if MultiTenant.current_tenant_id.nil?
            MultiTenant.with(record.attribute_was(partition_key)) { block.call }
          else
            block.call
          end
        }

But that might be vulnerable. perhaps it should only allow changing from untenanted nil to tenanted like

        around_update -> (record, block) {
          record_tenant = record.attribute_was(partition_key)
          if MultiTenant.current_tenant_id.nil? && !record_tenant.nil?
            MultiTenant.with(record_tenant) { block.call }
          else
            block.call
          end
        }

Or perhaps be further restricted to some setting on the model to allow nontenant=>tenant. Perhaps this would be a useful pattern for upgrading a single model/table to tenanted, rather than setting an app-wide MultiTenant.enable_write_only_mode

Thoughts/Suggestions?

stevenjonescgm avatar Dec 02 '20 23:12 stevenjonescgm