CIF-2388 - Rewrite category/product links #712
Conversation
* added Sling rewriter for link transformation * added unit tests
Codecov Report
@@ Coverage Diff @@
## master #712 +/- ##
============================================
+ Coverage 88.77% 88.82% +0.05%
- Complexity 1657 1663 +6
============================================
Files 290 291 +1
Lines 7306 7340 +34
Branches 1073 1078 +5
============================================
+ Hits 6486 6520 +34
Misses 615 615
Partials 205 205
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
* preserve original commerce attributes
mhaack
left a comment
mhaack
left a comment
There was a problem hiding this comment.
As a general though: why is this part of the CIF Core Components project? This relates to authoring functionality and therefor to me belongs to CIF authoring add-on.
| immediate = true, | ||
| service = TransformerFactory.class, | ||
| property = { | ||
| "pipeline.type=ciflinks" |
There was a problem hiding this comment.
Only naming but I would prefer commercelinks.
| property = { | ||
| "pipeline.type=ciflinks" | ||
| }) | ||
| public class CifLinksTransformerFactory implements TransformerFactory { |
There was a problem hiding this comment.
Also here either us Commerce as class name start or remove Cif at all this is already in the commerce package.
Link rewriting is not an authoring functionality (e.g. the Link Checker, etc.). This implementation is in the CIF Core Components because it has a direct dependency to the |
| if (newHref != null) { | ||
| String href = attributes.getValue(ATTR_HREF); | ||
| AttributesImpl attributesImpl; | ||
| if (StringUtils.isNotBlank(href)) { |
There was a problem hiding this comment.
You should not overwrite the href if it exists with a non bank value. Such a href should have precedence over the product/category link because it is an editors choice to set the href.
Also you should check this condition before you call the UrlProvider and (potentially) do a gql request.
Yes you are right with that it must be part of this project. However it still has sort of API, at least a dependency since it works with the data attributes used by the RTE extension. |
|
We defined this contract in the context of the associated content feature #647. |
|
I know, we maybe just should put this somewhere more visible in the docs somewhere. |
* addressing PR review: renamed transformer, refined transformer logic
|
I've renamed the transformer and also improved the transformation logic. |
...com/adobe/cq/commerce/core/components/internal/services/CommerceLinksTransformerFactory.java
Outdated
Show resolved
Hide resolved
* added commerce links transformer to the global pipeline * added OSGi config to enable/disabled the transformer, enabled by default * updated unit test
* fixing IT failure: commerce attributes should be preserved on the links
Description
Create a TransformerFactory in the global rewriter pipeline to rewrite the anchor tags with a data-product-sku or data-categroy-uid attribute.
Related Issue
CIF-2388
How Has This Been Tested?
Manually, JUnit.
Types of changes
Checklist: