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

[Content Fragment List] Add "All Tag" match ability #2382

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ky940819
Copy link
Contributor

@ky940819 ky940819 commented Nov 23, 2022

Q                       A
Fixed Issues? Fixes #2381
Patch: Bug Fix? No
Minor: New Feature? Yes
Major: Breaking Change? No
Tests Added + Pass? Yes
Documentation Provided Yes (code comments, markdown, and example content update)
Any Dependency Changes? No
License Apache License, Version 2.0

Updates the Content Fragment List component to allow matching either "Any Tags" or "All Tags".
This is accomplished by:

  1. Adding the same dialog option that is found in the List component to the Content Fragment List component dialog.
  2. Updating the CF-list model implementation to use the newly added dialog option to determine if an and or or condition is used for tag matching.

In addition to these changes, the following was also changed:

  1. Removed model property injection where the injected fields could be easily derived from other injected objects and are the values do not need to be retained in the model outside of the initializer method.
  2. Re-resolve resources produced by the QueryBuilder's leaky resource resolver using the non-leaky resource resolver to ensure no reference to the leaky resource resolver persists after it is closed.
  3. Updated Content Fragment List tag predicate group tests to ensure that there are no additional predicates in the actual predicate group vs what was expected. Previously, the test only tested for the presence of the expected predicates, but did not fail if there were additional unexpected predicates.
  4. Updated the Content Fragment List README.MD to add the new ./tagsMatch property to the Edit Dialog Properties section.
  5. Updated the Content Fragment List example content to use the same terminology as the List component to indicate that matching ALL tags is a possible configuration.

Updates the tag predicate testing to ensure that there are no
additional predicates in the actual predicate group that were
not present in the expected predicate group.

----
refs adobe#2381
Updates the Content Fragment List component model so to add the
ability to require all tags to match instead of any tag (i.e. us
`AND` instead of the default `OR`).

This is similar to the List component and makes use of the same
property name (`./tagsMatch`).

Additionally, injected properties are removed when they can easily
be derived from other injected objects and aren't used outside of
the initializer method - and so there is no reason to hold on to them
for the entire lifespan of the object.

Furthermore, resources resolved from the leaky query builder resource
resolver are re-resolved with the non-leaky resource resolver before
being used. This is done to ensure that no reference to the leaky
resource resolver persists after it is closed.

----
refs adobe#2381
Adds an option in the Content Fragment List dialog to select if the
tag matching should be `Any` or `All`. The model will use an `or` or
an `and` condition in the tag predicate based on this choice.

This dialog option is copied directly from the List component dialog.

----
refs adobe#2381
Adds text to the Content Fragment List component example content that
indicates that "all tags" matching is possible. This text is copied
from the List component example content.

----
refs adobe#2381
@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #2382 (9372db7) into main (525cbab) will increase coverage by 0.03%.
The diff coverage is 91.66%.

@@             Coverage Diff              @@
##               main    #2382      +/-   ##
============================================
+ Coverage     87.25%   87.28%   +0.03%     
  Complexity     2518     2518              
============================================
  Files           220      220              
  Lines          6724     6734      +10     
  Branches       1019     1018       -1     
============================================
+ Hits           5867     5878      +11     
  Misses          340      340              
+ Partials        517      516       -1     
Impacted Files Coverage Δ
...ts/models/contentfragment/ContentFragmentList.java 100.00% <ø> (ø)
...ls/v1/contentfragment/ContentFragmentListImpl.java 84.12% <91.66%> (+4.88%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Improves the failure message to communicate exactly why the
predicate did not match.
@sonarcloud
Copy link

sonarcloud bot commented Jan 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

Content fragment list does allow to select items with multiple tags where all are mandatory
1 participant