Skip to content

Conversation

@mhaack
Copy link
Contributor

@mhaack mhaack commented Aug 19, 2021

Description

ProductListItem did not give any access to the full product retrieved from backend, hence extended queries with custom attributes did not work since the retrieved custom attributes could not be accessed.

With this the a project developer gets access to the entire product retrieved from the backend.

Related Issue

CIF-2305

How Has This Been Tested?

Added unit test.

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.

@mhaack mhaack added the bug Something isn't working label Aug 19, 2021
@codecov
Copy link

codecov bot commented Aug 19, 2021

Codecov Report

Merging #661 (8ccaca5) into master (f33ad41) will increase coverage by 0.02%.
The diff coverage is 95.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #661      +/-   ##
============================================
+ Coverage     88.42%   88.44%   +0.02%     
- Complexity     1594     1598       +4     
============================================
  Files           280      280              
  Lines          6978     6986       +8     
  Branches       1046     1047       +1     
============================================
+ Hits           6170     6179       +9     
  Misses          610      610              
+ Partials        198      197       -1     
Flag Coverage Δ
integration 59.56% <85.00%> (+0.05%) ⬆️
jest 85.61% <ø> (ø)
karma 89.06% <ø> (ø)
unittests 89.02% <95.00%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
...core/components/models/common/ProductListItem.java 0.00% <ø> (ø)
...internal/models/v1/common/ProductListItemImpl.java 96.92% <94.44%> (-0.95%) ⬇️
.../converters/ProductToProductListItemConverter.java 100.00% <100.00%> (+8.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f33ad41...8ccaca5. Read the comment docs.

@herzog31 herzog31 requested a review from LSantha August 19, 2021 15:33
: CommerceIdentifierImpl.fromProductSku(sku);
}

public ProductListItemImpl(ProductInterface product, Page productPage, String activeVariantSku, SlingHttpServletRequest request,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the third constructor for this class. I think we should consider using another pattern for constructing these objects, maybe a "Builder"?

@LSantha LSantha merged commit 936f017 into master Aug 23, 2021
@LSantha LSantha deleted the issues/cif-2305 branch August 23, 2021 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working verified

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants