Error in multi tenant apps with foreign key

[gushonorato]

Hey,

first of all, thanks for release this great library for us all. I think I've found an issue when using Fun With Flags with a multi tenant app. I've implemented my multi tenant model following this ecto guide: https://hexdocs.pm/ecto/multi-tenancy-with-foreign-keys.html.

The problem is when I try to enable/disable a flag, I got the error:

** (RuntimeError) expected org_id or skip_org_id to be set

Obviously, the error occurs because FWF doesn't know anything about my tenant model. I propose to add a fun_with_flags: true option for every call of Ecto.Repo inside the lib. By doing that, I can figure out which queries are originated from Fun With Flags and execute these queries outside my tenant model, like this:

defmodule MyApp.Repo do
  use Ecto.Repo, otp_app: :my_app

  require Ecto.Query

  def prepare_query(operation, query, opts) do
    cond do
      opts[:skip_org_id] || opts[:schema_migration] || opts[:fun_with_flags] ->
        {query, opts}

      org_id = opts[:org_id] ->
        {Ecto.Query.where(query, org_id: ^org_id), opts}

      true ->
        raise "expected org_id or skip_org_id to be set"
    end
  end
end

I can work on that and send you a pull request. Do you think there's a chance for you to merge it?

[tompave]

Hi, thank you for using the package and for the opening the issue.

I see.

Wouldn't the arguments passed to prepare_query/3 contain the name of the table? You could check if it's a FunWithFlag query with that.

[tompave]

Also, would adding a second repo just for FunWithFlags solve the issue?

[gushonorato]

Hello @tompave

I've tried to extract the table name from the query parameter but doing that it's not very straightforward. There isn't a single field on query struct that I can get the table name. I'd have to search the query struct, which is a bit complex struct, and find all matching "fun_with_flags_toggles". It will add a lot of room for introducing bugs in client apps and It will be hard to test.

Regarding the repository solution, I think it can work, but it'll add unnecessary complexity to all client apps of FWF. I'll have to create another repository, scale the database to work with another connection pool, etc.

I think the solution give by Ecto SQL is the most elegant and gives a better developer experience to people using FWF. To adapt FWF to work like Ecto SQL, we have to adapt this file https://github.com/tompave/fun_with_flags/blob/master/lib/fun_with_flags/store/persistent/ecto.ex to pass [fun_with_flags: true] to all repo calls and adapt or create some small number of tests, which seems a lot simpler to me. Besides the fact that this solution will be available to all future library users. I can submit a pull request if I convinced you that this is the right path to solve this case.

Cheers,
Gustavo

[tompave]

I see.
Ok. Since Ecto SQL supports free-form options, I think your proposal makes sense. It could be handy for other things as well.

I'd be happy to accept a PR for this! Thanks! :-)