-
Notifications
You must be signed in to change notification settings - Fork 99
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
Adding an update method for debtor group API #185
Conversation
@@ -84,6 +87,54 @@ function create(req, res, next) { | |||
} | |||
|
|||
/** | |||
* PUT /debtor_groups/:uuid | |||
* | |||
* @exemple |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice comments! For future reference, the JSDoc tag name is @example
, not @exemple
. :).
@mbayopanda, just two comments and then LGTM. Your integration tests fail with this error: error: [ERROR] Error: ER_NO_REFERENCED_ROW_2: Cannot add or update a child row: a foreign key constraint fails (`bhima_staging`.`debitor_group`, CONSTRAINT `debitor_group_ibfk_4` FOREIGN KEY (`price_list_uuid`) REFERENCES `price_list` (`uuid`) ON DELETE CASCADE ON UPDATE CASCADE) Perhaps the price list uuid isn't properly configured. Happy debugging! |
@jniles this line below (in server/models/update/synt.sql) are responsible of the fail at travais : DROP TABLE IF EXISTS price_list; It's delete all data in price_list table which are in server/models/test/data.sql |
@jniles I think the best practice is simply to write in a single place for data : in server/models/test/data.sql and for schema in server/models/schema.sql |
@mbayopanda, thanks for the feedback! I just created an issue (#186) about where to put schema versus data. I'll implement this change tomorrow morning and open a PR. |
@jniles We need to touch price list test (priceList.js) for fixing some expect statements. |
.catch(next) | ||
.done(); | ||
|
||
function findDebtorGroup(rows) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better!
@mbayopanda, it looks like these changes broke the priceList tests. Go ahead and update them so that we have passing tests. It should be simple: changing lines like this one to expect the correct number of price lists. LGTM! |
8e5118e
to
8a4cef1
Compare
@jniles could you review this PR |
LGTM. |
Adding an update method for debtor group API
3010: Update snyk to the latest version 🚀 r=jniles a=greenkeeper[bot] ## Version **1.90.1** of **snyk** was just published. <table> <tr> <th align=left> Dependency </th> <td> <a target=_blank href=https://github.com/snyk/snyk>snyk</a> </td> </tr> <tr> <th align=left> Current Version </th> <td> 1.90.0 </td> </tr> <tr> <th align=left> Type </th> <td> dependency </td> </tr> </table> The version **1.90.1** 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. It might be worth looking into these changes and trying to get this project onto the latest version of snyk. If you have a solid test suite and good coverage, a passing build is a strong indicator that you can take advantage of these changes directly by merging the proposed change into your project. If the build fails or you don’t have such unconditional trust in your tests, this branch is a great starting point for you to work on the update. --- <details> <summary>Release Notes</summary> <strong>v1.90.1</strong> <p>Bumps various deps:</p> <ul> <li>bump snyk-config to fix env merge issue</li> <li>bump go plugin to update doc/typos</li> <li>bump nuget plugin to get rid of an unneeded dep</li> <li>bump python plugin to fix pipenv monitoring issue</li> <li>bump sbt plugin to update 'debug' dep version</li> </ul> </details> <details> <summary>Commits</summary> <p>The new version differs by 12 commits.</p> <ul> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/e14ab9eaf5ba6cc66751f3fc2803d54d354b6b6c"><code>e14ab9e</code></a> <code>Merge pull request #185 from snyk/fix/bump-deps</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/c6735e4f4ef5a44b9ae158687682193da284f3f8"><code>c6735e4</code></a> <code>Merge pull request #184 from snyk/chore/github-release</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/ace19f7394a9500d34bdb2247a6d75e78601cf5a"><code>ace19f7</code></a> <code>Merge pull request #183 from snyk/chore/eslint</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/f31602440e9740e73c1c8b659bd396ba4ceb29ac"><code>f316024</code></a> <code>fix: bump sbt plugin to update 'debug' dep version</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/2c79a4efd4580e251decbfa4587a5fd86f9e5155"><code>2c79a4e</code></a> <code>fix: bump python plugin to fix pipenv monitoring issue</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/975ca1ca1198c0eda61d60b586ce3d3702287e8e"><code>975ca1c</code></a> <code>fix: bump nuget plugin to get rid of an unneeded dep</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/828d579ff27d582635c1b1db4b15f50331c5d91f"><code>828d579</code></a> <code>fix: bump go plugin to update doc/typos</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/be8fa57983cf566c280d29d91a0d28814f14625e"><code>be8fa57</code></a> <code>fix: bump snyk-config to fix env merge issue</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/b638a37c093ba7200da94fc5424bd2816874ca35"><code>b638a37</code></a> <code>chore: eslint instead of jscs</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/0bfeb0b06df49b9acd67a2bb27993313546d138c"><code>0bfeb0b</code></a> <code>chore: fix github-release for assets uploading</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/9312a048c1d27021b52a90d335a8f68d0a7f9db5"><code>9312a04</code></a> <code>Merge pull request #181 from snyk/chore/semantic-release</code></li> <li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/6abdfd957d07337b86e0ba3aafb1e219e1083665"><code>6abdfd9</code></a> <code>chore: upgrade semantic-release, proper travis & appveyor setup</code></li> </ul> <p>See the <a href="https://urls.greenkeeper.io/snyk/snyk/compare/2a6938fe92377c99fe4991f0159a1e32e6e58834...e14ab9eaf5ba6cc66751f3fc2803d54d354b6b6c">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>
This PR add an update methode "PUT" for the debtor group API