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

Feature: Posting Journal Server Core #320

Merged

Conversation

jniles
Copy link
Collaborator

@jniles jniles commented Apr 15, 2016

This pull request introduces a core set of queries and APIs for posting data to the posting journal. A simplified example is given for journal vouchers, though it is lacking proper handling of gain/loss on exchange. This feature is a TODO for all journal vouchers and cash transactions.

In contrast to the bhima-1.X posting journal, this posting journal uses transaction objects to ensure data integrity and moves all logic into SQL. This ensures that we have accurate error handling, and a really clean interface for writing SQL statements. The downside, is that each posting function (for vouchers, sales, etc) must set a few local variables before calling the core.setup() method. I think this is a worthwhile trade off, but am wiling to hear other opinions.

This PR removes all of the previous posting journal code, in favor of using a small set of utilities. These utilities are found in journal/core.js. They include both error handing and setting SQL variables.

Closes #313.


Hi! Thank you for contributing!

Before submitting this pull request, please verify that you have:

  • Run your code through JSHint. Check out our styleguide
  • Run integration tests.
  • Run end-to-end tests.
  • Accurately described the changes your are making in this pull request.

For a more detailed checklist, see the official review checklist
that this PR will be evaluated against.

Ensuring that the above checkboxes are completed will help speed the review process
and help build a stronger application. Thanks!

This commit updates the `voucher` table's schema to be more reminiscent of
the posting journal schema.  It adds in `entity_uuid`, and
`document_uuid` to the `voucher_item`, to allow greater flexibility in
attaching references to payments and other entities.  The integration
tests have been updated to reflect this change.

Closes #280.
This commit introduces code to post journal vouchers to the posting
journal, though no data integrity checks have been included yet.

Here are the steps involved in posting journal vouchers:
 1. The voucher controller imports the `journal/voucher` module
 2. The `voucher` journal controller is called with a transaction object and
 the binary uuid of the voucher record.
 3. The `voucher` journal controller sets up some posting session
 variables using the journal `core.js`.  These are:
  1. @enterprise
  2. @project
  3. @fiscalId
  4. @periodId
  5. @currencyId
  6. @eXchange
 4. The journal controller sets up migration from the `voucher` table to
 the `posting_journal` using the above variables.

The voucher integration tests have been updated accordingly.
This commit adds the PostingJournalErrorHandler stored procedure to the
procedures.sql file.  This ensures that all necessary SQL variables are
defined, and properly sends SQL SIGNAL errors if they are not,
terminating the transaction (and triggering a rollback).

The next step is to make these errors intelligible to the user via
translation keys.
This commit removes the application's dependency on `req.codes`
completely, following the merger of #317.  The `req.codes` can now be
completely removed from interceptors.

It also skips the patient invoice tests while the journal is broken.
This commit tests and edge case where the journal voucher will not have
a fiscal year, an error state thrown in the database and reformatted in
JavaScript.  It shows that the posting process does proper error
handling.

I've also cleaned up the comments for documentation purposes.

Closes #313.
This commit skips broken end to end tests introduced by the huge posting
journal refactor.
@jniles
Copy link
Collaborator Author

jniles commented Apr 16, 2016

@sfount once this lands, someone else could build the posting procedure from patientInvoice. I'll work on the cash posting procedure and try and sort out gain/loss on rounding.

@sfount sfount added this to the 2.x Process milestone Apr 16, 2016
@@ -30,7 +30,7 @@ describe('patient invoice', function () {
browser.get(path);
});

it('invoices a patient for a single item', function () {
it.skip('invoices a patient for a single item', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these intentionally left skipped? I assume with these should be re-implemented by the person that does the patient invoice posting core.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the patient invoice module relies on journal to post, who's code I completely removed. This skip was introduced last minute in d1cab30. Whoever works on the patient invoice module will be responsible for implementing this route and unskipping the test (as well as possibly writing a few more) to prove that this whole procedure works.

@sfount
Copy link
Contributor

sfount commented Apr 16, 2016

This code looks good to me 👍 - potentially something as core as this would benefit from a wiki entry showing the flow of data from request through to database records/ error handling. I would suggest this could be done after this has been used across a number of the core modules. (Developer focussed documentation - just so when we come back to this in a few months the idea is still clear!)

Everyone should consider the use of some of the initialised variables in the core set up process as @jniles mentioned in his comments - I'm sure this will be iterated on over time.

As I have worked on patient invoicing in the past I will take the procedure for patient invoicing, it will have to consider subsidies and billing services as well - in 2.x these are completely separate the from the invoice database record.

@sfount sfount merged commit 2a0b148 into Third-Culture-Software:master Apr 16, 2016
@jniles jniles deleted the feature-server-journal-core branch April 16, 2016 09:45
@jniles
Copy link
Collaborator Author

jniles commented Apr 16, 2016

@sfount, I appreciate the review.

The wiki entry is a good idea. I'll work on cash and tackle rounding while you tackle patientInvoice posting with all of the considerations (billing services and subsidies), and then we'll have much more experience to write a "best practices" wiki entry.

bors bot added a commit that referenced this pull request Jan 3, 2019
3513: Update snyk to the latest version 🚀 r=jniles a=greenkeeper[bot]


## The dependency [snyk](https://github.com/snyk/snyk) was updated from `1.119.0` to `1.120.1`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v1.120.1</summary>

<h2><a href="https://urls.greenkeeper.io/snyk/snyk/compare/v1.120.0...v1.120.1">1.120.1</a> (2019-01-03)</h2>
<h3>Bug Fixes</h3>
<ul>
<li>typos and grammar cleanup (<a href="https://urls.greenkeeper.io/snyk/snyk/commit/d62e30c">d62e30c</a>)</li>
<li>update keywords for npm package (<a href="https://urls.greenkeeper.io/snyk/snyk/commit/66fbc32">66fbc32</a>)</li>
<li>use consistent word style (<a href="https://urls.greenkeeper.io/snyk/snyk/commit/c903a43">c903a43</a>)</li>
</ul>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 7 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/5938f576c782762d1cbaa9cd4d10afa7dde3b91b"><code>5938f57</code></a> <code>Merge pull request #320 from snyk/fix/typos-and-grammar</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/89ac96a27504a7f7b710dd1050cd2608da105844"><code>89ac96a</code></a> <code>Merge pull request #319 from snyk/fix/npm-package-keywords</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/c903a43930239065864bd9f6440ea890d8b617b6"><code>c903a43</code></a> <code>fix: use consistent word style</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/d62e30c42b8b397fce7bd1ef6f660fdf03aaf18d"><code>d62e30c</code></a> <code>fix: typos and grammar cleanup</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/66fbc329652bc81137f72471b3935020fc75ee6a"><code>66fbc32</code></a> <code>fix: update keywords for npm package</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/bb31229124d10845e9291e399e0fc7694c6d6329"><code>bb31229</code></a> <code>Merge pull request #317 from snyk/feat/support-docker-binaries</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/80daaf588d8892bc5fa44bf2bf02e6df39968428"><code>80daaf5</code></a> <code>feat: display fixed in version key &amp; refactor binaries issues display logic</code></li>
</ul>
<p>See the <a href="https://urls.greenkeeper.io/snyk/snyk/compare/ee43b4a59716d43a41764ce08cb0a758737c8b1b...5938f576c782762d1cbaa9cd4d10afa7dde3b91b">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴



Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
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.

2 participants