Skip to content

[Bug/VB-447] Fixes data-cslp generation logic in case of applied_variants #176

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: development
Choose a base branch
from
Open

[Bug/VB-447] Fixes data-cslp generation logic in case of applied_variants #176

wants to merge 10 commits into from
+318 −45

Conversation

csAyushDubey
Copy link

@csAyushDubey csAyushDubey commented Sep 1, 2025 •
edited
Loading

Description:
Added logic for handling the following case:

  1. For a variant resolved entry (applied_variants), there exist instances of nested content that is resolved from a variant, in these cases, only the path of the parent field is available in the applied_variants and so all the children on this field are provided data-cslp of the base entry while they should have been provided of the variant.

@csAyushDubey csAyushDubey requested a review from a team as a code owner September 1, 2025 09:41
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2025 •
edited
Loading

Coverage report

St.❔
 Category Percentage Covered / Total
🟢 Statements
99.1% (+0.79% 🔼)
 553/558
🟢 Branches
87.69% (+1.8% 🔼)
 513/585
🟢 Functions 100% 131/131
🟢 Lines
99.42% (+0.63% 🔼)
 511/514

Test suite run success

249 tests passing in 19 suites.

Report generated by 🧪jest coverage report action from 2210ed2

hiteshshetty-dev
hiteshshetty-dev reviewed Sep 3, 2025
View reviewed changes
Copy link

@hiteshshetty-dev hiteshshetty-dev left a comment

Choose a reason for hiding this comment

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

Let's also add test cases for these changes.

@csAyushDubey
Copy link
Author

Added test cases

hiteshshetty-dev
hiteshshetty-dev reviewed Sep 4, 2025
View reviewed changes
@csAyushDubey csAyushDubey requested a review from a team as a code owner September 8, 2025 19:02
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 9, 2025

Coverage report

St.❔
 Category Percentage Covered / Total
🟢 Statements
99.1% (+0.79% 🔼)
 553/558
🟢 Branches
87.69% (+1.8% 🔼)
 513/585
🟢 Functions 100% 131/131
🟢 Lines
99.42% (+0.63% 🔼)
 511/514

Test suite run success

249 tests passing in 19 suites.

Report generated by 🧪jest coverage report action from 2210ed2

hiteshshetty-dev
hiteshshetty-dev previously approved these changes Sep 10, 2025
View reviewed changes
Copy link

@hiteshshetty-dev hiteshshetty-dev left a comment

Choose a reason for hiding this comment

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

LGTM!! Loved the improvement in CSLP generation.

@csAyushDubey csAyushDubey changed the base branch from master to development September 11, 2025 05:51
@csAyushDubey csAyushDubey dismissed hiteshshetty-dev’s stale review September 11, 2025 05:51

The base branch was changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hiteshshetty-dev hiteshshetty-dev hiteshshetty-dev left review comments

@contentstackMridul contentstackMridul Awaiting requested review from contentstackMridul

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@csAyushDubey @hiteshshetty-dev @aravindbuilt