Skip to content

Conversation

@buuhuu
Copy link
Contributor

@buuhuu buuhuu commented Nov 30, 2021

Description

With #654 a caching mechanism was introduced to createProductPageUrl() in order to prevent querying for the store config meta element multiple times. This causes a regression as the caching stored a full product url and returned it in subsequent calls instead of just the storeRootUrl part. This PR removes the caching and adds more coverage to the method to validate the behaviour for subsequent calls.

Related Issue

#654 CIF-2292
CIF-2578

Motivation and Context

Regression introduced in CIF-2292

How Has This Been Tested?

Unit tests and locally

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.

@buuhuu buuhuu added the bug Something isn't working label Nov 30, 2021
@codecov
Copy link

codecov bot commented Nov 30, 2021

Codecov Report

Merging #767 (4f671d4) into master (fea9a55) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #767      +/-   ##
============================================
+ Coverage     88.75%   88.77%   +0.01%     
  Complexity     1751     1751              
============================================
  Files           305      305              
  Lines          7832     7828       -4     
  Branches       1167     1166       -1     
============================================
- Hits           6951     6949       -2     
+ Misses          653      651       -2     
  Partials        228      228              
Flag Coverage Δ
integration 58.50% <ø> (ø)
jest 86.23% <100.00%> (+0.05%) ⬆️
karma 89.86% <ø> (ø)
unittests 88.72% <ø> (ø)

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

Impacted Files Coverage Δ
react-components/src/utils/createProductPageUrl.js 100.00% <100.00%> (+12.50%) ⬆️

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 fea9a55...4f671d4. Read the comment docs.

@mhaack mhaack merged commit 11e98e6 into master Nov 30, 2021
@mhaack mhaack deleted the issue/CIF-2578 branch November 30, 2021 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants