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

Support alternate primary keys / remove assumption of :id key on struct #88

Closed
JohnnyCurran opened this issue Jan 18, 2024 · 6 comments · Fixed by #89
Closed

Support alternate primary keys / remove assumption of :id key on struct #88

JohnnyCurran opened this issue Jan 18, 2024 · 6 comments · Fixed by #89
Labels
bug Something isn't working

Comments

@JohnnyCurran
Copy link

Describe the bug
If a schema does not have an :id key, it cannot be rendered with LiveAdmin due to the assumption on line 109 at

data-record-id={record.id}

To Reproduce
Steps to reproduce the behavior:
0. Create a schema without an :id key

  1. Go to LiveAdmin / try to list out the resource
  2. See error

Expected behavior
Resource should show correctly

Screenshots
If applicable, add screenshots to help explain your problem.

Environment:

  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Additional context
Conversation starts here https://elixirforum.com/t/liveadmin-phoenix-admin-ui-built-on-liveview/46421/33?u=johnnycurran

@JohnnyCurran JohnnyCurran added the bug Something isn't working label Jan 18, 2024
@tfwright
Copy link
Owner

Hi @JohnnyCurran thanks for bringing this to my attention. Can you share any more info about your schema? Are you using a different naming scheme for your primary keys?

@JohnnyCurran
Copy link
Author

JohnnyCurran commented Jan 18, 2024

@tfwright The data comes from an external API and we use it basically as a cache so we aren't doing http calls to fetch it.

This is the only schema we have named this way. Table was created with primary_key: false:

create table(:business_redacted, primary_key: false)

and then the schema is decorated with:

@primary_key {:unitid, :integer, autogenerate: false}

For the time being, I've added:

field :id, :string, virtual: true

to our schema so that live_admin works

EDIT: Live_admin works for viewing, but not for editing with the addition of the virtual field. I'll keep digging to see if this is related to the :id issue

Edit 2: Looks like we have an {:array, :map} field on our struct which is incompatible with some of the input options. Unrelated to this issue and I hid it from the form editor and things are running smoothly again

@tfwright
Copy link
Owner

OK yeah I think LiveAdmin just needs to be made aware of primary keys with alternate names. Probably this should be doable without requiring additional config by reflecting on the schema struct, but I'll take a closer look at let you know.

@tfwright
Copy link
Owner

tfwright commented Jan 21, 2024

@JohnnyCurran if you want to try out the changes in #89 they should allow you to use your custom primary key without any additional config. Let me know if you run into any issues!

@JohnnyCurran
Copy link
Author

@tfwright I can confirm this allowed me to use it without defining a virtual :id field.

To note: I did need to derive Phoenix.Param on the struct which is, of course not an issue with the lib, but would be a helpful addition to a section of the docs if there's a section on non-:id primary keys in the future

@tfwright
Copy link
Owner

@JohnnyCurran thanks for reminding me of the issue with Phoenix.Param I do think that might be something that LiveAdmin should address although you are right that generally one would expect library authors to already be doing that. But Phoenix/Ecto itself doesn't enforce use of to_param so it's possible they wouldn't otherwise need to, and there's no technical reason for LiveAdmin to rely on it, just a convenience, so I think I will take it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants