CRM-19061 Updated Reports from sprint #8539

robbrandt · 2016-06-08T22:06:44Z

SYBUNT updated to handle line items. I tried to take care to keep previous default behavior as expected, so when someone who doesn't care about line items continues to use this report they will get the same results as always. So while there is an option to not display Contribution yearly totals, it will if neither Contribution or Line Item yearly totals are selected. There are issues with getting results using the MySQL ROLLUP modifier when filtering on both line item financial types and contribution financial types; it results in subtotals in rollup results with no reliable way to filter them out. So I opted to include hints not to do that in the Columns tab. Overall it works well.

Bookkeeping report update only needed the Financial Types options to explicitly say "line Item" Financial Types. Everywhere else in CiviCRM, if not specified it means Contribution Financial Types.

CRM-19061: Update CiviReports (starting with Lybunt) to optionally show line items

civicrm-builder · 2016-06-08T22:06:48Z

Can one of the admins verify this patch?

The meatiest change here is to the INNER JOIN, for when filtering on line item financial types. The only query in this context is to compare them with older line items rather than older contributions.

Discovered problems with previous commit; report crashed when line_amount (but not line item financial type) is filtered, and it didn't include line item stats when filtered on line items.

eileenmcnaughton · 2016-06-09T15:57:14Z

Congrats on getting this to the PR stage!!

eileenmcnaughton · 2016-06-09T15:57:24Z

test this please

colemanw · 2016-06-10T13:13:39Z

jenkins, add to whitelist

colemanw · 2016-06-10T13:46:48Z

@robbrandt Looks like there are some style inconsistencies with the code formatting. Click the "details" tab at https://test.civicrm.org/job/CiviCRM-Core-PR/9955/checkstyleResult/new/

JoeMurray · 2016-06-13T16:37:37Z

Heh Rob, as Release Manager this month, I'm trying to recruit people to help pare down the backlog of almost 100 PRs, some going back to last summer. I'm wondering if you would be able to help QA another PR if I got someone to QA this PR?

robbrandt · 2016-06-13T16:43:50Z

@JoeMurray give me a couple of weeks, I am severely overloaded at the moment because of the week "off" I had at the sprint.

robbrandt · 2016-06-13T17:03:59Z

@colemanw It appears I didn't do a reformat for Details.php after I set the editor to follow Drupal guidelines. Done now.
The issue on Details.php line 306 isn't clear to me. The warning says "A comma should follow the last multiline array item. Found: )", but that line uses a string, not an array. If there's another error there, I am blind to it.
All other errors are addressed. Do I need to do a new pull request?

eileenmcnaughton · 2016-06-13T23:35:17Z

pushing new commits to the PR (or amending existing commits & force pushing them with git push -f) will cause the tests & code checks to run again

JoeMurray · 2016-06-14T03:16:23Z

@robbrandt Thanks for your contributions on your 'vacation'!! Hopefully next month. Cheers!

JoeMurray · 2016-06-20T15:31:41Z

@robbrandt could you create JIRA issue explaining what you were intending to do with this PR? Thanks.

KarinG · 2016-07-15T01:23:54Z

jenkins test this please

KarinG · 2016-07-15T02:09:00Z

Jenkins has difficulty applying the patches in this PR - I'll try manually on my local.

KarinG · 2016-07-15T02:30:42Z

@robbrandt - I can't get this to apply cleanly either - have you recovered from the Sprint enough to find some time to rebase your work on current dmaster? Ping me and I'll QA it soon after -- KarinG

karins-macbook-pro:civicrm sysadmin$ cat 8539.patch | patch -p1 --dry-run
patching file CRM/Report/Form/Contribute/Sybunt.php
Hunk #7 succeeded at 389 (offset 1 line).
Hunk #8 FAILED at 466.
Hunk #9 succeeded at 485 (offset 2 lines).
Hunk #10 succeeded at 566 (offset 2 lines).
Hunk #11 succeeded at 575 (offset 2 lines).
Hunk #12 succeeded at 680 (offset 2 lines).
Hunk #13 succeeded at 724 (offset 2 lines).
1 out of 13 hunks FAILED -- saving rejects to file CRM/Report/Form/Contribute/Sybunt.php.rej
patching file CRM/Report/Form/Contribute/Sybunt.php
Reversed (or previously applied) patch detected!  Assume -R? [n] y
Hunk #2 FAILED at 32.
Hunk #3 succeeded at 170 with fuzz 2 (offset 5 lines).
Hunk #4 FAILED at 219.
Hunk #5 FAILED at 238.
Hunk #6 succeeded at 215 (offset -28 lines).
Hunk #7 FAILED at 228.
Hunk #8 FAILED at 287.
Hunk #9 FAILED at 299.
Hunk #10 FAILED at 330.
Hunk #11 FAILED at 360.
Hunk #12 FAILED at 414.
Hunk #13 FAILED at 438.
Hunk #14 FAILED at 650.
Hunk #15 FAILED at 694.
12 out of 15 hunks FAILED -- saving rejects to file CRM/Report/Form/Contribute/Sybunt.php.rej
patching file CRM/Report/Form/Contribute/Bookkeeping.php
patching file CRM/Report/Form/Contribute/Detail.php
Hunk #1 FAILED at 61.
Hunk #2 succeeded at 444 (offset -4 lines).
Hunk #3 succeeded at 500 with fuzz 2 (offset -10 lines).
Hunk #4 succeeded at 825 (offset -9 lines).
1 out of 4 hunks FAILED -- saving rejects to file CRM/Report/Form/Contribute/Detail.php.rej
patching file CRM/Report/Form/Contribute/Detail.php
Hunk #1 FAILED at 448.
Hunk #2 FAILED at 513.
Hunk #3 succeeded at 480 (offset -62 lines).
2 out of 3 hunks FAILED -- saving rejects to file CRM/Report/Form/Contribute/Detail.php.rej
patching file CRM/Report/Form/Contribute/Detail.php
Hunk #1 FAILED at 32.
Hunk #2 FAILED at 60.
Hunk #3 FAILED at 79.
Hunk #4 FAILED at 174.
Hunk #5 FAILED at 205.
Hunk #6 FAILED at 258.
Hunk #7 FAILED at 361.
Hunk #8 succeeded at 353 (offset -79 lines).
Hunk #9 FAILED at 364.
8 out of 9 hunks FAILED -- saving rejects to file CRM/Report/Form/Contribute/Detail.php.rej
patching file CRM/Report/Form/Contribute/Sybunt.php
Reversed (or previously applied) patch detected!  Assume -R? [n] y
Hunk #1 succeeded at 234 with fuzz 2 (offset 20 lines).
Hunk #2 FAILED at 288.
Hunk #3 FAILED at 324.
Hunk #4 FAILED at 340.
3 out of 4 hunks FAILED -- saving rejects to file CRM/Report/Form/Contribute/Sybunt.php.rej
karins-macbook-pro:civicrm sysadmin$

robbrandt · 2016-07-24T18:21:04Z

Hi. I would very much like to know what needs to be done. Not a git expert
here. Last I knew, there were formatting issues, but I set up my phpstorm
project according to CiviCRM guidelines. So I don't know what the problem
is.

Specifics would be helpful.

Rob

On Jul 14, 2016 7:30 PM, "Karin Gerritsen" notifications@github.com wrote:

@robbrandt https://github.com/robbrandt - I can't get this to apply
cleanly either - have you recovered from the Sprint enough to find some
time to rebase your work on current dmaster? Ping me and I'll QA it soon
after -- KarinG

karins-macbook-pro:civicrm sysadmin$ cat 8539.patch | patch -p1 --dry-run
patching file CRM/Report/Form/Contribute/Sybunt.php
Hunk #7 #7 succeeded at 389
(offset 1 line).
Hunk #8 #8 FAILED at 466.
Hunk #9 #9 succeeded at 485
(offset 2 lines).
Hunk #10 #10 succeeded at
566 (offset 2 lines).
Hunk #11 #11 succeeded at
575 (offset 2 lines).
Hunk #12 #12 succeeded at
680 (offset 2 lines).
Hunk #13 #13 succeeded at
724 (offset 2 lines).
1 out of 13 hunks FAILED -- saving rejects to file
CRM/Report/Form/Contribute/Sybunt.php.rej
patching file CRM/Report/Form/Contribute/Sybunt.php
Reversed (or previously applied) patch detected! Assume -R? [n] y
Hunk #2 #2 FAILED at 32.
Hunk #3 #3 succeeded at 170
with fuzz 2 (offset 5 lines).
Hunk #4 #4 FAILED at 219.
Hunk #5 #5 FAILED at 238.
Hunk #6 #6 succeeded at 215
(offset -28 lines).
Hunk #7 #7 FAILED at 228.
Hunk #8 #8 FAILED at 287.
Hunk #9 #9 FAILED at 299.
Hunk #10 #10 FAILED at 330.
Hunk #11 #11 FAILED at 360.
Hunk #12 #12 FAILED at 414.
Hunk #13 #13 FAILED at 438.
Hunk #14 #14 FAILED at 650.
Hunk #15 #15 FAILED at 694.
12 out of 15 hunks FAILED -- saving rejects to file
CRM/Report/Form/Contribute/Sybunt.php.rej
patching file CRM/Report/Form/Contribute/Bookkeeping.php
patching file CRM/Report/Form/Contribute/Detail.php
Hunk #1 #1 FAILED at 61.
Hunk #2 #2 succeeded at 444
(offset -4 lines).
Hunk #3 #3 succeeded at 500
with fuzz 2 (offset -10 lines).
Hunk #4 #4 succeeded at 825
(offset -9 lines).
1 out of 4 hunks FAILED -- saving rejects to file
CRM/Report/Form/Contribute/Detail.php.rej
patching file CRM/Report/Form/Contribute/Detail.php
Hunk #1 #1 FAILED at 448.
Hunk #2 #2 FAILED at 513.
Hunk #3 #3 succeeded at 480
(offset -62 lines).
2 out of 3 hunks FAILED -- saving rejects to file
CRM/Report/Form/Contribute/Detail.php.rej
patching file CRM/Report/Form/Contribute/Detail.php
Hunk #1 #1 FAILED at 32.
Hunk #2 #2 FAILED at 60.
Hunk #3 #3 FAILED at 79.
Hunk #4 #4 FAILED at 174.
Hunk #5 #5 FAILED at 205.
Hunk #6 #6 FAILED at 258.
Hunk #7 #7 FAILED at 361.
Hunk #8 #8 succeeded at 353
(offset -79 lines).
Hunk #9 #9 FAILED at 364.
8 out of 9 hunks FAILED -- saving rejects to file
CRM/Report/Form/Contribute/Detail.php.rej
patching file CRM/Report/Form/Contribute/Sybunt.php
Reversed (or previously applied) patch detected! Assume -R? [n] y
Hunk #1 #1 succeeded at 234
with fuzz 2 (offset 20 lines).
Hunk #2 #2 FAILED at 288.
Hunk #3 #3 FAILED at 324.
Hunk #4 #4 FAILED at 340.
3 out of 4 hunks FAILED -- saving rejects to file
CRM/Report/Form/Contribute/Sybunt.php.rej
karins-macbook-pro:civicrm sysadmin$

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8539 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAVpedsrGsqnUAO04mRV2qR6SYhTL-6Rks5qVvDUgaJpZM4Ixc7x
.

KarinG · 2016-07-24T21:15:53Z

Hi Rob, essentially the report php files you had modified have been modified by someone else before your code got merged with master - and your code no longer can be merged.

robbrandt · 2016-07-24T21:58:21Z

Thanks. But it's not clear to me if I need to do anything now. And if so, what.
On Jul 24, 2016 2:15 PM, Karin Gerritsen notifications@github.com wrote:Hi Rob, essentially the report php files you had modified have been modified by someone else before your code got merged with master - and your code no longer can be merged.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

eileenmcnaughton · 2016-07-26T09:05:55Z

