feature-604-did_meta_search: Implement filter on DID search by extending existing functionality #656
Conversation
maany
left a comment
maany
left a comment
There was a problem hiding this comment.
Ciao Luca,
Thanks again for your work on this! The PR logic looks solid and fits well with the overall architecture. I just left a few minor comments, mainly around variable naming.
In Rucio, DID metadata is managed by the metadata plugins. In this PR, you’re introducing DID filters that are used in the list did endpoint. To avoid confusion, could you please rename the occurrences of "meta" so it’s clear we’re referring to DID filters and not DID metadata?
Also, the build is currently failing. If you run npm run build locally, you should be able to reproduce the error and fix it.
Thanks a lot again for your contribution!
.gitignore
Outdated
| # local env files | ||
| .env*.local | ||
| .env.test | ||
| .env.development.local.template |
There was a problem hiding this comment.
We should not ignore the .env.test and .env.local.template files.
| const [scope, setScope] = useState<string | null>(initialScope ?? null); | ||
| const [name, setName] = useState<string | null>(initialName ?? null); | ||
| const [type, setType] = useState<DIDType>(DIDType.DATASET); | ||
| const [metaFilters, setMetaFilters] = useState([{ key: '', operator: '=' as Operator, value: '' }]); |
There was a problem hiding this comment.
Could you not use the word "meta". This is technically not DID Metadata in Rucio which is managed by Metadata plugins.
You can call these didFilters for example?
|
|
||
| const scopeInputRef = useRef<HTMLInputElement>(null); | ||
| const nameInputRef = useRef<HTMLInputElement>(null); | ||
| const metaKeyRefs = useRef<(HTMLInputElement | null)[]>([]); |
| metaValueRefs.current[i]?.focus(); | ||
| return false; | ||
| } | ||
| // if (key.includes(SCOPE_DELIMITER)) { |
There was a problem hiding this comment.
please remove commented out code if it is not needed
src/lib/core/entity/rucio.ts
Outdated
| }; | ||
|
|
||
| // Metadata filter operators | ||
| export type MetaOperator = '=' | '!=' | '>' | '<' | '>=' | '<='; |
There was a problem hiding this comment.
could be called: DIDFilterOperator
src/pages/api/feature/list-dids.ts
Outdated
| return; | ||
| } | ||
| const { query, type } = req.query; | ||
| const { query, type, meta } = req.query; |
LucaPacioselli
force-pushed
the
feature-604-did_meta_search
branch
from
ee01481 to
e365660
Compare
|
Ciao @maany, I’ve pushed a few commits to fix the build check failure, revert the Could you please have a look and review the updated state? |
maany
left a comment
maany
left a comment
There was a problem hiding this comment.
Looks great! Thanks @LucaPacioselli
2 participants
Description:
This PR implements a filter on the DID search functionality, specifically targeting the
/dids/{scope}/dids/searchendpoint. The new feature was developed by extending the pre-existing DID search logic, allowing users to refine their queries and obtain more targeted results.For testing purposes, I also modified the
.gitignorefile. If these changes are not needed, they can be discarded by the reviewer of the PR.