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

Resolving an error when deleting segments #14553

Open
wants to merge 3 commits into
base: 6.x
Choose a base branch
from

Conversation

escopecz
Copy link
Member

@escopecz escopecz commented Feb 4, 2025

Q A
Bug fix? (use the a.b branch) [y]
New feature/enhancement? (use the a.x branch) [n]
Deprecations? [n]
BC breaks? (use the c.x branch) [n]
Automated tests included? [y]
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed /

Description:

The batch deleting segments feature can end up with a 500 error when the segment has a specific filter.

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)

  2. Create a segment A with filter segment membership filter “Segment Membership“ EMPTY.

  3. Create a Segment B with any filter.

  4. Try to batch delete a segment.

  5. check error in console.

Other areas of Mautic that may be affected by the change:

  1. Just segment delete

List of areas covered by the unit and/or functional tests:

  1. Batch deleting segments with a filter of segment membership is empty

Maut 11906 - Resolving an error when deleting segments
@escopecz escopecz added bug Issues or PR's relating to bugs code-review-needed PR's that require a code review before merging segments Anything related to segments unforking Used for PRs in the Acquia's unforking initiative labels Feb 4, 2025
removing the delete command test as it hasn't been pushed to the community yet
@escopecz escopecz requested review from a team and aarohiprasad and removed request for a team February 4, 2025 11:54
Copy link

codecov bot commented Feb 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.04%. Comparing base (b94713f) to head (2f59eba).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                6.x   #14553   +/-   ##
=========================================
  Coverage     64.04%   64.04%           
  Complexity    34664    34664           
=========================================
  Files          2287     2287           
  Lines        103757   103757           
=========================================
  Hits          66446    66446           
  Misses        37311    37311           
Files with missing lines Coverage Δ
app/bundles/LeadBundle/Model/ListModel.php 65.06% <100.00%> (ø)

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.

Looks good. 👍

@shinde-rahul shinde-rahul added code-review-passed PRs which have passed code review pending-test-confirmation PR's that require one test before they can be merged and removed code-review-needed PR's that require a code review before merging labels Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs code-review-passed PRs which have passed code review pending-test-confirmation PR's that require one test before they can be merged segments Anything related to segments unforking Used for PRs in the Acquia's unforking initiative
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants