acs-aem-commons
acs-aem-commons copied to clipboard
CCVAR: Fixed Same Attribute not updating correctly.
Defect: CCVAR Same Attribute doesn't update all placeholders in the value.
Uses the original attribute value and only updates the last placeholder value leaving the other placeholders as it is.
Example:
Attr "title=((custom_prefix)) ((custom_title))"
Expected: "title=Prefix Title"
Actual: "title=((custom_prefix)) Title"
Solution: Update the currentAttr Value with new attrValue when looping placeholders.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 55.45%. Comparing base (
7676ed5
) to head (9d78151
). Report is 19 commits behind head on master.
:exclamation: Current head 9d78151 differs from pull request most recent head 2000b24
Please upload reports for the commit 2000b24 to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## master #3275 +/- ##
============================================
+ Coverage 55.34% 55.45% +0.11%
+ Complexity 5543 5506 -37
============================================
Files 728 721 -7
Lines 29705 29535 -170
Branches 3875 3840 -35
============================================
- Hits 16440 16379 -61
+ Misses 11722 11630 -92
+ Partials 1543 1526 -17
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@kwin @csaboka @davidjgonzalez I added this fix, can you please review.
Also, I added test case but it still failing. Can you please tell me what am I missing here?
@vabs95 : JUnit 4 tests are not running at the moment, I'm trying to fix that in #3273 . Since the test you are trying to extend uses JUnit 4 tools, it doesn't run at all and won't contribute to coverage no matter what you change.
You can either wait until #3273 is merged, or try rewriting the test class to JUnit 5.
@csaboka Can you please review it now?
@vabs95 : I'm not a project owner, just a minor contributor to the project. I'm not familiar with the code you touched, but even if I were, my approval wouldn't be enough to allow merging this.
@kwin can you please review it ?
@davidjgonzalez can you please re-run the checks?
hi @davidjgonzalez can you please merge this now? It has been pending for a long time.