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

Add search by relationship fields #8966

Closed
wants to merge 10 commits into from
Closed

Conversation

pnodet
Copy link
Contributor

@pnodet pnodet commented Dec 15, 2023

Allow admin users inside a list page to search by relations text fields

Copy link

codesandbox-ci bot commented Dec 15, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 520e026:

Sandbox Source
@keystone-6/sandbox Configuration

@pnodet
Copy link
Contributor Author

pnodet commented Dec 18, 2023

Hi @dcousens I see some tests failing but some are due to CI setup failure I think, not sure what to do tbh

@dcousens
Copy link
Member

Thanks @pnodet, I'll check that out as soon as I can

@pnodet
Copy link
Contributor Author

pnodet commented Jan 2, 2024

HNY @dcousens thanks for the formatting commit!
I this PR something you are interested in? do you think this will get merged or I better publish a npm fork?

@pnodet
Copy link
Contributor Author

pnodet commented Jan 25, 2024

@dcousens from the 58 tests the failing one seem to be related to tsx, not sure how to fix tbh

> @keystone-6/[email protected] test /home/runner/work/keystone/keystone/examples/testing
> node --loader tsx example-test.ts


node:internal/process/esm_loader:40
      internalBinding('errors').triggerUncaughtException(
                                ^
Error: tsx must be loaded with --import instead of --loader
The --loader flag was deprecated in Node v20.6.0

@dcousens
Copy link
Member

dcousens commented Jan 25, 2024

@pnodet happy new years to you too 💛

I'm not convinced this will be intuitive and could result in unexpected performance costs in some usecases.
Can the user opt-in to the behavior?

@pnodet
Copy link
Contributor Author

pnodet commented Jan 25, 2024

@dcousens

By user you mean end-user (ie: a checkbox with label Activate search by relation fields) or keystone user the developers in the schema for example?

@dcousens
Copy link
Member

dcousens commented Jan 25, 2024

sorry, I mean developer (but, maybe the user too...)

@pnodet
Copy link
Contributor Author

pnodet commented Jan 29, 2024

Sure, for the developer we could have something in the schema / list. Inside the relationship field there is already a isFilterable and isOrderable properties if I'm not mistaken. I'm not an expert and I don't quite remember how we use them but I imagine we could have another field like isSearchable for exemple, defaults to false for backward compatibility.

@pnodet
Copy link
Contributor Author

pnodet commented Mar 14, 2024

Hi @dcousens What do you have in mind? anything I can do to get this PR merged?

@dcousens dcousens changed the title feat: add search by relations fields Add search by relations fields Jul 17, 2024
@dcousens dcousens changed the title Add search by relations fields Add search by Relationship fields Jul 17, 2024
@dcousens dcousens changed the title Add search by Relationship fields Add search by relationship fields Jul 17, 2024
@pnodet
Copy link
Contributor Author

pnodet commented Aug 29, 2024

@dcousens rebased main to keep the branch up to date

@@ -99,6 +98,40 @@ export function useFilter (value: string, list: ListMeta, searchFields: string[]

for (const fieldKey of searchFields) {
const field = list.fields[fieldKey]

// @ts-expect-error TODO: fix fieldMeta type for relationship fields
if (field.fieldMeta?.refSearchFields) {
Copy link
Member

@dcousens dcousens Sep 3, 2024

Choose a reason for hiding this comment

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

@pnodet I merged this into the pre-existing searchFields query building code, less type passing and easier to cross-check

@henribruvier
Copy link

This would be a nice feature !!

@dcousens
Copy link
Member

Replaced by #8966

@dcousens dcousens closed this Nov 19, 2024
@pnodet
Copy link
Contributor Author

pnodet commented Nov 19, 2024

Edit: Replaced by #9401

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

Successfully merging this pull request may close these issues.

4 participants