-
-
Notifications
You must be signed in to change notification settings - Fork 3k
fix: [DPMMA-3098] fix report data bool filter #14909
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: [DPMMA-3098] fix report data bool filter #14909
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 5.2 #14909 +/- ##
============================================
+ Coverage 63.27% 63.68% +0.40%
- Complexity 34619 34676 +57
============================================
Files 2272 2274 +2
Lines 103558 103729 +171
============================================
+ Hits 65530 66058 +528
+ Misses 38028 37671 -357 🚀 New features to boost your workflow:
|
Can you rebase this onto the 5.2 branch? We should have this fix in that version. |
@AlanWierzchonCA, just a heads-up, it needs to be rebased onto the 5.2 branch, not 5.x. The 5.x branch is locked and doesn’t accept any PRs. We'll merge it into the 6.0 and 7.x branches later. |
364aefa
to
57f7984
Compare
@patrykgruszka @matbcvo rebased this onto the 5.2 branch |
This fix makes the boolean appear, but there’s another issue - the boolean value is not being saved. I checked the database and confirmed that the boolean value is not being saved or updated. For example, the "Dynamic?" boolean works correctly. Screen.Recording.2025-04-26.180515.mp4 |
I have tested this PR locally, and everything works correctly with the latest commit ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Faced the same issue. And this fixed it 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach will cause ID conflicts, I would suggest to try with something like this:
if (filterType == 'bool' || filterType == 'boolean') {
const yesId = valueId + '_1';
const noId = valueId + '_0';
const isYes = valueVal == '1';
const $template = mQuery(mQuery('#filterValueYesNoTemplate').html());
const $label = $template.find('#report_value_template_yesno_label');
const $yesOption = $template.find('#report_value_template_yesno_1');
const $noOption = $template.find('#report_value_template_yesno_0');
$yesOption.attr('name', valueName)
.attr('id', yesId);
$noOption.attr('name', valueName)
.attr('id', noId);
$label.attr('id', valueId + '_bool-label')
.attr('data-yes-id', yesId)
.attr('data-no-id', noId);
mQuery(valueEl).replaceWith($template);
if (!isYes) {
Mautic.toggleYesNo($label);
}
} else if // ....
26a6401
to
c2f0b78
Compare
@kuzmany , @matbcvo I made changes as suggested by @patrykgruszka |
69ad29b
to
b5124b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine now, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes look good
Description
Fix for Issue: #14857
Bug: "Is Published" filter in Reports doesn't allow selecting "True" or "False"
📋 Steps to test this PR: