Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Fixes Issue 467 #538

Closed
wants to merge 1 commit into from
Closed

Fixes Issue 467 #538

wants to merge 1 commit into from

Conversation

kaungst
Copy link

@kaungst kaungst commented Jun 29, 2016

Fixes #467

Problem Summary

  • A payment model is created when hitting the Add Payment button on invoices.
  • This model is invalid, but it's creation sets the paidTotal to 0.
  • This set's status to 'Paid' for invoices requiring 0 payment.

Fix Summary

  • request adds additional logic to where the status field is set
    • keeps track of last unpaid status if it needs to return to it
      (e.g. return to either Bill or Draft)
  • fixes missing checks for invalid payments when calculating remaining
    balance
  • implements cleanup function to destroy payment model placeholder
    upon modal exit with invalid model state.

A `payment` model is created when hitting the `Add Payment` button on
invoices. This model is invalid, but it's creation sets the `paidTotal`
to 0. This set's `status` to 'Paid' for invoices requiring 0 payment.

- request adds additional logic to where the `status` field is set
  - keeps track of last unpaid status if it needs to return to it
    (e.g. return to either Bill or Draft)
- fixes missing checks for invalid payments when calculating remaining
  balance
- implements cleanup function to destroy payment model placeholder
  upon modal exit with invalid model state.
cleanup: function() {
var model = this.model;
this.model.validate().then(function() {
model.save();
Copy link
Member

Choose a reason for hiding this comment

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

This cleanup code should not save the model because the model should not be saved on the cancel action, it should only be saved when the user clicks on the update/add action which is already handled elsewhere in the controller.

@jkleinsc
Copy link
Member

jkleinsc commented Jul 7, 2016

@kaungst Thanks for the PR. I made some comments in your code changes.

@jkleinsc jkleinsc added the in progress indicates that issue/pull request is currently being worked on label Jul 7, 2016
@tangollama
Copy link
Member

@kaungst, hi. Are you planning to address this PR further or should we close it?

@tangollama
Copy link
Member

It looks like this issue has been addressed in pr #654. @jkleinsc can you confirm and close if so?

@jkleinsc
Copy link
Member

@tangollama yup. Closing this one.

@jkleinsc jkleinsc closed this Apr 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
in progress indicates that issue/pull request is currently being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Billed Invoices - Buttons disappear from the invoice
3 participants