-
Notifications
You must be signed in to change notification settings - Fork 79
CIF-1974 - Core Components: navigation - use UID for category queries #528
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
Conversation
* added category UID support for navigation component
* fixed test failure
Codecov Report
@@ Coverage Diff @@
## master #528 +/- ##
============================================
- Coverage 87.47% 87.41% -0.06%
- Complexity 1267 1268 +1
============================================
Files 232 232
Lines 5955 5968 +13
Branches 874 878 +4
============================================
+ Hits 5209 5217 +8
- Misses 582 583 +1
- Partials 164 168 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
# Conflicts: # examples/bundle/src/main/java/com/adobe/cq/commerce/core/examples/servlets/GraphqlServlet.java
* added support for URLProvider driven UID rendering in URLs
* fixed test failure
...java/com/adobe/cq/commerce/core/components/internal/models/v1/navigation/NavigationImpl.java
Outdated
Show resolved
Hide resolved
...java/com/adobe/cq/commerce/core/components/internal/models/v1/navigation/NavigationImpl.java
Outdated
Show resolved
Hide resolved
# Conflicts: # examples/bundle/src/main/java/com/adobe/cq/commerce/core/examples/servlets/GraphqlServlet.java
* fixed build failure
|
|
||
| private static final Logger LOGGER = LoggerFactory.getLogger(GraphQLCategoryProvider.class); | ||
| private static final Function<CategoryTreeQuery, CategoryTreeQuery> CATEGORIES_QUERY = q -> q.id().name().urlPath().position(); | ||
| private static final Function<CategoryTreeQuery, CategoryTreeQuery> CATEGORIES_QUERY = q -> q.id().uid().name().urlPath().position(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make this backwards compatible, the uid should be included in the query only if UID support is enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this break backwards compatibility? Next version of CIF components will require 2.4.2 and you can always query for the UID. The only place where we have to be careful is when we query by UID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@laurentiumagureanu , could you please clarify? I also think that in Magento 2.4.2 the category uid will be always available.
You can find similar code in master, like here: https://github.com/adobe/aem-core-cif-components/blob/master/bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/models/v1/breadcrumb/BreadcrumbRetriever.java#L191
Related Issue
CIF-1974
Motivation and Context
Manually.
Types of changes
Checklist: