refactor: Posting Journal and Associated Tools #2085

jniles · 2017-09-09T12:02:41Z

This Pull Request finalizes the refactor of the Posting Journal and associated modules. The commit history contains a full list of changes. In brief:

Editing has been moved out into a modal. By breaking out the editing code, the complexity of the Posting Journal is reduced. It also enforces editing a single transaction at a time.
The Trial Balance has moved into Stored Procedures. This separates the JS code from the SQL code and speeds up the execution of the Trail Balance slightly.
The Trial Balance no longer reloads data between $state changes. The modal is much more snappy than previous.
Links to the receipts/documents/patients associated with each transaction are embedded in the Posting Journal. This makes it easy to find the details associated with a given transaction.
The Posting Journal is able to load "posted" transactions. To accommodate this, the Posting Journal footer has been improved to show the number of unposted or posted transactions. Additionally, the "Posted Journal" module has been removed.

There have been a number of miscellaneous bug fixes addressed as well. As with any change, this may introduce new bugs specific to the changes listed above.

Partially addresses #1432.

Closes #1034.
Closes #1163.
Closes #1402.
Closes #1500.
Closes #1640.
Closes #1659.
Closes #1716.
Closes #1717.
Closes #1724.
Closes #1832.
Closes #1839.
Closes #1921.
Closes #1928.
Closes #1934.
Closes #1943.
Closes #1944.
Closes #1948.
Closes #1950.
Closes #1961.
Closes #2031.
Closes #2041.

TODO Before Merge

Ensure all keys are translated into both French and English
Ensure tests pass
Make sure all filters/links work on Posting Journal page.
Make sure all filters/links work on the Trial Balance
Ensure all editors work as expected.
Ensure that new code passes lint checks.

DedrickEnc · 2017-09-12T15:02:03Z

@jniles
This PR needs a rebase first

lomamech · 2017-09-19T11:12:47Z

Error analysis in the new version of the Input Journal

When you want to edit a transaction, the check box that precedes the transaction ID does not activate the Edit button, but only if you click on a line in the transaction
Validation functions do not seem to work well

Non-balanced transactions are validated
It is possible to edit on one line of a transaction at the same time the debit as well as the credit
It is also possible to validate a line of a transaction from the null values to the debit but also to the credit

jniles · 2017-09-19T11:56:02Z

I believe the lack of validation was by design from @sfount. I think the motivation was to make it really easy for a user to edit a transaction, but the posting process would still ensure that the transaction's rules were respected.

That being said, I would personally be more comfortable with stricter validation. I'll write some more checks before this is merged.

Your first critique is a bug. Good catch! I never tested that path. It may be related to #2041.

mbayopanda · 2017-09-19T12:04:28Z

Refactor Journal Tools – Assessment

1. The comment tool has bug :

Generate an error when click on submit
The comment doesn’t succeed

2. Selecting a group of transactions doesn’t select children transactions

3. The text “Transactions Vérifiées” must clearly refer to the posted transactions

The text can just be “Transactions Postées”

4. The cash payment receipt has bug

Generate an error

5. The edit tool is not strict in terms of validation

The good thing is that the trial balance modal blocks well bad transactions

jniles · 2017-09-19T12:36:05Z

@mbayopanda, thanks for the feedback!

I cannot reproduce the cash payment error. Can you give me steps to reproduce?

The problem with UI Grid selection was reported in #2041. The main problem is that we are fighting ui-grid on this one - we will be changing default behavior if we try to hack this. What do you think the best option is?

lomamech · 2017-09-19T12:39:06Z

I think that the description of a transaction should be the same for all the lines of the transposition, so this property should be defined only once

lomamech · 2017-09-19T13:24:26Z

I discovered an error with respect to updating TPA18, TPA19, TPA20, TPA21 and TPA22 transactions with the bhima_test database, all of these transactions were only initializing with a single Accounts number, but the problem is that when trying to edit any of these transactions, the modal displays 60lines whatever transaction you choose

But also, if one modifies for example the date, this modification will repercute on the other transactions

jniles · 2017-09-19T13:30:12Z

