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

Add balance sheet report #734

Merged
merged 34 commits into from
Oct 11, 2023
Merged

Add balance sheet report #734

merged 34 commits into from
Oct 11, 2023

Conversation

o-psi
Copy link
Contributor

@o-psi o-psi commented Oct 7, 2023

Summary:

This PR brings several key updates to the IT Flow project, including the introduction of a Balance Sheet report and enhancements to account management.

Changes:

Balance Sheet Report: Introduced a new screen for balance sheet reports. This report is also accessible via the side navigation (report_balance_sheet.php).

Account Types: Added a dropdown for selecting account types (Current Assets, Fixed Assets, etc.) in the add and edit account modals (account_add_modal.php and account_edit_modal.php).

Updated Accounts Screen: Accounts now display their associated types (accounts.php).

New Account Types: Accounts now have an Account Type, and account types are editable in the settings.

Database Updates:

Added a new account_type field to the accounts table.
Database version incremented to 0.8.7.
Code Refactoring: Added comments for better code readability and future development.

Other Backend Changes:

Updated post/account.php to accommodate the new account_type field.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello & Welcome! :)

Thanks for taking the time to help improve ITFlow. We're excited to review your contributions - we'll review this PR as soon as we can!

Whilst you're waiting, please feel free to check out the forum.

Just so you know, all contributions to ITFlow are licensed under the GNU GPL. By contributing you grant us a perpetual & irrevocable license to include your work in ITFlow.

@wrongecho
Copy link
Collaborator

Test these changes at: https://balancesheet734.pr-review.itflow.org
(automatic message)

@o-psi o-psi changed the title Balance sheet Add balance sheet report Oct 8, 2023
echo "
<tr>
<td>$account_type_string</td>
<td><a class=\"text-dark\" href=\"account_details.php?account_name=$account_name_encoded\">{$accountRow['account_name']}</a></td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • All non-int variables should be echoed/assigned using nullable_htmlentities to prevent XSS

  • Ideally (where possible) we try to assign the variable once (e.g. as $account_type and $account_name and then just re-use as required), rather than constantly reusing $accountRow['account_type'] - its much cleaner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been resolved. Looking over the rest of the code now.

@wrongecho
Copy link
Collaborator

Heyy @o-psi! Thanks for the PR :)

I've made a few suggestions, but will leave the rest to Johnny. I know he's quite busy recently with client projects though so it may be a few days before he has chance to take a look.

@o-psi o-psi marked this pull request as draft October 8, 2023 21:21
@o-psi o-psi marked this pull request as ready for review October 9, 2023 20:40
@o-psi o-psi marked this pull request as draft October 9, 2023 20:40
@o-psi o-psi marked this pull request as ready for review October 9, 2023 21:25
@o-psi
Copy link
Contributor Author

o-psi commented Oct 9, 2023

@wrongecho @johnnyq I believe this PR is ready to ship. If we can do one of those fancy test deployments to test from clean.

@wrongecho wrongecho closed this Oct 9, 2023
@wrongecho wrongecho reopened this Oct 9, 2023
@wrongecho
Copy link
Collaborator

Test these changes at: https://balancesheet734.pr-review.itflow.org
(automatic message)

@wrongecho
Copy link
Collaborator

Test these changes at: https://balancesheet734.pr-review.itflow.org
(automatic message)

Wellllll... Seems I didn't really account for PRs with different database changes with the same version number.

I thought a close and reopen would start it fresh but seems not - even with a manual webhook redelivery for the PR Open event :/

@o-psi
Copy link
Contributor Author

o-psi commented Oct 9, 2023

@wrongecho It seems like you didnt account for bozos like me to be messing with your DevOps hahah.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug B 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 30 Code Smells

No Coverage information No Coverage information
6.5% 6.5% Duplication

@johnnyq
Copy link
Collaborator

johnnyq commented Oct 10, 2023

This one looks exciting @o-psi Good work and my accountant will be happy, It looks like it may be ready any discrepancies @o-psi and @wrongecho

@o-psi
Copy link
Contributor Author

o-psi commented Oct 11, 2023

I am having a meeting with my accountant today to see if she needs or wants anything else for this PR, and to ensure it is all working according to accrual basis accounting.

I also wanted to ask @johnnyq if there was any reason there is not an account balance field in the DB. Each time its calculated in the frontend, it should be logged and recorded in the DB. This may be a post 1.0 feature.

@johnnyq
Copy link
Collaborator

johnnyq commented Oct 11, 2023

I am having a meeting with my accountant today to see if she needs or wants anything else for this PR, and to ensure it is all working according to accrual basis accounting.

I also wanted to ask @johnnyq if there was any reason there is not an account balance field in the DB. Each time its calculated in the frontend, it should be logged and recorded in the DB. This may be a post 1.0 feature.

I love the idea of the account_balance being in the db. This makes sense so PHP doesn't have to do so much leg work, we just didn't think of it at the time, So I'm totally for it

@johnnyq
Copy link
Collaborator

johnnyq commented Oct 11, 2023

im going to merge this in for now

@johnnyq johnnyq merged commit 44671d4 into itflow-org:master Oct 11, 2023
3 checks passed
johnnyq added a commit that referenced this pull request Oct 11, 2023
johnnyq added a commit that referenced this pull request Oct 11, 2023
@johnnyq
Copy link
Collaborator

johnnyq commented Oct 11, 2023

I made some code changes after merging this PR and something still isn't right with the calculations
We use expenses and revenues tables for transfers as well
a transfer expense is if the expense_vendor_id = 0 and a transfer revenue is if revenue_category_id = 0
so I modified the SQL Query to exclude those expenses and and revenues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants