Unify job&owner-report, display more metadata... #535
Conversation
christopherlam
force-pushed
the
unify-job-owner-reports
branch
2 times, most recently
from
e4e1777
to
93edea8
Compare
|
Candidate commit to supercharge owner-report Various types of invoices / payments (fully paid, unpaid, multi-invoice "7/7/19 - 1 txn for 3 pmts", partial-payments "1/7/19 - inv-13"). Invoices->Partial payments link to the payment in register; Payment->Invoices link to the actual invoice.
|
christopherlam
force-pushed
the
unify-job-owner-reports
branch
9 times, most recently
from
55392c3
to
d7d59c6
Compare
|
Supercharged Invoice lines show associated payment details + any outstanding amount, 1 in each line.
Also: Removal of AP/AR account-selection and enable search all AP/AR accounts. If an APAR account query is null, skip it. The result means any owner can be selected for querying without needing to find corresponding APAR account.
|
christopherlam
force-pushed
the
unify-job-owner-reports
branch
5 times, most recently
from
4ad72a8
to
f08f16d
Compare
christopherlam
changed the title
christopherlam
force-pushed
the
unify-job-owner-reports
branch
8 times, most recently
from
33160da
to
789096d
Compare
There was a problem hiding this comment.
Looks generally OK, though I'm not keen on inlining large functions into other large functions to make even larger functions: It's a serious hit on readability and ability to locally reason about the code. I called out one instance as an example.
I'm assuming that in your zeal to replace gnc-numeric calls you're taking care to ensure that all scheme numbers are exact and stay that way.
Are you sure that looking at both AR and AP for all flavors of the report is the right thing to do? As long as the books are correct it should be just wasted time producing no results to search for Customer transactions in AP or Vendor ones in AR. I guess if the books aren't correct then having the report show the misplaced transactions would be helpful.
|
A lot of inlining single-caller functions have been done to reduce complexity and reduce reliance on With regards to removing AP/AR account option -- I have some doubts as well... Doesn't seem to make a lot of sense to request customer/vendor/employee/job, and also limit to one APAR account. We could limit the search to AP or AR (or both, for jobs) only according to owner-type. The only reason to enable an account option is to satisfy the register hook (i.e. from register APAR acct, click |
this will obviate the need to reverse its output.
owner-anchor-text doesn't handle jobs well.
remove too much whitespace
these are rather useful in this report
previously the report would run 1 qof-query for APAR account to generate transactions, then rerun query to obtain invoice lots. this change ensures the transaction-list originally obtained is reused to retrieve their txn->invoice->lots.
previously the report would run 1 qof-query per APAR account. this change ensures only 1 qof-query is run involving *all* APAR accounts, generating a list of transactions which is filtered using srfi-1 calls for each APAR account.
handling APAR splits is easier (and faster) than querying and handling transactions.
faster than repeatedly filtering a long splitlist
we want to show reports differently if the owner has more than 1 AP/AR account. if only 1 AP/AR account, show the table. If >1, show header-row prior to each table. therefore we need to run 2 passes through the accounts list; first to count the accounts found, second to query the length prior to each table printed.
this removes need to hard-code <br/> tags
when there's a valid owner, valid APAR accounts, but no posted invoices, show appropriate error message.
christopherlam
force-pushed
the
unify-job-owner-reports
branch
from
1211917
to
5177207
Compare
|
Merged in as commit d893723 |
This PR aims to merge
job-report.scmintoowner-report.scm.Some handling is necessary -- job always map into 1 customer/vendor, but can map into multiple invoices/bills, all using originating original invoice/bill currency.
The owner-report aims to limit search of invoices/bills into 1 AP/AR account (thereby limiting currency)... I think it could be a reasonable upgrade, later on, to remove the account option, thereby searching all APAR accounts and supporting multiple currencies.