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.

Sign up for GitHub

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

Jump to bottom

CIF-2188 - Expose HTML id attribute in component edit dialogs #639

Merged
merged 5 commits into from
Jul 30, 2021
Merged

CIF-2188 - Expose HTML id attribute in component edit dialogs #639

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5144ae1
CIF-2188 - Expose HTML id attribute in component edit dialogs
LSantha Jul 26, 2021
2e93482
CIF-2188 - Expose HTML id attribute in component edit dialogs
LSantha Jul 26, 2021
d78d820
Merge branch 'master' into CIF-2188
LSantha Jul 28, 2021
2cad8b9
CIF-2188 - Expose HTML id attribute in component edit dialogs
LSantha Jul 28, 2021
9788641
Merge branch 'master' into CIF-2188
laurentiumagureanu Jul 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions ...ain/java/com/adobe/cq/commerce/core/components/internal/datalayer/DataLayerComponent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,28 @@

import javax.inject.Inject;

import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ValueMap;
import org.apache.sling.caconfig.ConfigurationBuilder;
import org.apache.sling.models.annotations.injectorspecific.Self;

import com.adobe.cq.commerce.core.components.datalayer.CategoryData;
import com.adobe.cq.wcm.core.components.models.Component;
import com.adobe.cq.wcm.core.components.models.datalayer.AssetData;
import com.adobe.cq.wcm.core.components.models.datalayer.ComponentData;
import com.adobe.cq.wcm.core.components.util.ComponentUtils;
import com.fasterxml.jackson.annotation.JsonIgnore;

