Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: filter path because a dynamic zone is a component but it should be classified as a dynamic zone #15139

Merged
merged 2 commits into from Dec 9, 2022

Conversation

joshuaellis
Copy link
Member

What does it do?

  • avoids undefined sections of paths in the recursive finding functions that have something like path.undefined with dynamic zones

Why is it needed?

  • dynamic zone indicies are components but the path to the component would just be dynamiczone.0 but we don't want the index in these paths so it thinks it should just be dynamiczone.undefined when looking for components, however this should actually be captured by the dynamic zone findings. It's a bit confusing.

Related issue(s)/PR(s)

@joshuaellis joshuaellis added source: core:content-manager Source is core/content-manager package pr: fix This PR is fixing a bug labels Dec 9, 2022
@codecov
Copy link

codecov bot commented Dec 9, 2022

Codecov Report

Base: 64.33% // Head: 59.88% // Decreases project coverage by -4.44% ⚠️

Coverage data is based on head (d5ef35e) compared to base (11d5ceb).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15139      +/-   ##
==========================================
- Coverage   64.33%   59.88%   -4.45%     
==========================================
  Files        1058     1348     +290     
  Lines       22551    32806   +10255     
  Branches     3986     6256    +2270     
==========================================
+ Hits        14508    19646    +5138     
- Misses       7081    11304    +4223     
- Partials      962     1856     +894     
Flag Coverage Δ
back 50.08% <ø> (?)
front 64.34% <100.00%> (+<0.01%) ⬆️
unit_back 50.08% <ø> (?)
unit_front 64.34% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
...ponents/DynamicZone/components/DynamicComponent.js 92.53% <100.00%> (+0.11%) ⬆️
...ider/utils/recursivelyFindPathsBasedOnCondition.js 100.00% <100.00%> (ø)
...lationInputDataManager/RelationInputDataManager.js 97.46% <100.00%> (ø)
...ore/database/lib/query/helpers/populate/process.js 10.20% <0.00%> (ø)
...ages/plugins/i18n/server/services/content-types.js 90.81% <0.00%> (ø)
packages/utils/typescript/lib/compile.js 42.85% <0.00%> (ø)
packages/core/utils/lib/pipe-async.js 100.00% <0.00%> (ø)
packages/core/admin/server/domain/role.js 60.00% <0.00%> (ø)
packages/core/admin/server/services/token.js 90.90% <0.00%> (ø)
... and 284 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

ronronscelestes
ronronscelestes previously approved these changes Dec 9, 2022
Copy link
Contributor

@ronronscelestes ronronscelestes left a comment

Choose a reason for hiding this comment

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

except for Gustav's comment about adding a test case, looks good to me!
nice catch thank you 🙏

@joshuaellis joshuaellis merged commit 03f2de9 into main Dec 9, 2022
@joshuaellis joshuaellis deleted the fix/avoid-undefined-paths-in-recursion branch December 9, 2022 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix This PR is fixing a bug source: core:content-manager Source is core/content-manager package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants