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

Adding print functionality #669

Closed

Conversation

lomamech
Copy link
Collaborator

Adding print functionality

Update bhBreadcrumb.js component for adding property print
Update template for breadCrumb with the using of bh-breadcrumb
Implement patient invoice report in PDF
Update inventory list, voucher registry for Prevent modale windows
Adding route for manage invoice / patient / report

@jniles

project : req.session.project
};

let invoiceListQuery =
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code is the exact same code as the patientInvoice list() function. Since it is a duplication, I propose that you use the same function.

I propose that you make a new function in the patientInvoice file called listInvoices(). You could then write:

// GET /invoices
function list(req, res, next) {
  listInvoices()
  .then(function (invoices) {
    res.status(200).json(invoices);
  })
  .catch(next)
  .done();
}

// GET /invoices/patient/report
function getPatientInvoice(req, res, next) {

  const request = { /* some values */ };

  listInvoices()
  .then(invoices => listReceipt.build(invoices, request):
   // ... etc ...
  })
  .catch(next)
  .done();
}

Remember, in software engineering, we don't like to repeat ourselves.

@jniles
Copy link
Collaborator

jniles commented Aug 26, 2016

@lomamech I've completed an initial code review. Thanks for taking the time to work all this out.

Here are some additional comments outside the code.

  1. When I use the application, I notice some things that are poor quality. For example, the buttons are different sizes. See below:

    voucherbuttons
    Fig 1: Different Sized Buttons?
    Please make all the buttons the same size. I propose you remove the btn-sm classes in the bhBreadCrumb component.

  2. The two reports, Invoice Registry and Voucher Registry, are in two different orientations. They both contain about the same amount of information. Why are they in two different orientations? I propose that they both be in portrait orientation. See below:

    invoicesreport
    Fig 2: Landscaped Invoices
    voucherreport
    Fig 3: Portrait Vouchers

Let me know if you need any clarification or when you need another review.

@jniles jniles added the Report label Aug 29, 2016
@lomamech lomamech force-pushed the printFunctionality branch from 699372b to f7a047d Compare August 31, 2016 14:08
@lomamech
Copy link
Collaborator Author

  • Using the function listInvoices in controllers/finance/patientInvoice.js
  • Put currency_id in filter for the currency
  • Sanitaze the reports invoices
  • Sanitaze code
  • Fix the size of button
  • Update the orientation remplace landscape by portrait
  • Resolve conflict with branch Master

@jniles

@lomamech
Copy link
Collaborator Author

lomamech commented Sep 2, 2016

Hello @jniles
All tests integrations and end to end work very well
Let me know what works not
Thank you

@jniles
Copy link
Collaborator

jniles commented Sep 2, 2016

@lomamech, the tests do not pass on Node versions less than v6.

The error you are receiving is due to missing 'use strict' in one of the files you changed to use let or const. See this answer.

jniles referenced this pull request in jniles/bhima Sep 5, 2016
This commit fixes the strict mode errors on lower node versions.

Closes #669.
@jniles jniles mentioned this pull request Sep 5, 2016
4 tasks
jniles referenced this pull request in jniles/bhima Sep 6, 2016
This commit fixes the strict mode errors on lower node versions.

Closes #669.
@jniles jniles closed this in #682 Sep 6, 2016
@lomamech lomamech deleted the printFunctionality branch May 15, 2017 12:16
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.

2 participants