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

ReferenceOneField accept sort & filter props #8306

Merged
merged 1 commit into from
Oct 26, 2022
Merged

Conversation

nicgirault
Copy link
Contributor

@nicgirault nicgirault commented Oct 25, 2022

Fix #8304

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, everything seems good, thanks for your contribution!

@@ -5,7 +5,7 @@ title: "The ReferenceOneField Component"

# `<ReferenceOneField>`

This field fetches a one-to-one relationship, e.g. the details of a book, when using a foreign key on the distant resource.
This field fetches a one-to-one relationship or the first record of a one-to-many relationship, e.g. the details of a book, when using a foreign key on the distant resource.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not mention this use-case so early in the doc, since it's after all a quite specialized use-case, and we want this first paragraph to introduce the most common use-case of the component only (let's keep things simple 😉 ).

Instead I'd mention this ability later, for instance as a Tip.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, you need to update the ## Properties section too, to document these new props

@@ -18,7 +18,7 @@ This field fetches a one-to-one relationship, e.g. the details of a book, when u
└──────────────┘
```

`<ReferenceOneField>` behaves like `<ReferenceManyField>`: it uses the current `record` (a book in this example) to build a filter for the book details with the foreign key (`book_id`). Then, it uses `dataProvider.getManyReference('book_details', { target: 'book_id', id: book.id })` to fetch the related details, and takes the first one.
`<ReferenceOneField>` behaves like `<ReferenceManyField>`: it uses the current `record` (a book in this example) to build a filter for the book details with the foreign key (`book_id`) and optional additional filters. Then, it uses `dataProvider.getManyReference('book_details', { target: 'book_id', id: book.id })` to fetch the related details, and takes the first one.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, let's not mention the 'optional additional filters' so early, and keep them for later in the doc.

Comment on lines 8 to 11
export interface SortPayload {
field: string;
order: SortOrder;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend avoiding duplication of the SortPayload interface, and using the one from ra-core instead. It does not force the SortOrder to be 'ASC' | 'DESC', but actually we shouldn't force it since it's dependent on the dataProvider.

@slax57
Copy link
Contributor

slax57 commented Oct 25, 2022

Oh and one last thing, since it's a new feature, your PR should target the next branch instead of master. Can you change it?

@nicgirault
Copy link
Contributor Author

@slax57 thanks for your review. Are you satisfied with the latest changes?

@WiXSL WiXSL added the RFR Ready For Review label Oct 25, 2022
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's great, thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to apply sorting and filtering on ReferenceOneField
3 participants