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 a target prop in ReferenceField #2022

Closed
christiaanwesterbeek opened this issue Jul 17, 2018 · 8 comments
Closed

Allow a target prop in ReferenceField #2022

christiaanwesterbeek opened this issue Jul 17, 2018 · 8 comments

Comments

@christiaanwesterbeek
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Consider

const CommentShow = props => (
    <Show {...props}>
        <SimpleShowLayout>
            <TextField source="id" />
            <ReferenceField source="post_id" reference="posts">
                <TextField source="title" />
            </ReferenceField>
            <TextField source="author.name" />
            <DateField source="created_at" />
            <TextField source="body" />
        </SimpleShowLayout>
    </Show>
);

In some database all primary keys are named id instead of [table_name]_id. I'm not saying having id as pk's is recommendable, but it is used quite often.

Describe the solution you'd like
How about allowing a target in ReferenceField? Just like ReferenceManyField is accepting both source and target.

const CommentShow = props => (
    <Show {...props}>
        <SimpleShowLayout>
            <TextField source="id" />
            <ReferenceField source="post_id" target="id" reference="posts">
                                             ^^^^^^^^^^^
                <TextField source="title" />
            </ReferenceField>
            <TextField source="author.name" />
            <DateField source="created_at" />
            <TextField source="body" />
        </SimpleShowLayout>
    </Show>
);

Describe alternatives you've considered
/

Additional context
/

@fzaninotto
Copy link
Member

React-admin requires that the records it manipulates have an id attribute. If your backend doesn't provide one, it's the job of the dataProvider to do the translation.

Adding the target prop that you ask would imply leaking this "adapter" logic outside of the dataProvider. We won't do it.

@christiaanwesterbeek
Copy link
Contributor Author

No problem. I hadn't considered translating in the dataProvider. Thanks for the suggestion.

Btw, you guys are awesome! I had this Datagrid with overly nested ReferenceManyFields and ReferenceFields. And it worked flawlessly. Really amazing what you are building! Again, 👏👏👏.

@fzaninotto
Copy link
Member

Thanks a lot, we appreciate!

@aramando
Copy link
Contributor

aramando commented Apr 26, 2019

@fzaninotto @aramando I was looking into adding this functionality myself and submitting a PR - I'm glad I checked here first!

But with regard to the reason given here for not supporting a target prop on the ReferenceField component, why does this not also apply to the ReferenceManyField component, which does accept a target prop?

I also don't understand why the requirement for all records to be primarily-keyed by an id property precludes the use case of fetching a record that is related to the current record, just not by its primary key? What if one wishes to reference a related record by a foreign key that exists on the target, e.g. source="id" and target="user_id"?

@nickgrealy
Copy link

React-admin requires that the records it manipulates have an id attribute. If your backend doesn't provide one, it's the job of the dataProvider to do the translation.

Adding the target prop that you ask would imply leaking this "adapter" logic outside of the dataProvider. We won't do it.

Sorry about coming in late to the party - one question for @fzaninotto:

If you don't support joins on other properties (i.e. target), then why does ReferenceManyField have a target field? (Are there plans to remove it?)

export const ReferenceManyField: FC<ReferenceManyFieldProps> = props => {
const {
basePath,
children,
filter,
page = 1,
perPage,
record,
reference,
resource,
sort,
source,
target,
} = props;

@fzaninotto
Copy link
Member

No, there are no plans to remove it from ReferenceManyField.

One of the reasons why ReferenceField requires that you use the id field is that we do an optim in datagrids having one ReferenceField per row to replace many getOne with one getMany... and this only works with the id field.

We don't have (or plan to have) such optims in ReferenceManyField.

@lauri865
Copy link

lauri865 commented Jul 6, 2022

@fzaninotto, I'm gitting the same constraints as many others, trying to add a ReferenceField to Datagrid in order to pull in data from another table on composite id or just any foreign key that's not id. I'd be most fine adding some translation logic in my dataprovider to do so, but lacking the ability to send any options to the dataprovider from the ReferenceField component.

useGetManyAggregate does pass along meta to the dataprovider, but there's no way to send em through via ReferenceField component + useReference hook. I did an implementation in my codebase, but had to import 400 lines of code just to add 1 prop (=meta) to ReferenceField + the same prop to useReference.

A big win would be the ability to set meta={...} props on ReferenceField that would be sent along to the dataprovider.

@fzaninotto
Copy link
Member

If ReferenceField doesn't work in your case, use one of the the data provider hooks (like useGetManyReference together with useList to create a ListContext and use the Datagrid as you like.

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

No branches or pull requests

5 participants