@lomamech, that actually isn't an issue with this module - it was an issue with the posting procedure for Purchase Orders. It was fixed in #2104, but I haven't updated that branch to master yet.

jniles · 2017-09-19T14:28:59Z

@lomamech, @mbayopanda thanks for your feed back!

I have pushed some fixes that should address everything except validation - could you check out the branch and see if those fixes work as you expect?

lomamech · 2017-09-19T15:04:18Z

@jniles, somes issues are fixed, but in regard to the description, it would have to be defined at once as for the date, or else it should be maintained like this

jniles · 2017-09-20T08:10:18Z

@lomamech, you are right. The description is allowed to be different on every line by design. We actually change the description for things like billing services in invoices. There's also a suggestion to put the inventory item description on each line of an invoice or cash payment (#1824). For this reason, it is best to keep the ability to change each individual line's description.

jniles · 2017-09-20T15:01:07Z

Alright, all feedback including validation has been addressed.

@DedrickEnc @mbayopanda can I get a code review?

mbayopanda · 2017-09-22T10:22:10Z

@jniles

There are some new bugs found :

Cannot post data into the general ledger

Cannot remove a comment

This behavior is also in account statement, we can only add and update a comment to a row, but we cannot remove completely a comment.

jniles · 2017-09-22T10:30:42Z

@mbayopanda thanks for these catches! I guess the comment PR wasn't properly vetted :(

I'll put some patches in by this afternoon.

- This PR makes it possible to put the period as well as the year whenever the date changes and prevent the modification of the date for closed or inexistent fiscal years - Implement of function lookupFiscalYearByDate for check the FiscalYear and period for one Date -

- Fix the comment for the description of function - Put all alteration of data in the function transformColumns() - rename the name of function selectTransdate

This commit makes sure that the posting journal's data is in the correct period.

This commit rounds out the posting journal tools by translating all the keys that are used. It also prunes the `posting_journal.json` file to find keys that are not used.

This reworks journal.js so that it passes lint checks. It also removes many of the old imports.

The previous journal edit code only considered edited lines to update the fiscal_year_id on. This behavior threw errors when new lines were added to a transaction. The errors are fixed by updating the editing code to work on both new lines and old lines.

This commit cleans up the lint issues in the transaction edit modal of the Posting Journal.

This commit improves the UX by passing the full account label (human readable label) to the Posting Journal filters. The account is now properly shown instead of just the number.

Fixes the currencies to be right-aligned instead of left-aligned. It also ensures footer values are defined before rendering "NaN" and defaults to "0".

This commit fixes a broken reference that prevented commenting the Posting Journal.

This commit right-aligns the currencies in the edit modal footer.

This commit fixes the PDF report by combining posted and unposted report routes into one and the same route.

@mbayopanda

This commit fixes various miscellaneous usability errors and typos in the journal, trial balance, and transaction editing process. Specifically: 1. Better French translation in the Journal Search Modal (thanks @mbayopanda) 2. Description is automatically copied onto new modal line when adding a new lint (thanks @lomamech) 3. Caught a typo that prevented commenting from working (@mbayopanda). 4. Caught a bug that prevents editing a row when clicking on its group header (@lomamech).

This commit adds "offline" validation to the transaction modal, which was the no 1 requested item on the feedback from developers. The intent is not to be comprehensive, but to prevent users from making silly mistakes.

This commit updates the journal with editing tests for transactions. These tests ensure that the validation works in the modal.

This commit allows the journal to remove comments. Additionally, the comment modal has been improved such that, if a unique value is used for a comment, it is displayed in the input. This makes the component feel more like it belongs on the page.

This commit renames the Posting Journal and subsequent translations to "Journal". It also removes "Posted Journal" from the database. Closes #2031

This commit fixes a few typos, consolidates a few CSS rules and removes unused code. It fixes a bug that disallowed posted records from being commented, as this path was never tested before.

jniles · 2017-09-25T11:58:47Z

@mbayopanda, changes landed!

mbayopanda · 2017-09-25T14:33:59Z

bors r+

2085: refactor: Posting Journal and Associated Tools r=mbayopanda a=jniles This Pull Request finalizes the refactor of the Posting Journal and associated modules. The commit history contains a full list of changes. In brief: 1. Editing has been moved out into a modal. By breaking out the editing code, the complexity of the Posting Journal is reduced. It also enforces editing a single transaction at a time. 2. The Trial Balance has moved into Stored Procedures. This separates the JS code from the SQL code and speeds up the execution of the Trail Balance slightly. 3. The Trial Balance no longer reloads data between `$state` changes. The modal is much more snappy than previous. 4. Links to the receipts/documents/patients associated with each transaction are embedded in the Posting Journal. This makes it easy to find the details associated with a given transaction. 5. The Posting Journal is able to load "posted" transactions. To accommodate this, the Posting Journal footer has been improved to show the number of unposted or posted transactions. Additionally, the "Posted Journal" module has been removed. There have been a number of miscellaneous bug fixes addressed as well. As with any change, this may introduce new bugs specific to the changes listed above. Partially addresses #1432. Closes #1034. Closes #1163. Closes #1402. Closes #1500. Closes #1640. Closes #1659. Closes #1716. Closes #1717. Closes #1724. Closes #1832. Closes #1839. Closes #1921. Closes #1928. Closes #1934. Closes #1943. Closes #1944. Closes #1948. Closes #1950. Closes #1961. Closes #2031. Closes #2041. --- ### TODO Before Merge - [x] Ensure all keys are translated into both French and English - [x] Ensure tests pass - [x] Make sure all filters/links work on Posting Journal page. - [x] Make sure all filters/links work on the Trial Balance - [x] Ensure all editors work as expected. - [x] Ensure that new code passes lint checks.

bors · 2017-09-25T15:08:26Z

Build succeeded

continuous-integration/travis-ci/push

jniles added Journal Client Core performance Refactor labels Sep 9, 2017

jniles requested a review from DedrickEnc September 11, 2017 18:25

jniles force-pushed the refactor-journal-tools branch 2 times, most recently from cd74780 to 367a213 Compare September 18, 2017 14:57

jniles force-pushed the refactor-journal-tools branch from 367a213 to cdf72f4 Compare September 19, 2017 14:25

jniles force-pushed the refactor-journal-tools branch from cdf72f4 to 28292b9 Compare September 20, 2017 15:00

jniles requested a review from mbayopanda September 20, 2017 15:01

jniles mentioned this pull request Sep 21, 2017

feat(journals export) Download Journals as Excel #2118

Merged

jniles force-pushed the refactor-journal-tools branch 2 times, most recently from cfa522d to 594fab9 Compare September 21, 2017 14:26

jniles force-pushed the refactor-journal-tools branch 3 times, most recently from ce2faa9 to 4383cc7 Compare September 22, 2017 14:39

lomamech and others added 19 commits September 25, 2017 12:57

refactor(Editing transaction Date)

89b90e7

- Fix the comment for the description of function - Put all alteration of data in the function transformColumns() - rename the name of function selectTransdate

feat(posting jour) marking transactions as comment

8ee5c50

test(journal): fix erroneous test data

339bb40

This commit makes sure that the posting journal's data is in the correct period.

fix(i18n): translate journal modules

cbac530

This commit rounds out the posting journal tools by translating all the keys that are used. It also prunes the `posting_journal.json` file to find keys that are not used.

chore(journal): journal.js passes lint checks

eee73a8

This reworks journal.js so that it passes lint checks. It also removes many of the old imports.

chore(editModal): clean up lint issues

aa3cb45

This commit cleans up the lint issues in the transaction edit modal of the Posting Journal.

fix(trial balance): pass account label to filters

bdcd3ba

This commit improves the UX by passing the full account label (human readable label) to the Posting Journal filters. The account is now properly shown instead of just the number.

fix(journal): right-align footer currencies

a06efcf

Fixes the currencies to be right-aligned instead of left-aligned. It also ensures footer values are defined before rendering "NaN" and defaults to "0".

fix(journal): broken comment reference

a4a0c0e

