IMA-WorldHealth / bhima-1.X

A hospital information system for developing countries.
GNU General Public License v2.0
10 stars 14 forks source link

Payslip employee with directive #795

Closed mbayopanda closed 9 years ago

mbayopanda commented 9 years ago

In this PR we have used Angular Directive for the "Multi Payroll" ui

jniles commented 9 years ago

This is a good start.

I noticed you do not create a link function or controller. It would be nice to encapsulate the HTTP request to the server within this directive. That way the parent controller does not have to worry about the details of engaging an employee, it can just download the correct data, create all the employee payslip directives, and concern itself with changing the currency.

The directive should have a controller with a submit() method which sends the HTTP request to approve payment for the employee. It should perform any data validation necessary before sending the HTTP request. If the request succeeds, the directive should call the onSubmit() method that was passed to it with true as the only parameter. If the HTTP request fails, call onSubmit(false).

That way, the parent controller doesn't have to do any of the work, except for configuration! All of the logic are contained in the directive.

jniles commented 9 years ago

After some discussion, it doesn't seem like a directive is the appropriate way forward for this module, unless we rethink how payroll happens. Thanks for the effort though!