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

Allow admin-instance resource config #34

Closed
tfwright opened this issue May 7, 2023 · 5 comments
Closed

Allow admin-instance resource config #34

tfwright opened this issue May 7, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@tfwright
Copy link
Owner

tfwright commented May 7, 2023

See #33

Problems:

  1. right now default config must be defined at the app level, which makes it awkward to use a different set of default configs for different admin UIs (if more than 1 is desired for whatever reason).

  2. since overrides are likely to be desired for most resources, the router config is large/noisy (see this thread)

Proposed solution: deprecate app level default config, and add support for passing a module, which specifies all configuration, including resources:

defmodule MyApp.AdminA do
  use LiveAdmin, 
    ecto_repo: MyApp.RepoA, 
    create_with: {MyApp.AdminContext, :default_create, []},
    update_with: :schema_function_name,
    resources: :get_resources

    def get_resources, do: [MyApp.SchemaA]
end

defmodule MyApp.AdminB do
  use LiveAdmin, 
    ecto_repo: MyApp.RepoB,
    resources: [MyApp.SchemaB]
end

Alternatives:

  • extract resource level config into separate macro that would be called as part of a block, similar to how Phoenix routes work:
live_admin "/admin_url", title: "MyAdmin" do
  admin_resource "/a", MyApp.SchemaA, create_with: :create
  admin_resource "/b", MyApp.SchemaB, create_with: :something
end

It might be that these are two separate problems with config that need to be addressed separately.

@tejpochiraju
Copy link

@tfwright - thanks a lot for your work here. Just discovered live_admin and it fit our needs perfectly. Specifically, I stumbled upon this issue when looking for multi-repo support. However, I realised, accidentally, that this is already supported out of the box.

I am documenting our use case and how we got it working so someone else looking for the same thing doesn't spend too long on this.
We have 3 apps each with a separate repo. Let's call them AppA.Repo, AppB.Repo and AppC.Repo. Of these, AppA is the parent app to which AppB and AppC are dependencies.

To get live_admin working on all of these with their schemas, all we had to do was the following:

In AppA's config.exs

config :app_a,
  ecto_repos: [AppC.Repo, AppB.Repo, AppA.Repo]

The reason AppA.Repo is listed last is because some of its migrations depend on AppB and AppC and this way migrations run in the correct order when you run mix ecto.migrate.

Then, in AppAWeb's router.ex,

live_admin("/admin",
     resources: [
       AppA.SomeSchema,
       AppB.SomeSchema,
       AppC.SomeSchema
     ]
)

This works because (and I didn't know this previously), AppA.Repo.all(%AppB.SomeSchema{}) just works as long as AppB.Repo is included in ecto_repos for AppA.

Ignore if this is known behaviour but it certainly surprised me and Kaffy's autodetection only picks up AppA schemas.

@tfwright
Copy link
Owner Author

Definitely not expected behavior! I haven't tried to use multiple Ecto repos myself, so I'm not sure, but LiveAdmin currently executes all queries in the context of whatever repo is set in its config. I am surprised that it would work with any other repos.

@tejpochiraju
Copy link

Here's a simple example that demos this behaviour: https://github.com/tejpochiraju/multi_repo_live_admin

@tfwright
Copy link
Owner Author

tfwright commented May 12, 2023

Hm, just a guess but I think the key implementation detail there is that the same db is being used. I think that means that even though LiveAdmin is still using the "wrong" repo when querying whatever schemas are "supposed" to be on the other repo, since the db is the same the queries end up being directed to the same place. My expectation would be if you alter the AppB.Repo config to point at a different file with the AppB schema tables, and remove those tables from AppA.Repo file, then AppB schema resources will no longer work in LiveAdmin

@tejpochiraju
Copy link

tejpochiraju commented May 12, 2023

You are right. With separate DBs (SQLite3 or Postgres), I get the following error:

[error] GenServer #PID<0.851.0> terminating
** (Postgrex.Error) ERROR 42P01 (undefined_table) relation "users" does not exist

Nevertheless, this is still useful behaviour if possibly undocumented on the Ecto side of things (I can't seem to find a reference). Before I stumbled upon this approach, I was considering my options.

  • You said it was "awkward" to configure multiple admin UIs. Does this mean it's possible, how would one go about this?
  • Your suggested approach of passing an MFA during the router-level config would work.
  • Replace calls to repo/0 with a new repo/1 that infers the right Repo from the resource being queried. This seemed the simplest to me. What are the pitfalls here?

EDIT

  • Confirmed that setting ecto_repos: [AppA.Repo] also works. I think this confirms your theory that once you have a Repo connection, the second schema query works if the table is found the current context (DB + prefix).
  • It follows that this will also fail if AppA was configured with a user that does not have access to AppB tables - even if pointing to the same DB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants