Skip to content

Conversation

@naoya-kawakatsu
Copy link
Contributor

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 N/A
Related developer documentation PR URL N/A
Issue(s) addressed N/A

Description

Currently, in the email report, "Ignored / Read / Failed emails" does not provide accurate information because it always reflects only the information of the email_stats1 record.
スクリーンショット 2025-07-11 12 08 22
The query executed in StatRepository's getIgnoredReadFailed is as follows:

SELECT count(es.id) as sent, count(CASE WHEN es.is_read THEN 1 ELSE null END) as "read", count(CASE WHEN es.is_failed THEN 1 ELSE null END) as failed FROM email_stats es LEFT JOIN emails e ON e.id = es.email_id WHERE (es.date_sent IS NULL OR (es.date_sent BETWEEN :dateFrom AND :dateTo)) AND (e.is_published = :i0ceispublished) GROUP BY es.id;

The query itself is not a big problem. However, the problem is that because of the "GROUP BY es.id;" statement, there is only one piece of email_stats information per row, but getIgnoredReadFailed only uses that one row of data.
As a result, the pie chart always only provides information for one email.

スクリーンショット 2025-07-11 12 10 47

This PR solves the problem by removing the group by statement from the query, allowing getIgnoredReadFailed to execute the desired query.


📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Open the reports screen and check the "Ignored / Read / Failed emails" pie chart.

Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

Yes, we use the same fix in our branch. Good to go

@matbcvo matbcvo added bug Issues or PR's relating to bugs reports Anything related to reports labels Sep 26, 2025
@sonarqubecloud
Copy link

Copy link
Contributor

@matbcvo matbcvo left a comment

Choose a reason for hiding this comment

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

I have tested the PR, works as expected. Thanks! 👍

@matbcvo
Copy link
Contributor

matbcvo commented Sep 29, 2025

@all-contributors please add @naoya-kawakatsu for code

@allcontributors
Copy link
Contributor

@matbcvo

I've put up a pull request to add @naoya-kawakatsu! 🎉

@matbcvo matbcvo added code-review-passed PRs which have passed code review user-testing-passed PRs which have been successfully tested by the required number of people. labels Sep 29, 2025
@matbcvo matbcvo added this to the 7.0.0-beta milestone Sep 29, 2025
@matbcvo matbcvo added the ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged label Sep 29, 2025
@matbcvo matbcvo merged commit d745fcd into mautic:7.x Sep 29, 2025
24 checks passed
@codecov
Copy link

codecov bot commented Sep 29, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.91%. Comparing base (cd00101) to head (1fad126).
⚠️ Report is 8 commits behind head on 7.x.

Files with missing lines Patch % Lines
...les/EmailBundle/EventListener/ReportSubscriber.php 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                7.x   #15519      +/-   ##
============================================
- Coverage     67.91%   67.91%   -0.01%     
  Complexity    36386    36386              
============================================
  Files          2366     2366              
  Lines        146163   146164       +1     
============================================
- Hits          99266    99265       -1     
- Misses        46897    46899       +2     
Files with missing lines Coverage Δ
...les/EmailBundle/EventListener/ReportSubscriber.php 71.83% <0.00%> (-0.13%) ⬇️

... and 1 file 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.

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 ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged reports Anything related to reports user-testing-passed PRs which have been successfully tested by the required number of people.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants