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

NEW add Opening Balance column to balance.php #13067

Merged
merged 1 commit into from
Feb 14, 2020
Merged

NEW add Opening Balance column to balance.php #13067

merged 1 commit into from
Feb 14, 2020

Conversation

maboelfotoh
Copy link

Add Opening Balance column to balance.php where colspans and line break widths are adjusted accordingly.

@maboelfotoh maboelfotoh requested a review from eldy February 9, 2020 11:08
@eldy
Copy link
Member

eldy commented Feb 10, 2020

Here we are losing an important feature making the report grouped by account.
Can you make this an option from a parameter so we can switch between this view and the old behavior ? (lilke it is done for the ledger page with button "Group by account")

@eldy eldy added the PR to fix or conflict to solve PR needs to be fixed to be integrated (except for conflicts, a comment describes the fix to do) label Feb 10, 2020
@maboelfotoh
Copy link
Author

maboelfotoh commented Feb 10, 2020

Under Ledger (accountancy/bookkeeping/list.php) I do see the button 'Group by finance account', whereas under the Account balance page (accountancy/bookkeeping/balance.php) I only see the 'EXPORT (CSV)' button. Is there supposed to be a 'Group by account' button there too?
balance
(I do acknowledge that removing the line break would break sorting by 'Financial account')
Thanks.

Copy link
Member

@eldy eldy left a comment

Choose a reason for hiding this comment

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

The button "group by" is not yet. My suggestion is to add it so we won't loose the existing feature.

@maboelfotoh
Copy link
Author

maboelfotoh commented Feb 13, 2020

The button "group by" is not yet. My suggestion is to add it so we won't loose the existing feature.

If you may, kindly provide a screenshot of the "existing feature" that would break by adding an Opening Balance column. So far, I tested balance.php by sorting by 'Financial Account', as well as search filter options and those work without issues.

Alternatively, if you would rather have this new version of balance.php as a separate page, say openingbalance.php, and have a button from balance.php that links to openingbalance.php (passing over search filter parameters), then that's also fine with me.

Thank you.

@maboelfotoh maboelfotoh reopened this Feb 13, 2020
@maboelfotoh
Copy link
Author

Re-implemented this as a separate view that can be toggled, similar to how the Ledger page views (list.php and listbyaccount.php) are implemented. Thanks.

@eldy eldy merged commit 58a2a71 into Dolibarr:develop Feb 14, 2020
@eldy
Copy link
Member

eldy commented Feb 14, 2020

So, i misunderstood what was the goal of your PR. I though you move the breakdown of accounting account as a simple column, this is why i guide you to this change. I finally revert your PR to take last version - 1.

eldy added a commit that referenced this pull request Feb 14, 2020
@maboelfotoh
Copy link
Author

maboelfotoh commented Feb 14, 2020

So, i misunderstood what was the goal of your PR. I though you move the breakdown of accounting account as a simple column, this is why i guide you to this change. I finally revert your PR to take last version - 1.

The merge didn't take the commit.
I actually reverted my commit before committing the re-implementation as balance.php and openingbalance.php. So there is now one commit (one version), where balance.php has a button that links to openingbalance.php, and vice versa. In addition to your suggestion, I figured this would keep the balance.php view as-is for those who do not want the extra "Opening Balance" column.

balance.php screenshot:
balance php

openingbalance.php screenshot:
openingbalance php

maboelfotoh pushed a commit to maboelfotoh/dolibarr that referenced this pull request Nov 25, 2020
eldy added a commit that referenced this pull request Nov 25, 2020
FIX #13067 including opening balance in calculation of displayed balance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR to fix or conflict to solve PR needs to be fixed to be integrated (except for conflicts, a comment describes the fix to do)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants