Skip to content

Validation to prevent using DWC within DWC #14744

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
Merged
merged 3 commits into from
Mar 17, 2025
Merged

Validation to prevent using DWC within DWC #14744

merged 3 commits into from
Mar 17, 2025

Conversation

fedys
Copy link
Member

@fedys fedys commented Mar 14, 2025

Q A
Bug fix? (use the a.b branch) ❌
New feature/enhancement? (use the a.x branch) ✔️
Deprecations? ❌
BC breaks? (use the c.x branch) ❌
Automated tests included? ✔️
Related user documentation PR URL
Related developer documentation PR URL
Issue(s) addressed

Description

This PR adds a new validation that prevents putting a dynamic web content token into another dynamic web content. It also renames the menu item from Dynamic Content to Dynamic Web Content.

📋 Steps to test this PR:

  1. Navigate to /s/dwc/new.
  2. Enter {dwc=some} into the Content field.
  3. Click the Save button.
  4. You should see the following validation error message below the Content field. 
    DWC tokens cannot be used within another DWC. Please remove any DWC tokens from the content to proceed.
  5. Validate the above steps for editing an existing dynamic web content.
  6. Make sure that the menu item no longer reads Dynamic Content. It should be renamed to Dynamic Web Content.

@fedys fedys requested review from a team, rohitpavaskar and rahuld-dev and removed request for a team March 14, 2025 14:56
@codecov Codecov
Copy link

codecov bot commented Mar 14, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.77%. Comparing base (a83da4a) to head (b4c5298).
Report is 10 commits behind head on 7.x.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                7.x   #14744      +/-   ##
============================================
+ Coverage     64.75%   64.77%   +0.02%     
- Complexity    34823    34831       +8     
============================================
  Files          2283     2289       +6     
  Lines        103933   103954      +21     
============================================
+ Hits          67297    67340      +43     
+ Misses        36636    36614      -22
Files with missing lines Coverage Δ
...les/DynamicContentBundle/Entity/DynamicContent.php 74.83% <100.00%> (+0.16%) ⬆️
...undle/Validator/Constraints/NoNestingValidator.php 100.00% <100.00%> (ø)

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@matbcvo matbcvo added this to the 7.0.0-alpha milestone Mar 14, 2025
@matbcvo matbcvo added the enhancement Any improvement to an existing feature or functionality label Mar 14, 2025
@matbcvo matbcvo added ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging labels Mar 14, 2025
rahuld-dev
rahuld-dev approved these changes Mar 17, 2025
View reviewed changes
Copy link
Contributor

@rahuld-dev rahuld-dev left a comment

Choose a reason for hiding this comment

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

Thanks @fedys

@rahuld-dev rahuld-dev added code-review-passed PRs which have passed code review and removed code-review-needed PR's that require a code review before merging labels Mar 17, 2025
@shinde-rahul shinde-rahul added dynamic-content feature A new feature for inclusion in minor or major releases labels Mar 17, 2025
shinde-rahul
shinde-rahul reviewed Mar 17, 2025
View reviewed changes
Copy link
Contributor

@shinde-rahul shinde-rahul left a comment

Choose a reason for hiding this comment

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

@fedys, the coverage needs some attention.

The code looks good to me.

@shinde-rahul shinde-rahul added the pending-feedback PR's and issues that are awaiting feedback from the author label Mar 17, 2025
rohitpavaskar
rohitpavaskar approved these changes Mar 17, 2025
View reviewed changes
Copy link
Contributor

@rohitpavaskar rohitpavaskar left a comment

Choose a reason for hiding this comment

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

Tested PR, working as expected.
image

@rohitpavaskar rohitpavaskar added user-testing-passed PRs which have been successfully tested by the required number of people. and removed ready-to-test PR's that are ready to test labels Mar 17, 2025
@escopecz escopecz merged commit bad47fb into mautic:7.x Mar 17, 2025
15 checks passed
@escopecz escopecz removed the pending-feedback PR's and issues that are awaiting feedback from the author label Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shinde-rahul shinde-rahul Awaiting requested review from shinde-rahul

@rahuld-dev rahuld-dev Awaiting requested review from rahuld-dev

1 more reviewer

@rohitpavaskar rohitpavaskar rohitpavaskar approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
code-review-passed PRs which have passed code review dynamic-content enhancement Any improvement to an existing feature or functionality feature A new feature for inclusion in minor or major releases user-testing-passed PRs which have been successfully tested by the required number of people.
Projects
Open Source Fridays
Status: 🥳 Done
Milestone
7.0.0-alpha
Development

Successfully merging this pull request may close these issues.
6 participants
@fedys @shinde-rahul @rohitpavaskar @rahuld-dev @matbcvo @escopecz