public abstract class DataLayerComponent {
public static final String ID_SEPARATOR = "-";
public static final String ID_SEPARATOR = ComponentUtils.ID_SEPARATOR;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why redefining the const instead of using ComponentUtils.ID_SEPARATOR directly via static import?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The ID_SEPARATOR is used at more other places in the codebe in sub-classes of DataLayerComponnet. If you define it here you don't need any static import in many other files. In fact the constant was defined earlier, I've just aligned the definition with ComponentUtils.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know the constant was defined earlier and it was wrong. So instead of having the same constant twice I would prefer to remove it and only use ComponentUtils.ID_SEPARATOR. The static import should not hurt.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 - this is more explicit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've inlined the ID_SEPARATOR constant.


@Inject
protected Resource resource;

@Self
private Component wcmComponent;

private String id;
private Boolean dataLayerEnabled;
private ComponentData componentData;
Expand Down Expand Up @@ -67,10 +72,14 @@ protected ComponentData getComponentData() {
}

protected String generateId() {
String resourceType = resource.getResourceType();
String prefix = StringUtils.substringAfterLast(resourceType, "/");
String path = resource.getPath();
return StringUtils.join(prefix, ID_SEPARATOR, StringUtils.substring(DigestUtils.sha256Hex(path), 0, 10));
if (wcmComponent == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

how can wcmComponent be null?

We only have 2 models using an optional default injection strategy (button v1 and v2). For all others the wcmComponent is required.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

DataLayerComponent is also extended by classes which are not Sling models, we handle that case here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add this detail to the javadoc of the class while you are on it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What detail?

Copy link
Contributor

Choose a reason for hiding this comment

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

That this abstract class may be extended by classes that are annotated as sling models and others that are not.

String resourceType = resource.getResourceType();
String prefix = StringUtils.substringAfterLast(resourceType, "/");
String path = resource.getPath();
return ComponentUtils.generateId(prefix, path);
} else {
return wcmComponent.getId();
}
}

public String getId() {
Expand Down
5 changes: 3 additions & 2 deletions ...main/java/com/adobe/cq/commerce/core/components/internal/datalayer/DataLayerListItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
******************************************************************************/
package com.adobe.cq.commerce.core.components.internal.datalayer;

import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.resource.Resource;

import com.adobe.cq.wcm.core.components.util.ComponentUtils;

public abstract class DataLayerListItem extends DataLayerComponent {

public static final String ITEM_ID_PREFIX = "item";
Expand All @@ -35,6 +36,6 @@ protected String getIdentifier() {
@Override
protected String generateId() {
String prefix = StringUtils.join(parentId, ID_SEPARATOR, ITEM_ID_PREFIX);
return StringUtils.join(prefix, ID_SEPARATOR, StringUtils.substring(DigestUtils.sha256Hex(getIdentifier()), 0, 10));
return ComponentUtils.generateId(prefix, getIdentifier());
}
}
4 changes: 2 additions & 2 deletions .../com/adobe/cq/commerce/core/components/internal/models/v1/common/ProductListItemImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import javax.annotation.Nullable;

import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.SlingHttpServletRequest;

Expand All @@ -30,6 +29,7 @@
import com.adobe.cq.commerce.core.components.services.urls.UrlProvider;
import com.adobe.cq.commerce.core.components.services.urls.UrlProvider.ParamsBuilder;
import com.adobe.cq.wcm.core.components.models.datalayer.ComponentData;
import com.adobe.cq.wcm.core.components.util.ComponentUtils;
import com.day.cq.wcm.api.Page;
import com.fasterxml.jackson.annotation.JsonIgnore;

Expand Down Expand Up @@ -150,7 +150,7 @@ protected ComponentData getComponentData() {
@Override
protected String generateId() {
String prefix = StringUtils.join(parentId, ID_SEPARATOR, ITEM_ID_PREFIX);
return StringUtils.join(prefix, ID_SEPARATOR, StringUtils.substring(DigestUtils.sha256Hex(getSKU()), 0, 10));
return ComponentUtils.generateId(prefix, getSKU());
}

@Override
Expand Down
5 changes: 5 additions & 0 deletions ...merce/core/components/internal/models/v1/contentfragment/CommerceContentFragmentImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,11 @@ public String getEditorJSON() {
return contentFragment.getEditorJSON();
}

@Override
public String getId() {
return contentFragment.getId();
}

static class EmptyContentFragment implements ContentFragment {
/**
* The empty return value means that the model does not contain a valid content fragment.
Expand Down
13 changes: 12 additions & 1 deletion ...in/java/com/adobe/cq/commerce/core/components/internal/models/v1/product/ProductImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@
import com.adobe.cq.commerce.magento.graphql.SimpleProduct;
import com.adobe.cq.commerce.magento.graphql.VirtualProduct;
import com.adobe.cq.sightly.SightlyWCMMode;
import com.adobe.cq.wcm.core.components.models.Component;
import com.adobe.cq.wcm.core.components.models.datalayer.AssetData;
import com.adobe.cq.wcm.core.components.models.datalayer.ComponentData;
import com.adobe.cq.wcm.core.components.util.ComponentUtils;
import com.adobe.cq.wcm.launches.utils.LaunchUtils;
import com.day.cq.commons.Externalizer;
import com.day.cq.wcm.api.Page;
Expand Down Expand Up @@ -428,7 +430,16 @@ public ComponentData getComponentData() {

@Override
protected String generateId() {
return StringUtils.join("product", ID_SEPARATOR, StringUtils.substring(DigestUtils.sha256Hex(getSku()), 0, 10));
String id = super.generateId();
if (StringUtils.isNotBlank(properties.get(Component.PN_ID, String.class))) {
// if available use the id provided by the user
return id;
} else {
// otherwise include the product SKU in the id
String prefix = StringUtils.substringBefore(id, ID_SEPARATOR);
String suffix = StringUtils.substringAfterLast(id, ID_SEPARATOR) + getSku();
return ComponentUtils.generateId(prefix, suffix);
}
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...a/com/adobe/cq/commerce/core/components/internal/models/v1/page/PageMetadataImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public void testPageMetadataModelOnProductPage() throws Exception {
// Asserts that the right product resource is used when PageMetadataImpl adapts the request to the Product component
ComponentData data = productModel.getData();
assertEquals("venia/components/commerce/product", data.getType());
assertEquals("product-8309e8957e", data.getId());
assertEquals("product-3944cc709b", data.getId());
}

@Test
Expand Down
22 changes: 19 additions & 3 deletions ...ava/com/adobe/cq/commerce/core/components/internal/models/v1/product/ProductImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import io.wcm.testing.mock.aem.junit.AemContextCallback;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -118,6 +119,7 @@ private static AemContext createContext(String contentPath) {

private static final String PAGE = "/content/pageA";
private static final String PRODUCT = "/content/pageA/jcr:content/root/responsivegrid/product";
private static final String PRODUCT_WITH_ID = "/content/pageA/jcr:content/root/responsivegrid/productwithid";

private Resource productResource;
private Resource pageResource;
Expand Down Expand Up @@ -325,7 +327,7 @@ public void testSafeDescriptionWithNull() {
SimpleProduct product = mock(SimpleProduct.class, RETURNS_DEEP_STUBS);
when(product.getDescription()).thenReturn(null);
Whitebox.setInternalState(productModel.getProductRetriever(), "product", Optional.of(product));
Assert.assertNull(productModel.getDescription());
assertNull(productModel.getDescription());
}

@Test
Expand All @@ -338,7 +340,7 @@ public void testSafeDescriptionHtmlNull() {

Whitebox.setInternalState(productModel.getProductRetriever(), "product", Optional.of(product));

Assert.assertNull(productModel.getDescription());
assertNull(productModel.getDescription());
}

@Test
Expand Down Expand Up @@ -486,7 +488,7 @@ public void testProductNoGraphqlClient() {
Assert.assertFalse("Product is not configurable", productModel.isConfigurable());
Assert.assertFalse("Product is not a grouped product", productModel.isGroupedProduct());
Assert.assertFalse("Product is not virtual", productModel.isVirtualProduct());
Assert.assertNull("The product retriever is not created", productModel.getProductRetriever());
assertNull("The product retriever is not created", productModel.getProductRetriever());
}

@Test
Expand Down Expand Up @@ -552,4 +554,18 @@ public void testStorefrontContextRender() throws IOException {
String jsonResult = productModel.getStorefrontContext().getJson();
assertEquals(mapper.readTree(expected), mapper.readTree(jsonResult));
}

@Test
public void testManualHtmlId() throws IOException {
context.currentResource(PRODUCT_WITH_ID);
context.request().setServletPath(PRODUCT_WITH_ID + ".beaumont-summit-kit.html");
productResource = context.resourceResolver().getResource(PRODUCT_WITH_ID);
SlingBindings slingBindings = (SlingBindings) context.request().getAttribute(SlingBindings.class.getName());
slingBindings.setResource(productResource);
slingBindings.put(WCMBindingsConstants.NAME_PROPERTIES, productResource.getValueMap());

productModel = context.request().adaptTo(ProductImpl.class);

assertEquals("custom-id", productModel.getId());
}
}
4 changes: 4 additions & 0 deletions bundles/core/src/test/resources/context/jcr-content.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@
"product": {
"sling:resourceType": "venia/components/commerce/product"
},
"productwithid": {
"sling:resourceType": "venia/components/commerce/product",
"id": "custom-id"
},
"productlist": {
"loadClientPrice": true,
"showImage": true,
Expand Down
2 changes: 1 addition & 1 deletion bundles/core/src/test/resources/results/result-datalayer-product-component.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"product-8309e8957e": {
"product-a8f26bcdca": {
"xdm:SKU": "MJ01",
"xdm:listPrice": 58.0,
"xdm:categories": [
Expand Down
2 changes: 1 addition & 1 deletion it/http/src/test/resources/datalayer/chaz-kangeroo-hoodie-product.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"product-500741b9f1": {
"product-4489fda6a6": {
"xdm:SKU": "MH01",
"xdm:listPrice": 52.0,
"xdm:assets": [
Expand Down
7 changes: 6 additions & 1 deletion ...t/jcr_root/apps/core/cif/components/commerce/carousel/v1/carousel/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<title jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/textfield"
fieldDescription="Optional title text. Leave empty to hide the title."
fieldLabel="Title"
fieldLabel="Title"
name="./jcr:title"/>
<titleType granite:class="core-title-sizes"
jcr:primaryType="nt:unstructured"
Expand All @@ -28,6 +28,11 @@
jcr:primaryType="nt:unstructured"
sling:resourceType="core/wcm/components/commons/datasources/allowedheadingelements/v1"/>
</titleType>
<id jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/textfield"
fieldDescription="HTML ID attribute to apply to the component."
fieldLabel="ID"
name="./id"/>
</items>
</column>
</items>
Expand Down
3 changes: 2 additions & 1 deletion ...core/cif/components/commerce/categorycarousel/v1/categorycarousel/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
value="Shop by category" />
<content granite:class="cmp-cif-categorycarousel__editor"
jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/multifield"
sling:resourceType="granite/ui/components/coral/foundation/form/multifield"
sling:orderBefore="id"
composite="{Boolean}true"
fieldLabel="Categories">
<field granite:class="cmp-cif-categorycarousel__editor-item-multifield-composite-item coral-Well"
Expand Down
5 changes: 5 additions & 0 deletions .../cif/components/commerce/experiencefragment/v1/experiencefragment/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
fieldDescription="The name of this experience fragment location."
fieldLabel="Experience fragment location name."
name="./fragmentLocation"/>
<id jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/textfield"
fieldDescription="HTML ID attribute to apply to the component."
fieldLabel="ID"
name="./id"/>
</items>
</column>
</items>
Expand Down
5 changes: 5 additions & 0 deletions .../components/commerce/featuredcategorylist/v1/featuredcategorylist/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@
</items>
</field>
</content>
<id jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/textfield"
fieldDescription="HTML ID attribute to apply to the component."
fieldLabel="ID"
name="./id"/>
</items>
</column>
</items>
Expand Down
6 changes: 5 additions & 1 deletion ...ent/jcr_root/apps/core/cif/components/commerce/product/v1/product/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@
text="Without a manual product selection, this component will display a product based on an URL selector."/>
</items>
</well>

<id jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/textfield"
fieldDescription="HTML ID attribute to apply to the component."
fieldLabel="ID"
name="./id"/>
</items>
</column>
</items>
Expand Down
1 change: 1 addition & 0 deletions ...s/core/cif/components/commerce/productcarousel/v1/productcarousel/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<items jcr:primaryType="nt:unstructured">
<content jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/multifield"
sling:orderBefore="id"
fieldLabel="Carousel content"
typeHint="[product@String[]]">
<field jcr:primaryType="nt:unstructured"
Expand Down
5 changes: 5 additions & 0 deletions ...re/cif/components/commerce/productcollection/v2/productcollection/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
fieldDescription="Number of products to show on single page"
fieldLabel="Page Size"
name="./pageSize"/>
<id jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/textfield"
fieldDescription="HTML ID attribute to apply to the component."
fieldLabel="ID"
name="./id"/>
</items>
</column>
</items>
Expand Down
2 changes: 2 additions & 0 deletions ...root/apps/core/cif/components/commerce/productlist/v2/productlist/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<showTitle
jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/checkbox"
sling:orderBefore="id"
fieldDescription="Show category title of product list"
name="./showTitle"
text="Show title"
Expand All @@ -26,6 +27,7 @@
<showImage
jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/checkbox"
sling:orderBefore="id"
fieldDescription="Show category image of product list"
name="./showImage"
text="Show image"
Expand Down
5 changes: 5 additions & 0 deletions .../apps/core/cif/components/commerce/productteaser/v1/productteaser/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@
fieldDescription="The text displayed by the call-to-action button"
fieldLabel="Call to action text"
name="./ctaText"/>
<id jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/textfield"
fieldDescription="HTML ID attribute to apply to the component."
fieldLabel="ID"
name="./id"/>
</items>
</column>
</items>
Expand Down
1 change: 1 addition & 0 deletions ...s/core/cif/components/commerce/relatedproducts/v1/relatedproducts/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
selectionId="sku" />
<relationType jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/select"
sling:orderBefore="id"
fieldLabel="Target products to display."
name="./relationType">
<datasource jcr:primaryType="nt:unstructured"
Expand Down
23 changes: 23 additions & 0 deletions ...jcr_root/apps/core/cif/components/commerce/searchbar/v1/searchbar/_cq_dialog/.content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>
<jcr:root xmlns:sling="http://sling.apache.org/jcr/sling/1.0" xmlns:cq="http://www.day.com/jcr/cq/1.0" xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:nt="http://www.jcp.org/jcr/nt/1.0"
jcr:primaryType="nt:unstructured"
jcr:title="Search Bar"
sling:resourceType="cq/gui/components/authoring/dialog"
trackingFeature="cif-core-components:searchbar:v1">
<content
jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/container">
<items jcr:primaryType="nt:unstructured">
<column jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/container">
<items jcr:primaryType="nt:unstructured">
<id jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/form/textfield"
fieldDescription="HTML ID attribute to apply to the component."
fieldLabel="ID"
name="./id"/>
</items>
</column>
</items>
</content>
</jcr:root>
Loading