adobe / commerce-cif-connector

AEM Commerce connector for Magento and GraphQL
Apache License 2.0
43 stars 27 forks source link

CIF-1065 - Preview PDP with real product data #66

Closed LSantha closed 4 years ago

LSantha commented 5 years ago

Description

Currently, the product detail template has the PDP component with dummy data. We want to add a preview feature to enable authors to preview the product page with specific product data.

Related Issue

CIF-1065

How Has This Been Tested?

Manually, IT

Types of changes

Checklist:

codecov[bot] commented 5 years ago

Codecov Report

Merging #66 into master will decrease coverage by 0.19%. The diff coverage is 87.09%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master      #66     +/-   ##
===========================================
- Coverage     86.05%   85.85%   -0.2%     
- Complexity      365      373      +8     
===========================================
  Files            23       24      +1     
  Lines          1348     1379     +31     
  Branches        210      216      +6     
===========================================
+ Hits           1160     1184     +24     
- Misses           86       89      +3     
- Partials        102      106      +4
Flag Coverage Δ Complexity Δ
#integration 54.24% <54.83%> (+0.01%) 233 <5> (+5) :arrow_up:
#unittests 81.05% <87.09%> (-0.09%) 352 <10> (+8)
Impacted Files Coverage Δ Complexity Δ
...e/renderconditions/IsProductDetailPageServlet.java 87.09% <87.09%> (ø) 10 <10> (?)
...e/cq/commerce/graphql/resource/ResourceMapper.java 75.41% <0%> (-1.68%) 35% <0%> (-2%)

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 62855c9...7a9fbaf. Read the comment docs.

lgtm-com[bot] commented 4 years ago

This pull request introduces 2 alerts when merging f9bf740f4283aef60e9d65eb0db247c65403d333 into 580a743792658bb4601423a425be40fff97d0d4f - view on LGTM.com

new alerts:

herzog31 commented 4 years ago

Are you planning to add the newly added Karma tests to CircleCI?

LSantha commented 4 years ago

Are you planning to add the newly added Karma tests to CircleCI?

@herzog31 , not in this PR but eventually they will have to be in CircleCI, I think.

herzog31 commented 4 years ago

@LSantha That's fine. Can you create a follow up issue, so we can track this and you don't forget about it?

LSantha commented 4 years ago

@herzog31 , I think the fastest solution would be if you could included it in https://github.com/adobe/commerce-cif-connector/pull/69 . WDYT, would that work?

LSantha commented 4 years ago

@herzog31, @herzog31 , @mhaack , with my latest commit I have passing JS tests, coverage and Java related concerns addressed. Could we move forward with the PR?

herzog31 commented 4 years ago

@LSantha https://github.com/adobe/commerce-cif-connector/pull/69 is about moving Java integration tests to CircleCI and is un-related to any JavaScript unit tests.

herzog31 commented 4 years ago

Did you try to run the tests you added? For me npm test fails with:

26 11 2019 14:10:42.008:ERROR [coverage]: HeadlessChrome 78.0.3904 (Mac OS X 10.15.0): Coverage for statements (79.33%) does not meet global threshold (90%)
26 11 2019 14:10:42.009:ERROR [coverage]: HeadlessChrome 78.0.3904 (Mac OS X 10.15.0): Coverage for statements (40%) does not meet per-file (/commerce-cif-connector/content/cif-connector/src/main/content/jcr_root/libs/commerce/gui/components/common/cifcategoryfield/clientlibs/cifcategorypicker.js)  threshold (80%)
26 11 2019 14:10:42.009:ERROR [coverage]: HeadlessChrome 78.0.3904 (Mac OS X 10.15.0): Coverage for branches (8.33%) does not meet per-file (/commerce-cif-connector/content/cif-connector/src/main/content/jcr_root/libs/commerce/gui/components/common/cifcategoryfield/clientlibs/cifcategorypicker.js)  threshold (65%)
26 11 2019 14:10:42.016:ERROR [coverage]: Firefox 70.0.0 (Mac OS X 10.15.0): Coverage for statements (79.33%) does not meet global threshold (90%)
26 11 2019 14:10:42.016:ERROR [coverage]: Firefox 70.0.0 (Mac OS X 10.15.0): Coverage for statements (40%) does not meet per-file (/commerce-cif-connector/content/cif-connector/src/main/content/jcr_root/libs/commerce/gui/components/common/cifcategoryfield/clientlibs/cifcategorypicker.js)  threshold (80%)
26 11 2019 14:10:42.016:ERROR [coverage]: Firefox 70.0.0 (Mac OS X 10.15.0): Coverage for branches (8.33%) does not meet per-file (/commerce-cif-connector/content/cif-connector/src/main/content/jcr_root/libs/commerce/gui/components/common/cifcategoryfield/clientlibs/cifcategorypicker.js)  threshold (65%)
LSantha commented 4 years ago

Please also make sure that npm test actually passes.

@herzog31, my bad, some files were committed which do not belong here. Tests should pass again.

mhaack commented 4 years ago

Verified