Skip to content

Conversation

@cjelger
Copy link
Contributor

@cjelger cjelger commented Dec 10, 2019

  • added sling POST processor to add products to the array property
  • modified carousel to support products paths and skus

Description

This PR adds the possibility to add products to the carousel with drag'n'drop, and extends the array of products with each new product added to the carousel.

How Has This Been Tested?

Added unit tests and manually tested.

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:

  • 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.

- added sling POST processor to add products to the array property
- modified carousel to support products paths and skus
@codecov
Copy link

codecov bot commented Dec 10, 2019

Codecov Report

Merging #158 into master will decrease coverage by 28.54%.
The diff coverage is 90%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master     #158       +/-   ##
=============================================
- Coverage     83.47%   54.92%   -28.55%     
- Complexity      395      406       +11     
=============================================
  Files            50      140       +90     
  Lines          1452     3814     +2362     
  Branches        118      665      +547     
=============================================
+ Hits           1212     2095      +883     
- Misses          161     1636     +1475     
- Partials         79       83        +4
Flag Coverage Δ Complexity Δ
#jest 36.27% <ø> (?) 0 <ø> (?)
#karma 93.66% <ø> (ø) 0 <ø> (ø) ⬇️
#unittests 81.34% <90%> (+0.35%) 406 <12> (+11) ⬆️
Impacted Files Coverage Δ Complexity Δ
...models/v1/productcarousel/ProductCarouselImpl.java 76.74% <80%> (-0.18%) 11 <2> (+1)
...al/servlets/MultiFieldDropTargetPostProcessor.java 91.11% <91.11%> (ø) 10 <10> (?)
...t-components/src/components/Minicart/couponForm.js 100% <0%> (ø) 0% <0%> (?)
...nents/src/components/Minicart/emptyMinicartBody.js 60% <0%> (ø) 0% <0%> (?)
...mponents/src/components/CartTrigger/cartCounter.js 100% <0%> (ø) 0% <0%> (?)
.../src/components/Checkout/shippingAddressSummary.js 22.22% <0%> (ø) 0% <0%> (?)
...-components/src/components/Checkout/addressForm.js 23.52% <0%> (ø) 0% <0%> (?)
react-components/src/components/SignIn/signIn.js 100% <0%> (ø) 0% <0%> (?)
...t-components/src/components/Minicart/couponItem.js 100% <0%> (ø) 0% <0%> (?)
...act-components/src/queries/query_countries.graphql 12.5% <0%> (ø) 0% <0%> (?)
... and 82 more

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 6ca98e1...76fdfba. Read the comment docs.

*
*/
@Component(service = { SlingPostProcessor.class })
public class MultiFieldDropTargetPostProcessor implements SlingPostProcessor {
Copy link
Member

Choose a reason for hiding this comment

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

Would it possible to scope this servlet somehow to our CIF components? I'm wondering what would happen if a customer already has a custom drop target servlet like this for other components?

At least we should mention this in the documentation or component readme.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's kind of scoped because it only handles property keys starting with ./dropTarget-> so it could only "collide with itself" if a customer already uses the same post processor. I will extend the documentation or component readme.

- changed property prefix to "multiDropTarget->"
@LSantha LSantha merged commit 4965d41 into master Dec 13, 2019
@LSantha LSantha deleted the CIF-1115 branch December 13, 2019 10:55
@herzog31 herzog31 added the enhancement New feature or request label Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request To Verify

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants