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

(Report) Stock entry report #3724

Merged
merged 5 commits into from
Jun 28, 2019
Merged

Conversation

mbayopanda
Copy link
Contributor

This PR adds the stock entry report.

image

@kwilu kwilu added the size: XL label May 13, 2019
@jniles
Copy link
Contributor

jniles commented May 14, 2019

@jeremielodi, can you give this a review?

@jniles jniles requested a review from jeremielodi May 14, 2019 08:03
@kwilu
Copy link

kwilu commented May 14, 2019

Heads up @mbayopanda, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

@mbayopanda
Copy link
Contributor Author

@jeremielodi what is the status of the review of this PR ?

@mbayopanda
Copy link
Contributor Author

@jeremielodi can i get a review ?

Copy link
Contributor

@jniles jniles left a comment

Choose a reason for hiding this comment

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

@mbayopanda just a few comments here. The code is really clean - well done!

"SHOW_DETAILS": "Show Details"
"SHOW_DETAILS": "Show Details",
"ENTRY_REPORT": "Stock Entry Report",
"ENTRY_REPORT_DESCRIPTION": "this report displays the different stock entries that took place in depots",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's capitalize the first letter in this sentence: "This report ..."

const reportUrl = 'reports/stock/entry';

// default values
vm.includePurchaseEntry = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FYI - most reports put their default values in an object, usually called reportDetails. This allows us to really easily merge default values with current values using Object.assign(). Here is how that works:

var userOptions = { date : Date('2015-03-01'), includeDonation : 0 };
var defaults = { includePurchaseEntry : 1, includeDonation : 1 };

var selected = Object.assign({}, defaults, userOptions);

// selected is now:
// { date : Date('2015-03-01'), includeDonation : 0, includePurchaseEntry : 1 }

You can try it in your console!

My proposal is to wrap all these vm.include* variables in an object called vm.reportDetails. Like this:

vm.reportDetails = {
  includeIntegrationEntry : 0,
  // etc...
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you want to change this? I'm fine either way.

SUM(m.quantity) as quantity, m.date, m.description,
u.display_name AS user_display_name, sup.display_name AS supplier_display_name,
dm.text AS document_reference, d.text AS depot_name,
CONCAT_WS('.', '${identifiers.PURCHASE_ORDER.key}', proj.abbr, p.reference) AS purchase_reference
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use dm.text instead of this CONCAT_WS? We noticed some bugs at HEV where the project wasn't correctly assigned in a few cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, i don't think so, because CONCAT_WS is for getting the purchase reference, there is no relation between purchase and stock movement, dm.text is the reference of the stock movement, CONCAT_WS is for the purchase reference

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, what I mean is just do another JOIN on the document_map table on p.uuid. Call it dm2 or something.

If someone changes the project identifier, it will change this value. We should probably keep this value static so that it always matches what is printed out on the receipts.

@@ -134,9 +134,6 @@ BEGIN
END $$
DELIMITER ;

DELETE FROM role_unit WHERE unit_id = 171;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this get removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have this unit in master

@kwilu
Copy link

kwilu commented Jun 15, 2019

Heads up @mbayopanda, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

@mbayopanda
Copy link
Contributor Author

@jniles could you review this PR again please

Copy link
Contributor

@jniles jniles left a comment

Choose a reason for hiding this comment

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

@mbayopanda I've just clarified one point. Feel free to merge this anytime.

SUM(m.quantity) as quantity, m.date, m.description,
u.display_name AS user_display_name, sup.display_name AS supplier_display_name,
dm.text AS document_reference, d.text AS depot_name,
CONCAT_WS('.', '${identifiers.PURCHASE_ORDER.key}', proj.abbr, p.reference) AS purchase_reference
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, what I mean is just do another JOIN on the document_map table on p.uuid. Call it dm2 or something.

If someone changes the project identifier, it will change this value. We should probably keep this value static so that it always matches what is printed out on the receipts.

const reportUrl = 'reports/stock/entry';

// default values
vm.includePurchaseEntry = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you want to change this? I'm fine either way.

@jniles
Copy link
Contributor

jniles commented Jun 28, 2019

bors r+

@jniles jniles closed this Jun 28, 2019
@bors
Copy link
Contributor

bors bot commented Jun 28, 2019

Already running a review

@jniles jniles reopened this Jun 28, 2019
@jniles
Copy link
Contributor

jniles commented Jun 28, 2019

bors r+

bors bot added a commit that referenced this pull request Jun 28, 2019
3724: (Report) Stock entry report r=jniles a=mbayopanda

This PR adds the stock entry report.

![image](https://user-images.githubusercontent.com/5445251/57624961-dc17e180-758a-11e9-87a8-d751ef0d866a.png)

Co-authored-by: mbayopanda <mbayopanda@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jun 28, 2019

Build succeeded

@bors bors bot merged commit 5f0dc18 into IMA-WorldHealth:master Jun 28, 2019
@jniles jniles mentioned this pull request Aug 20, 2019
bors bot added a commit that referenced this pull request Aug 20, 2019
3853: Release v1.2.0 r=jniles a=jniles

🚀 Let's release bhima (currently at 1.1.1)


Changelog:
* chore: use release-it for releases (f4efd33)
* chore: rename migration path (37fe3b4)
* chore: dump dependencies before release (492ac19)
* Merge #3846 (c2f27c8)
* Merge #3849 (7a41977)
* bug(Cash box translation) - Fix untranslation key for user deactivated (b0f4fbe)
* Merge #3836 (f20d459)
* refactor(Make Exchange Rates human readable) - Addition of the minimum value of the exchange rate to prevent the   exchange rate equal to zero - Show exchange rates less than 0.1 as a fraction for example 1/1630   instead of 0.0006134 (c21e24a)
* Merge #3829 (28fdd28)
* test: run client unit tests on AppVeyor (a1c8968)
* chore: refactor testing code (64b3c96)
* test: remove server-unit from TravisCI (0357ec2)
* test: add appveyor to bors (b790a45)
* ci: add initial appveyor support (c235a97)
* Merge #3840 (8d3010a)
* fix: bump unit ids in the migration file (9bf73ee)
* chore(package): update eslint-config-airbnb-base to version 14.0.0 (6fbde6a)
* Merge #3834 (8774b09)
* handle negative values in stock report (57380f1)
* Merge #3833 (217d0d1)
* add translation in employee report (aee91f6)
* Merge #3832 (3578b7f)
* fix(Calcul Sum Distributed) - Around the sum distributed for escape error when percent as decimal (30dbc83)
* Merge #3799 (852b220)
* clean code (95f1e93)
* improvement(Employee Standing) - clean the representation of tabSQL - Capitalize creditoropeningbalance (01893e9)
* Sanitaze code (7268cad)
* improvement(Employee Standing) - Fix and Upgrate the translation key - Adding opening balance until a date from - Adding new function openingBlanceCreditor with parameters like   creditorUuid and datefrom (3588ff8)
* Sanitaze code (b914bca)
* refactor(Employee Standing) - Refactor and Sanitaze code (e115407)
* improvement(Improve Employees Standings report) - Add the possibility to visualize the employees standings by range of   date, by the adding of component date by bh-date-interval - Add filter by range of date (f4c4774)
* Merge #3827 (f1a7e6f)
* Merge #3828 (19f0753)
* fix(tests): no async forEach in merge patients (c4b257a)
* feat(report) debtor summary report (a835fb3)
* chore: update depends (b78ab2a)
* Merge #3825 (1326302)
* Merge #3794 (7ae875f)
* (cron job) add cron email componet to main reports (438b1ac)
* improvement(Invoicing Bug) - Restoring the old debtor balance request and modifying the   hasCreditorBalance calculation by analyzing whether the balance is   greater than 0.01 (b289a4e)
* Merge #3815 (d115442)
* improve(Monthly Balance) - Rename Monthly Balance instead Monthly Balance Analysis - Sanitaze code and refactor Report Design (0ae1f6a)
* Merge #3820 (64497cc)
* improvement(MonthlyBalance) - Sanitaze code and improve report and translate key (58cb1d0)
* Sanitaze code (1167990)
* Sanitaze code (9455a3d)
* feature(Monthly Balance Analysis) - Add new report for Analysis Monthly Balance of accounts (06d9f60)
* Merge #3801 (87edd98)
* remove toggle required (7c0f583)
* Improve and sanitaze code (b613b29)
* fix (Invoicing Bug) - resolution of the problem when calling the Debtors.balance function with rounding to two ranks after the decimal point of the total credit and debit values (1edfd7b)
* rename cashboxId to cashboxAccountId (f895692)
* fix component require condition for validation (00869ad)
* Enhance caution filter and fix cashflow by service report (a6bfc8b)
* Merge #3821 (2973736)
* fix stock adjustment deletion (ab9db1d)
* Sanitaze html form (3494521)
* improvement(Cash Flow Reports) - Adding Possibilty to view the detailed cashflow report (946e278)
* Merge #3813 (21d19d3)
* chore(package): update cz-conventional-changelog to version 3.0.0 (6c941a5)
* Merge #3808 (34fe9fc)
* Merge #3809 (f5d7fb2)
* chore(package): update commitizen to version 4.0.0 (2dfc6cb)
* chore(package): update gulp-if to version 3.0.0 (6af28e8)
* Merge #3797 (cda7c86)
* chore(package): update karma-chrome-launcher to version 3.0.0 (d3be817)
* Merge #3795 (0d5adeb)
* fix cron table on migration (8ad64ba)
* Merge #3767 (84b5af0)
* Merge remote-tracking branch 'upstream/master' into auto-email-reporting (ada6cd6)
* Merge #3775 (ce8af95)
* Merge #3785 (f50b9c6)
* fix e2e tests for cron emailing (e39aa2a)
* Merge #3777 (e30235b)
* fix(Payslip) - Restaure the management of off days in payslip - fix the displaying of daily rate for Holidays periods and offdays - Exted the periode of definition of offdays (713c4bf)
* Merge #3782 (14ae5ba)
* fix: .snyk & package.json to reduce vulnerabilities (b3c8d1a)
* Merge #3779 (47ba601)
* Merge #3780 (ad5115c)
* fix integration test (cd531cb)
* chore(package): update lockfile yarn.lock (3ef4a2d)
* chore(package): update del to version 5.0.0 (72c68d7)
* fix label index length (45e6dc3)
* improve(Employee Standing) - Adding option to include medical care provided to the employee - Update Report, display Employee Standing Status in Report closes #3778 (331d9c1)
* fix column length (bf3259c)
* fix: restaure old file (e2e1e4a)
* feature(Bhima User) - Complete Bhima User Manual (18a0514)
* fix entity parent (852144d)
* update report id length (842b864)
* fix: migration script typo (bf1157a)
* fix translations and unit test for bhMoment (8109ab7)
* fix migration file (e1edfba)
* dev environment (bb2b5ba)
* fix ui and integration tests for cron email report (d68d383)
* instant email send for reports (2bbd820)
* ohada_balance_sheet, ohada_income_expense, exploitation, balance emailing (ea4fac0)
* Form reset after submission and cron job fixing (7ab8b44)
* handle deletion and job stop (2101165)
* balance report by email and use of async await (b9cec59)
* sending email (af4b449)
* cron job for email report (54fc906)
* load report details (c8c9115)
* simple crud for cron email report (46d7a34)
* contact group management (f3c040f)
* feature(Bhima User Manual) - References accounts User Manual - Fee Center User Manual - Payroll User Manual (0aacc6a)
* Merge #3772 (78a657a)
* fix: rubrics config tests (c31beee)
* fix: flaky role management test (90a6fcb)
* Merge #3768 (a18c9f5)
* Merge #3770 (c65a8d5)
* chore: bump dependencies (96207a2)
* Merge #3724 (b244207)
* fix purchase reference (5f0dc18)
* Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima into report-stock-entry (efc5e43)
* chore(package): update lockfile yarn.lock (c0f80d7)
* chore(package): update eslint to version 6.0.0 (e90a2e7)
* fix stock sheet report (ef42f3b)
* Capitalize report description and use of reportDetails correctly (0af0ec8)
* stock entry report (9b7f65c)
* Merge #3762 (d2bc25a)
* Merge branch 'master' into fix-depot-path (75ca9e2)
* Merge #3760 (c13c5a4)
* fix depot path (18d2eee)
* feat(component) bhperiod, use period translate_key as label (d061ecc)
* Merge #3759 (b7cc0d3)
* Merge #3736 (f95255b)
* Merge #3751 (30ea36d)
* add warning message for employees patients merge (3236947)
* Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima into merge-patient-records (ff4f7da)
* chore(package): update lockfile yarn.lock (aef66b2)
* chore(package): update mochawesome to version 4.0.0 (21fe8dd)
* Merge #3752 (9880ae8)
* Merge #3757 (fc69b92)
* update action link column (200d0a7)
* updates integration and e2e tests (ce256bc)
* Split hospitalization indicators by projects (18d659b)
* count services by project (995a746)
* add project in service (8a9b6bd)
* Merge #3755 (752723a)
* Merge #3756 (480f324)
* fix: typos in migration script setup (3c9ed3e)
* Reset Numbers of vouchers in E2E test (f3345be)
* Restore number of vouchers in Ui grid (d9319af)
* Reset defaut number of vouchers (dde4bb0)
* test(Voucher Registry) - Update number of vouchers (6b7683a)
* improvement(Payroll Configuration) - Update the max length of abbreviation of rubrics - remove unused require - Before we had to prevent the edition of all the headings which were   taxes, but it can be made that there are taxes which is not calculated   expressed as a percentage, to allow the edition of this kind of tax, it   would be enough to exclude the only tax which is expressed as a   percentage neither and which is not nor éditable reason why we had   excluded the IPR - closes #3754 (b70ef8d)
* improvement(Payroll) - Sanitaze code remove unused require employees - Use the date to instead the current days for commitment of paiement in   accounting (c62c98f)
* Merge #3753 (80e2607)
* fix(invoice): primary key collision invoicing_fees (df82dad)
* fix selection list (b6681a6)
* Merge #3743 (1a3d919)
* reload page before each tests (a939788)
* fix price list report (b4471b5)
* integration tests and e2e tests of patients merge fetaure (93bd991)
* merge patients tool (a6ca2de)
* merge patients (5dc2a0b)
* chore(package): update merge-stream to version 2.0.0 (e8005ec)
* Merge #3732 (54212b3)
* Merge #3733 #3734 (b2eec0b)
* chore: combine date/username lines on receipt head (52fdbdb)
* Merge #3735 (b68bd3f)
* Merge #3737 (4886914)
* Merge #3739 (f2b3983)
* feat(unpaid invoice) add filter by service (e14ed21)
* fix indicator report missing value (4bfac9a)
* fix: parse <head> contents from XLS renderer (f15481a)
* fix: dismiss fiscal loading indicator once loaded (6b5bd8f)
* fix: account reference dropdown header links (f40a358)
* fix: display correct currency in account footer (9642e6f)
* Merge #3730 (c33886f)

Co-authored-by: Jonathan Niles <jonathanwniles@gmail.com>
@mbayopanda mbayopanda deleted the report-stock-entry branch September 27, 2019 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants