Skip to content

feat(filters): Add new tag filtering modes AND, EXACT & NOT (@TristanMarion) #6388

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

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

TristanMarion
Copy link
Contributor

@TristanMarion TristanMarion commented Mar 21, 2025

Description

This PR implements issue #4239, which adds a new toggle feature to switch between OR, AND and EXACT logic modes for tag filtering in the account page :

  • OR: any selected tag must match
  • AND: all selected tags must match
  • EXACT: exactly the selected tags must match

Expected behaviors :

  • AND mode
    • none - Results without tags
    • tags - Results tags should include the selected tag/tags
    • none and tag - Nothing
    • all (or no tag) - Nothing
  • OR mode
    • none - Results without tags
    • tags - Result tags should include any of the selected tags
    • none and tag - Result without tags plus result tags should include any of the selected tags
    • all (or no tag) - Everything
  • EXACT mode
    • none - Results without tags
    • tags - Results tags should exactly match the selected tags
    • none and tag - Nothing
    • all (or no tag) - Nothing

Checks

  • Check if any open issues are related to this PR; if so, be sure to tag them below.
  • Make sure the PR title follows the Conventional Commits standard. (https://www.conventionalcommits.org for more info)
  • Make sure to include your GitHub username prefixed with @ inside parentheses at the end of the PR title.

Closes #4239

Several checks in video :

Dataset used

We will mainly have tag 1, 2 and 3 for our tests, the list of test tags is : 1, 2, 3, 1,2, 1,3, 2,3, 1,2,3

Screen.Recording.2025-03-23.at.14.52.55.mov

OR mode with tag 1 : we only have 1, 1,2, 1,3 & 1,2,3

Screen.Recording.2025-03-23.at.14.53.59.mov

OR mode with tags 1 & 2 : we only have 1, 2, 1,2, 2,3, 1,3 & 1,2,3

Screen.Recording.2025-03-23.at.14.54.15.mov

AND mode with tags 1 & 2 : we only have 1,2 & 1,2,3

Screen.Recording.2025-03-23.at.14.54.29.mov

EXACT mode with tags 1 & 2 : we only have 1,2

Screen.Recording.2025-03-23.at.14.54.43.mov

EXACT mode with tag 1 : we only have 1

Screen.Recording.2025-03-23.at.14.54.53.mov

AND mode with tag 1 (same behavior as OR with tag 1) : we only have 1, 1,2, 1,3 & 1,2,3

Screen.Recording.2025-03-23.at.14.55.03.mov

@monkeytypegeorge monkeytypegeorge added frontend User interface or web stuff packages Changes in local packages labels Mar 21, 2025
Copy link
Contributor

Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes.

@github-actions github-actions bot added waiting for update Pull requests or issues that require changes/comments before continuing and removed waiting for update Pull requests or issues that require changes/comments before continuing labels Mar 21, 2025
@monkeytypegeorge monkeytypegeorge added the backend Server stuff label Mar 21, 2025
@TristanMarion TristanMarion force-pushed the feature/tags-filtering-and-mode branch from b71dc32 to c5fea36 Compare March 21, 2025 14:26
@Miodec
Copy link
Member

Miodec commented Mar 21, 2025

Nice! How about we also add an 'exact' mode, where result tags have to exactly match your selection?

Comment on lines +478 to +480
if (enabledTagIds.length === 0) {
// No tag filters enabled, show everything
tagHide = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since "none" does not get pushed to enabledTagIds, does this mean when only "none" is enabled, all results (with & without tags) will be shown?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, TBH I did not know how to handle this edge case, do you think we should display only the tests with no tags at all ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, behavior-wise, only results with no tags should be shown imo.
Implementation-wise, I don't think these 3 lines are needed here (I did not test this however)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this is already done in the existing code and this block won't be reachable if the result has no tag so I will remove this part and ensure we have the correct behavior anyway

Comment on lines +483 to +485
const resultHasAllTags = enabledTagIds.every((tagId) =>
result.tags?.includes(tagId)
);

This comment was marked as resolved.

@TristanMarion
Copy link
Contributor Author

Thanks for the feedbacks 😊 indeed this was intended to have a non exclusive AND (to match at least the tags selected but not all the tags) but if you think that it should be an exact match I can perform the change 👍
Let me know what you prefer and I’ll update it 👌

@NadAlaba
Copy link
Contributor

Thanks for the feedbacks 😊 indeed this was intended to have a non exclusive AND (to match at least the tags selected but not all the tags)

My bad, I see the use case now. However, I think an "exact" mode would be equally useful.
This is just my free opinion as a user, it is up to miodec if he prefers 3 modes or 2.

@Miodec
Copy link
Member

Miodec commented Mar 22, 2025

Lets have 3 modes, or, non exclusive and and exclusive and (exact mode). Click the button to cycle through them.

@TristanMarion
Copy link
Contributor Author

Pretty clear, I’ll handle the changes to match the expected behaviour. Makes sense to have the three modes 👌

@TristanMarion TristanMarion changed the title feat(filters): Add new tag filtering mode AND (@TristanMarion) feat(filters): Add new tag filtering modes AND & EXACT (@TristanMarion) Mar 23, 2025
@TristanMarion TristanMarion force-pushed the feature/tags-filtering-and-mode branch 2 times, most recently from 81ab42a to 407c003 Compare March 23, 2025 17:00
@TristanMarion
Copy link
Contributor Author

I've reorder the imports to reduce the number of line changed 👍 (and to keep the same import order as before too 😅 )

} else if (
ResultFilters.getFilter("tags", "none") &&
result.tags.length === 0
) {
// Special case: "none" tag is enabled and result has no tags
tagHide = false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this block will never be reached (it is in an else block that gets executed when result.tags.length > 0), I wonder why the linter didn't complain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I will remove it, I'll test what happens with the none tag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handled in 70c1f31 👍
Indeed the code is already handling this case earlier 😅

Comment on lines 520 to 514
} else if (
ResultFilters.getFilter("tags", "none") &&
result.tags.length === 0
) {
// Special case: "none" tag is enabled and result has no tags
tagHide = false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this block will never be reached

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handled in 70c1f31 👍

@TristanMarion TristanMarion force-pushed the feature/tags-filtering-and-mode branch from 70c1f31 to 967d66d Compare March 25, 2025 15:12
@@ -33,6 +33,8 @@ export function mergeWithDefaultFilters(
merged[groupKey] = id;
} else if (groupKey === "name") {
merged[groupKey] = filters[groupKey] ?? defaultResultFilters[groupKey];
} else if (groupKey === "tagsFilterMode") {
Copy link
Contributor Author

@TristanMarion TristanMarion Mar 25, 2025

Choose a reason for hiding this comment

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

I can merge this condition with the previous one as they contain the same code, let me know what you prefer

@TristanMarion TristanMarion marked this pull request as draft March 26, 2025 08:57
@TristanMarion TristanMarion changed the title feat(filters): Add new tag filtering modes AND & EXACT (@TristanMarion) feat(filters): Add new tag filtering modes AND, EXACT & NOT (@TristanMarion) Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Server stuff frontend User interface or web stuff packages Changes in local packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tags filter logic mode: "or" and "and"
4 participants