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

Report group by and calculated fields #3824

Merged
merged 33 commits into from Apr 19, 2017

Conversation

mqueme
Copy link
Contributor

@mqueme mqueme commented Apr 6, 2017

…nd aggregated functions to reports

Q A
Bug fix?
New feature? y
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

This PR adds custom contact fields to all data sources that have contact fields, it adds group by options to reports, it adds aggregated function to reports

Steps to test this PR:

Apply PR, test that reports work, and that you can add group by and aggregated fields to your reports, also see all contact column in you data sources that use contact fields
1.
2.

List deprecations along with the new alternative:

List backwards compatibility breaks:

@matishaladiwala matishaladiwala modified the milestone: 2.8.0 Apr 7, 2017
@mqueme mqueme added the ready-to-test PR's that are ready to test label Apr 10, 2017
@mqueme mqueme changed the title added custom fields to all reports with lead fields, added group by a… Report group by and calculated fields Apr 10, 2017
@escopecz escopecz self-assigned this Apr 11, 2017
@escopecz escopecz added the feature A new feature for inclusion in minor or major releases label Apr 11, 2017
@escopecz
Copy link
Sponsor Member

I managed to break it on email data source with this configuration:
screen shot 2017-04-11 at 12 24 09

500 Internal Server Error - An exception occurred while executing 'SELECT cut.hits AS hits, CONCAT(ROUND(cut.hits/(e.sent_count * cut.trackable_count)*100),'%') AS hits_ratio, cut.unique_hits AS unique_hits, CONCAT(ROUND(cut.unique_hits/(e.sent_count * cut.trackable_count)*100),'%') AS unique_ratio, COUNT(DISTINCT dnc.id) AS unsubscribed, CONCAT(ROUND((COUNT(DISTINCT dnc.id)/e.sent_count)*100),'%') AS unsubscribed_ratio, e.sent_count AS sent_count, e.subject AS subject, CONCAT(ROUND((e.read_count/e.sent_count)*100),'%') AS read_ratio, e.read_count AS read_count, e.name AS e_name, COUNT() as 'COUNT' FROM emails e LEFT JOIN emails vp ON vp.id = e.variant_parent_id LEFT JOIN categories c ON c.id = e.category_id LEFT JOIN (SELECT COUNT(cut2.channel_id) AS trackable_count, SUM(cut2.hits) AS hits, SUM(cut2.unique_hits) AS unique_hits, cut2.channel_id FROM channel_url_trackables cut2 WHERE cut2.channel = 'email' GROUP BY cut2.channel_id) cut ON e.id = cut.channel_id LEFT JOIN lead_donotcontact dnc ON e.id = dnc.channel_id and dnc.channel='email' and dnc.reason=1 GROUP BY e.id, e.sent_count': SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ') as 'COUNT' FROM emails e LEFT JOIN emails vp ON vp.id = e.variant_parent_id LE' at line 1

@escopecz escopecz added the pending-feedback PR's and issues that are awaiting feedback from the author label Apr 11, 2017
@mqueme
Copy link
Contributor Author

mqueme commented Apr 11, 2017

I have just tested same config. it worked for me. Did you clear your cache? regeerated assets?

@mqueme mqueme removed the pending-feedback PR's and issues that are awaiting feedback from the author label Apr 11, 2017
@escopecz
Copy link
Sponsor Member

Just update for other testers, we found a problematic configuration:
screen shot 2017-04-11 at 16 34 52

@escopecz escopecz added the pending-feedback PR's and issues that are awaiting feedback from the author label Apr 11, 2017
@mqueme
Copy link
Contributor Author

mqueme commented Apr 11, 2017

I. Pushed a commit for this

@mqueme mqueme removed the pending-feedback PR's and issues that are awaiting feedback from the author label Apr 12, 2017
@@ -58,7 +59,7 @@ Mautic.addReportRow = function (elId) {
prototypeHolder.append(output);

var newColumnId = '#' + elId + '_' + index + '_column';

console.log(newColumnId);
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

If this debug line will be removed, I'll give this PR thumbs up :)

@escopecz escopecz added the pending-feedback PR's and issues that are awaiting feedback from the author label Apr 13, 2017
@mqueme
Copy link
Contributor Author

mqueme commented Apr 14, 2017

We are changing this completely.

@mqueme mqueme removed the pending-feedback PR's and issues that are awaiting feedback from the author label Apr 17, 2017
@mqueme
Copy link
Contributor Author

mqueme commented Apr 17, 2017

removed and changes made

@escopecz escopecz removed the ready-to-test PR's that are ready to test label Apr 18, 2017
@mqueme mqueme added the ready-to-test PR's that are ready to test label Apr 18, 2017
Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

I could not find any problem now. All works find and groups/counts correctly! Thanks! 👍

@escopecz escopecz added pending-test-confirmation PR's that require one test before they can be merged needs-documentation PR's that need documentation before they can be merged labels Apr 19, 2017
@dongilbert
Copy link
Member

+1 Thanks @mqueme

@dongilbert dongilbert merged commit d4650dd into mautic:staging Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature for inclusion in minor or major releases needs-documentation PR's that need documentation before they can be merged pending-test-confirmation PR's that require one test before they can be merged ready-to-test PR's that are ready to test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants