SITES-22965: Add function to set category filter type #1011

alwinjoseph02 · 2024-06-19T14:19:40Z

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes and the overall coverage did not decrease.
All unit tests pass on CircleCi.
I ran all tests locally and they pass.

codecov · 2024-06-20T05:44:26Z

Codecov Report

Attention: Patch coverage is 96.10390% with 3 lines in your changes missing coverage. Please review.

Project coverage is 89.14%. Comparing base (2ea43ba) to head (7eac3d3).

Files	Patch %	Lines
.../components/internal/services/UrlProviderImpl.java	95.34%	0 Missing and 2 partials ⚠️
...al/services/urlformats/CategoryPageWithUrlKey.java	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1011      +/-   ##
============================================
+ Coverage     89.11%   89.14%   +0.02%     
- Complexity     2226     2247      +21     
============================================
  Files           355      355              
  Lines         10042    10097      +55     
  Branches       1447     1461      +14     
============================================
+ Hits           8949     9001      +52     
  Misses          790      790              
- Partials        303      306       +3

Flag	Coverage Δ
integration	`51.63% <48.05%> (-0.20%)`	⬇️
jest	`86.68% <ø> (ø)`
karma	`95.54% <ø> (ø)`
unittests	`87.44% <96.10%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…ible with urlPath

LSantha · 2024-07-09T14:56:52Z

...be/cq/commerce/core/components/internal/models/v1/categorylist/FeaturedCategoryListImpl.java

@@ -106,6 +108,8 @@ private void initModel() {
        // After the identifier type has been determined, the specific list will be used further
        List<String> categoryIdentifiers = new ArrayList<>();
        assetOverride = new HashMap<>();
+        String categoryIdType = null;
+        String categoryFilterType;


Could you check if you really need this categoryFilterType variable? It looks like it's serving basically the same purpose as categoryIdType. If so, please keep categoryIdType only.

LSantha · 2024-07-09T15:18:23Z

...be/cq/commerce/core/components/internal/models/v1/categorylist/FeaturedCategoryListImpl.java

@@ -76,6 +77,7 @@ public class FeaturedCategoryListImpl extends DataLayerComponent implements Feat
    private static final String CATEGORY_IDENTIFIER = "categoryId";
    private static final String ASSET_PROP = "asset";
    private static final String ITEMS_PROP = "items";
+    private static final String CATEGORY_IDENTIFIER_TYPE = "categoryIdType";


You may want to put this on the public interface like here: https://github.com/adobe/aem-core-cif-components/pull/1011/files#diff-5e1c4c18538024ef6a86682d30b765717aeea3d4af9372f76de7b34190c67afdR44

LSantha · 2024-07-09T15:45:12Z

...java/com/adobe/cq/commerce/core/components/models/retriever/AbstractCategoriesRetriever.java

+     * Filter type that should be used when fetched. usually uid but we can define it explicitly in the implementation
+     * specific and should be checked in subclass implementations.
+     */
+    protected String filterType;


Here also categoryIdType is a better name and is consistent with the setter method bellow.

LSantha · 2024-07-09T15:45:53Z

...java/com/adobe/cq/commerce/core/components/models/retriever/AbstractCategoriesRetriever.java

+     * Set the Category filter type which will using during fetch. Categories are retrieved using
+     * this filter type if not set then it will use UID.
+     *
+     * @param filterType Filter Type


Parameter name: categoryIdType

LSantha · 2024-07-09T15:46:40Z

...java/com/adobe/cq/commerce/core/components/models/retriever/AbstractCategoriesRetriever.java

+     * @param filterType Filter Type
+     */
+    public void setCategoryIdType(String filterType) {
+        this.filterType = filterType;


this.categoryIdType = categoryIdType

LSantha · 2024-07-09T15:47:41Z

...n/java/com/adobe/cq/commerce/core/components/models/retriever/AbstractCategoryRetriever.java

+     * Filter type that should be used when fetched. usually uid but we can define it explicitly in the implementation
+     * specific and should be checked in subclass implementations.
+     */
+    protected String filterType;


Again, use categoryIdType.

LSantha · 2024-07-09T15:48:32Z

...n/java/com/adobe/cq/commerce/core/components/models/retriever/AbstractCategoryRetriever.java

+     * Set the Category filter type which will using during fetch. Categories are retrieved using
+     * this filter type if not set then it will use UID.
+     *
+     * @param filterType Filter Type


Here too: categoryIdType

LSantha · 2024-07-09T16:11:08Z

@alwinjoseph02 , could you please update the readme of components where you added the new JCR property (categoryIdType) like here https://github.com/adobe/aem-core-cif-components/tree/master/ui.apps/src/main/content/jcr_root/apps/core/cif/components/commerce/featuredcategorylist/v1/featuredcategorylist#edit-dialog-properties

… usages" This reverts commit 4ce0d82.

LSantha · 2024-07-17T13:29:43Z

@alwinjoseph02 , LGTM! Thank you.

SITES-21947: Add function to set category filter type

56a2268

alwinjoseph02 requested a review from LSantha June 19, 2024 14:19

alwinjoseph02 added 5 commits June 19, 2024 20:03

SITES-21947: Resolves the space issues & comment

3d53247

SITES-21947: Resolves the space issues & comment

e9e084c

SITES-21947: Updates the comment

2bb1584

SITES-21947: Removes the spaces

8571af9

SITES-21947: Adds proper formatting

8cb6da3

SITES-22965: Adds teaser,button & category carousel compoment compact…

ae09b83

…ible with urlPath

alwinjoseph02 changed the title ~~SITES-21947: Add function to set category filter type~~ SITES-22965: Add function to set category filter type Jun 27, 2024

alwinjoseph02 added 4 commits June 27, 2024 12:50

SITES-22965: Uses the void as return

e68bac5

SITES-22965: Adds code coverage

cbb153e

SITES-22965: resolves the code format issues

b0ef429

SITES-22965: Adds validate method on url formaters

2628e6c

alwinjoseph02 marked this pull request as ready for review July 2, 2024 08:11

alwinjoseph02 added 9 commits July 2, 2024 16:29

SITES-22965: Optimize the code & adds new unit test

9b43f44

SITES-22965: Adds test coverages

2b8105c

SITES-22965: Adds test coverages for urlproviderimpl

eabbe62

Merge branch 'master' into feature/SITES-21947

a2e01c5

SITES-22965: Adds test coverage

8f98dd3

SITES-22965: Formats the code

2cbebf4

SITES-22965: Adds new method to generate the category url format

088b6a9

SITES-22965: Resolves unwanted imports

673f47d

SITES-22965: Reverts the unwanted changes

5647675

GiovanniReza approved these changes Jul 4, 2024

View reviewed changes

SITES-22965: Adds new identifier

5c3043a

LSantha reviewed Jul 9, 2024

View reviewed changes

alwinjoseph02 added 3 commits July 10, 2024 12:00

SITES-22965: Adds the PR comments changes

8a7fd08

SITES-22965: Updates the readme & comments

9a1705b

SITES-22965: Adds more test coverage

5d500bb

alwinjoseph02 requested a review from LSantha July 10, 2024 15:57

alwinjoseph02 added 3 commits July 11, 2024 11:55

SITES-22965: Makes tocategoryurl to deprecated and reomve its usages

4ce0d82

Revert "SITES-22965: Makes tocategoryurl to deprecated and reomve its…

62bed42

… usages" This reverts commit 4ce0d82.

SITES-22965: Reverted deprecation changes and changes the method name

7eac3d3

LSantha approved these changes Jul 17, 2024

View reviewed changes

LSantha merged commit 6c9245f into master Jul 17, 2024
19 checks passed

LSantha deleted the feature/SITES-21947 branch July 17, 2024 13:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SITES-22965: Add function to set category filter type #1011

SITES-22965: Add function to set category filter type #1011

alwinjoseph02 commented Jun 19, 2024

codecov bot commented Jun 20, 2024 •

edited

Loading

LSantha Jul 9, 2024 •

edited

Loading

LSantha Jul 9, 2024

LSantha Jul 9, 2024

LSantha Jul 9, 2024

LSantha Jul 9, 2024

LSantha Jul 9, 2024

LSantha Jul 9, 2024

LSantha commented Jul 9, 2024

LSantha commented Jul 17, 2024 •

edited

Loading

SITES-22965: Add function to set category filter type #1011

SITES-22965: Add function to set category filter type #1011

Conversation

alwinjoseph02 commented Jun 19, 2024

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

codecov bot commented Jun 20, 2024 • edited Loading

Codecov Report

LSantha Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

LSantha Jul 9, 2024

Choose a reason for hiding this comment

LSantha Jul 9, 2024

Choose a reason for hiding this comment

LSantha Jul 9, 2024

Choose a reason for hiding this comment

LSantha Jul 9, 2024

Choose a reason for hiding this comment

LSantha Jul 9, 2024

Choose a reason for hiding this comment

LSantha Jul 9, 2024

Choose a reason for hiding this comment

LSantha commented Jul 9, 2024

LSantha commented Jul 17, 2024 • edited Loading

codecov bot commented Jun 20, 2024 •

edited

Loading

LSantha Jul 9, 2024 •

edited

Loading

LSantha commented Jul 17, 2024 •

edited

Loading