This commit fixes a broken reference that prevented commenting the Posting Journal.

fix(journal): right align edit modal footer

413f5af

This commit right-aligns the currencies in the edit modal footer.

fix(journal): allow PDF printing of posted txns

67c3b7e

This commit fixes the PDF report by combining posted and unposted report routes into one and the same route.

feat(journal): add transaction validation

3872263

This commit adds "offline" validation to the transaction modal, which was the no 1 requested item on the feedback from developers. The intent is not to be comprehensive, but to prevent users from making silly mistakes.

test(journal): update editing tests

76825d2

This commit updates the journal with editing tests for transactions. These tests ensure that the validation works in the modal.

fix(journal): comments can be removed

9bd7a36

This commit allows the journal to remove comments. Additionally, the comment modal has been improved such that, if a unique value is used for a comment, it is displayed in the input. This makes the component feel more like it belongs on the page.

chore(journal); rename Posting Journal to Journal

ecc163d

This commit renames the Posting Journal and subsequent translations to "Journal". It also removes "Posted Journal" from the database. Closes #2031

fix(journal): typos and commenting on posted recs

fbfdacc

This commit fixes a few typos, consolidates a few CSS rules and removes unused code. It fixes a bug that disallowed posted records from being commented, as this path was never tested before.

jniles force-pushed the refactor-journal-tools branch from 4383cc7 to fbfdacc Compare September 25, 2017 11:57

bors bot merged commit fbfdacc into master Sep 25, 2017

jniles deleted the refactor-journal-tools branch September 25, 2017 15:09

This was referenced Sep 26, 2017

Link all "Record", "Reference" and "Entity" fields from Account Statement #2148

Closed

Posting Journal Editing - Assign recipient/ reference #1082

Closed

jniles mentioned this pull request Oct 15, 2017

Rewrite Trial Balance End to End Tests #2201

Closed

jniles mentioned this pull request Oct 23, 2017

Unskip Posting Journal End to End Tests #2235

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Posting Journal and Associated Tools #2085

refactor: Posting Journal and Associated Tools #2085

jniles commented Sep 9, 2017 •

edited

Loading

DedrickEnc commented Sep 12, 2017

lomamech commented Sep 19, 2017

jniles commented Sep 19, 2017 •

edited

Loading

mbayopanda commented Sep 19, 2017

jniles commented Sep 19, 2017

lomamech commented Sep 19, 2017

lomamech commented Sep 19, 2017 •

edited

Loading

jniles commented Sep 19, 2017

jniles commented Sep 19, 2017

lomamech commented Sep 19, 2017

jniles commented Sep 20, 2017 •

edited

Loading

jniles commented Sep 20, 2017

mbayopanda commented Sep 22, 2017

jniles commented Sep 22, 2017

jniles commented Sep 25, 2017

mbayopanda commented Sep 25, 2017

bors bot commented Sep 25, 2017

refactor: Posting Journal and Associated Tools #2085

refactor: Posting Journal and Associated Tools #2085

Conversation

jniles commented Sep 9, 2017 • edited Loading

TODO Before Merge

DedrickEnc commented Sep 12, 2017

lomamech commented Sep 19, 2017

jniles commented Sep 19, 2017 • edited Loading

mbayopanda commented Sep 19, 2017

jniles commented Sep 19, 2017

lomamech commented Sep 19, 2017

lomamech commented Sep 19, 2017 • edited Loading

jniles commented Sep 19, 2017

jniles commented Sep 19, 2017

lomamech commented Sep 19, 2017

jniles commented Sep 20, 2017 • edited Loading

jniles commented Sep 20, 2017

mbayopanda commented Sep 22, 2017

jniles commented Sep 22, 2017

jniles commented Sep 25, 2017

mbayopanda commented Sep 25, 2017

bors bot commented Sep 25, 2017

Build succeeded

jniles commented Sep 9, 2017 •

edited

Loading

jniles commented Sep 19, 2017 •

edited

Loading

lomamech commented Sep 19, 2017 •

edited

Loading

jniles commented Sep 20, 2017 •

edited

Loading