Hi @robbrandt I took a look at this.

There are changes to 3 files in here

bookkeeping report - these are minor changes
CRM_Report_Form_Contribute_Detail changes - these appear to be unintentional?
changes to the Sybunt report - these are the main changes to be QA'd

What I suggest you do is create a new branch based on master (git checkout -b new_sybunt origin/master) and copy & past the raw sybunt from this PR into that branch and commit it as a single commit. Review them & check they look correct & still test well & if so do a new PR on that branch against master & close this one.

I would probably put the bookkeeping ones in a separate PR to keep the focus of the main PR on the Sybunt (and drop the changes that appear to be accidental)

robbrandt added 3 commits June 6, 2016 13:45

Update Sybunt.php

4562a26

Update Sybunt.php

52c325f

Update Bookkeeping.php

3dee86e

civicrm-builder added the master label Jun 8, 2016

Updated to handle Line Item Financial Types

de2f712

The meatiest change here is to the INNER JOIN, for when filtering on line item financial types. The only query in this context is to compare them with older line items rather than older contributions.

robbrandt changed the title ~~SYBUNT and Bookkeeping report updated~~ Updated Reports from sprint Jun 8, 2016

Update Detail.php

9f64021

Discovered problems with previous commit; report crashed when line_amount (but not line item financial type) is filtered, and it didn't include line item stats when filtered on line items.

robbrandt added 2 commits June 13, 2016 09:51

Update Detail.php

e8d4134

Update Sybunt.php

a54d99f

JoeMurray mentioned this pull request Jul 7, 2016

Invitation: CiviCRM 4.7.10 release August 2016 civicrm/release-management#4

Closed

eileenmcnaughton changed the title ~~Updated Reports from sprint~~ CRM-19061 Updated Reports from sprint Jul 12, 2016

colemanw closed this Sep 30, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CRM-19061 Updated Reports from sprint #8539

CRM-19061 Updated Reports from sprint #8539

robbrandt commented Jun 8, 2016 •

edited by civicrm-builder

Loading

civicrm-builder commented Jun 8, 2016

eileenmcnaughton commented Jun 9, 2016

eileenmcnaughton commented Jun 9, 2016

colemanw commented Jun 10, 2016

colemanw commented Jun 10, 2016

JoeMurray commented Jun 13, 2016

robbrandt commented Jun 13, 2016

robbrandt commented Jun 13, 2016

eileenmcnaughton commented Jun 13, 2016

JoeMurray commented Jun 14, 2016

JoeMurray commented Jun 20, 2016

KarinG commented Jul 15, 2016

KarinG commented Jul 15, 2016

KarinG commented Jul 15, 2016 •

edited

Loading

robbrandt commented Jul 24, 2016

KarinG commented Jul 24, 2016

robbrandt commented Jul 24, 2016

eileenmcnaughton commented Jul 26, 2016

CRM-19061 Updated Reports from sprint #8539

CRM-19061 Updated Reports from sprint #8539

Conversation

robbrandt commented Jun 8, 2016 • edited by civicrm-builder Loading

civicrm-builder commented Jun 8, 2016

eileenmcnaughton commented Jun 9, 2016

eileenmcnaughton commented Jun 9, 2016

colemanw commented Jun 10, 2016

colemanw commented Jun 10, 2016

JoeMurray commented Jun 13, 2016

robbrandt commented Jun 13, 2016

robbrandt commented Jun 13, 2016

eileenmcnaughton commented Jun 13, 2016

JoeMurray commented Jun 14, 2016

JoeMurray commented Jun 20, 2016

KarinG commented Jul 15, 2016

KarinG commented Jul 15, 2016

KarinG commented Jul 15, 2016 • edited Loading

robbrandt commented Jul 24, 2016

KarinG commented Jul 24, 2016

robbrandt commented Jul 24, 2016

eileenmcnaughton commented Jul 26, 2016

robbrandt commented Jun 8, 2016 •

edited by civicrm-builder

Loading

KarinG commented Jul 15, 2016 •

edited

Loading