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.

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

Several fixes to export reports #7872

Merged
merged 1 commit into from Mar 18, 2020
Merged

Several fixes to export reports #7872

merged 1 commit into from Mar 18, 2020

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Sep 11, 2019 •
edited

Please be sure you are submitting this against the staging branch.

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

Description:

While I worked on https://github.com/mautic/mautic/pull/7871/files I noticed few inconsistent issues with export results. The main issue was with regular columns and aggregated columns.

Steps to reproduce the bug:

  1. Create basic report. For example page hits with URL, date_hit and COUNT date_hit. You should see basic report

image

  1. Export to CSV - shoud works OK
  2. Export to HTML - should see just two columns

image

  1. Export to excel - should see wrong export

image

  1. Try also order report by Count date_hit column - should return nothing or 500 error (check logs)

Steps to test this PR:

  1. Load up this PR
  2. Repeat all steps to reproduce. Should works good

@kuzmany kuzmany added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels Sep 11, 2019
@kuzmany kuzmany added this to the 2.16.0 milestone Sep 11, 2019
@npracht npracht added this to Ready to Test (first time) in Mautic 2 Oct 3, 2019
@npracht npracht added T2 Medium difficulty to fix (issue) or test (PR) reports Anything related to reports labels Oct 4, 2019
@kuzmany kuzmany mentioned this pull request Oct 11, 2019
Closed
@mautibot
Copy link

This pull request has been mentioned on Mautic Community Forums. There might be relevant details there:

https://forum.mautic.org/t/is-there-a-reason-that-contact-id-will-not-be-exported-from-form-submits/11248/7

RCheesley
RCheesley approved these changes Oct 16, 2019
View reviewed changes
Copy link
Sponsor Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Confirmed the problems mentioned when creating the report and exporting.

Applied the PR and the correct exports were downloaded, HTML has all columns, and the filters I tested with seem to work fine.

@mautic mautic deleted a comment from mautibot Oct 16, 2019
@npracht npracht removed this from the 2.16.0 milestone Jan 23, 2020
@RCheesley RCheesley added this to the 2.16.1 milestone Mar 9, 2020
@npracht npracht added this to Ready to test in Mautic 2 Mar 10, 2020
@RCheesley RCheesley added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Mar 10, 2020
@RCheesley RCheesley moved this from Ready to test to Needs a second test/review in Mautic 2 Mar 10, 2020
dennisameling
dennisameling approved these changes Mar 18, 2020
View reviewed changes
Copy link
Member

@dennisameling dennisameling left a comment

Choose a reason for hiding this comment

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

Code looks good and since @RCheesley tested successfully already, I'm approving and merging this PR for 2.16.1. Thanks @kuzmany! 🎉

@dennisameling dennisameling merged commit 7869433 into mautic:staging Mar 18, 2020
Mautic 2 automation moved this from Needs a second test/review to Merged Mar 18, 2020
@mautibot
Copy link

This pull request has been mentioned on Mautic Community Forums. There might be relevant details there:

https://forum.mautic.org/t/announcing-mautic-2-16-1-beta/13438/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RCheesley RCheesley RCheesley approved these changes

@dennisameling dennisameling dennisameling approved these changes

Assignees
No one assigned
Labels
bug Issues or PR's relating to bugs pending-test-confirmation PR's that require one test before they can be merged reports Anything related to reports T2 Medium difficulty to fix (issue) or test (PR)
Projects
No open projects
Mautic 2
  
Merged
Milestone
2.16.1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@kuzmany @mautibot @RCheesley @dennisameling @npracht