fix: return OR results for multiple tags in Drive filter (WPB-27126)#21898
fix: return OR results for multiple tags in Drive filter (WPB-27126)#21898cristianoliveira wants to merge 2 commits into
Conversation
…r [WPB-27126] Multi-tag search in the Cells API previously collapsed all selected tags into a single JSON-encoded term (e.g. "sam,bug"), which the backend treated as one literal tag value that no node had, so results came back empty. Emit one Metadata entry per tag with Operation 'Should' and a raw term, mirroring the iOS client (WireMessaging RestAPI.swift) and the existing mimeTypes/creatorIds pattern in the same file. This restores OR semantics: a file matching any selected tag now appears in results. Single-tag search is unaffected. Verified end to end against the fulu backend: two tags on distinct files return both files.
screendriver
left a comment
screendriver
left a comment
There was a problem hiding this comment.
This pull request targets dev, but our development branch is now main. dev is currently 169 commits behind main. Please retarget the pull request to main and update the feature branch from main before continuing the review.
| ...(tags !== undefined && tags.length > 0 | ||
| ? [{Namespace: USER_META_TAGS_NAMESPACE, Term: this.transformTagsToJson(tags)}] | ||
| : []), | ||
| ...(tags?.map(tag => ({ |
There was a problem hiding this comment.
Could we extract the metadata-filter construction from this request literal? This expression currently combines optional-value handling, collection mapping, API serialization, query semantics, and a type assertion. Together with the MIME type and creator filters below, the request is becoming difficult to scan.
Named helpers such as createTagMetadataFilters, createMimeTypeMetadataFilters, and createCreatorMetadataFilters would let this request describe what it contains rather than how every filter is constructed. An explicit helper return type should also remove the need for as const.
There was a problem hiding this comment.
Good call. Extracted createTagMetadataFilters as a module-level helper with an explicit LookupFilterMetaFilter[] return type, which also drops the as const.
Kept the mime/creator extraction out of this PR to keep the diff scoped to WPB-27126 (both were untouched by the bugfix and I'd rather not modify unrelated code paths under a fix commit). Happy to open a follow-up cleanup PR that applies the same pattern to createMimeTypeMetadataFilters and createCreatorMetadataFilters, let me know if you'd prefer that as part of this one instead.
| expect(result).toEqual(mockResponse); | ||
| }); | ||
|
|
||
| it('filters by multiple tags with Should operation (OR) so any selected tag matches (WPB-27126)', async () => { |
There was a problem hiding this comment.
This test does not verify that "any selected tag matches." The API is mocked with an empty response, so it only verifies how the lookup request is serialized.
Either rename this to describe what it actually tests, for example, creates one Should metadata filter for each selected tag, or add an integration/E2E regression test that selects multiple tags and verifies that a file matching either tag is returned.
There was a problem hiding this comment.
I changed the description, the old name was promising something the test can't actually see
The OR part I verified against the real backend (fulu) I added a demo in the PR description
| Deleted: 'Not', | ||
| }, | ||
| Metadata: [{Namespace: 'usermeta-tags', Term: JSON.stringify(tags.join(','))}], | ||
| Metadata: [{Namespace: 'usermeta-tags', Term: 'tag1', Operation: 'Should'}], |
There was a problem hiding this comment.
The pull request description says single-tag search is unaffected. But the actual request changes from:
{
Namespace: USER_META_TAGS_NAMESPACE,
Term: this.transformTagsToJson(tags),
}
to
Namespace: USER_META_TAGS_NAMESPACE,
Term: tag,
Operation: 'Should',
}
That changes both the term representation and the explicit operation.
There was a problem hiding this comment.
You're right, "unaffected" was too loose. What I meant is that a single-tag search still returns only the tagged files with the given tag (See the demo)
|
@screendriver I appreciate the early feedback, but as of now this is a Draft PR, I'll take into account the concerns you raised |
Address review on PR #21898: - Extract tag metadata filter construction into a named module-level helper with an explicit LookupFilterMetaFilter[] return type, dropping the inline as const. - Rename the multi-tag test to describe what it actually verifies (request shape, not backend OR semantics). Mime/creator filter extraction is deferred to a follow-up so this fix stays scoped to WPB-27126.
2 participants
What
Multi-tag search in the Cells API previously collapsed all selected tags into a single JSON-encoded term (e.g.
"sam,bug"), which the backend treated as one literal tag value that no node had so any filter with 2+ tags returned zero results.This PR emits one
Metadataentry per tag withOperation: 'Should'and a raw term, mirroring the iOS client (WireMessaging/Sources/WireMessagingData/WireDrive/NodesAPI/RestAPI.swift:643) and the existingmimeTypes/creatorIdspattern in the same file.Tag filter construction is extracted into a small module-level helper
createTagMetadataFilterswith an explicitLookupFilterMetaFilter[]return type so the request literal reads by intent rather than by construction detail.Why
Restores the OR semantics defined in the PRD: a file matching any of the selected tags is returned.
Behaviour change
Shouldentry with a raw term. Verified against the fulu backend that a single-tag lookup with the new shape returns identical results to the old one.Demo
Screen.Recording.2026-07-17.at.18.53.04.mov