-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Conversation
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.
@tangollama there are JSCS (formatting) errors that need to be cleared up before this PR can be accepted. Also, can you merge in the latest from master?
tests/acceptance/invoices-test.js
Outdated
@@ -66,6 +66,57 @@ test('print invoice', function(assert) { | |||
}); | |||
}); | |||
|
|||
//test pricing profile |
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.
@tangollama your code is throwing JSCS (formatting) errors on test. You can verify these yourself by running ember test:
acceptance/invoices-test.js should pass jscs.
requireSpaceAfterLineComment: Missing space after line comment at acceptance/invoices-test.js :
67 |});
68 |
69 |//test pricing profile
-------------------^
70 |test('pricing profiles', function(assert) {
71 | runWithPouchDump('billing', function() {
requireSpaceAfterLineComment: Missing space after line comment at acceptance/invoices-test.js :
77 | waitToAppear('h4:contains(New Pricing Profile)');
78 | });
79 | //% discount
------------------^
80 | andThen(function() {
81 | fillIn('.pricing-profile-name input', '50% profile');
disallowTrailingWhitespace: Illegal trailing whitespace at acceptance/invoices-test.js :
80 | andThen(function() {
81 | fillIn('.pricing-profile-name input', '50% profile');
82 | fillIn('.pricing-profile-percentage input', '50');
----------------------------------------------------------------^
83 | click('button:contains(Add)');
84 | waitToAppear('button:contains(Ok)');
requireSpacesInFunction: Missing space before opening curly brace at acceptance/invoices-test.js :
85 | click('button:contains(Ok)');
86 | });
87 | andThen(function(){
------------------------------^
88 | click('button:contains(+ new item)');
89 | waitToAppear('h4:contains(New Pricing Profile)');
requireSpaceBeforeBlockStatements: One (or more) spaces required before opening brace for block expressions at acceptance/invoices-test.js :
85 | click('button:contains(Ok)');
86 | });
87 | andThen(function(){
------------------------------^
88 | click('button:contains(+ new item)');
89 | waitToAppear('h4:contains(New Pricing Profile)');
requireSpaceAfterLineComment: Missing space after line comment at acceptance/invoices-test.js :
89 | waitToAppear('h4:contains(New Pricing Profile)');
90 | });
91 | //flat discount
--------------------^
92 | andThen(function() {
93 | fillIn('.pricing-profile-name input', '$100 discount');
disallowTrailingWhitespace: Illegal trailing whitespace at acceptance/invoices-test.js :
92 | andThen(function() {
93 | fillIn('.pricing-profile-name input', '$100 discount');
94 | fillIn('.pricing-profile-discount input', '100');
---------------------------------------------------------------^
95 | click('button:contains(Add)');
96 | waitToAppear('button:contains(Ok)');
requireSpaceBeforeBlockStatements: One (or more) spaces required before opening brace for block expressions at acceptance/invoices-test.js :
97 | click('button:contains(Ok)');
98 | });
99 | andThen(function(){
------------------------------^
100 | click('button:contains(+ new item)');
101 | waitToAppear('h4:contains(New Pricing Profile)');
requireSpacesInFunction: Missing space before opening curly brace at acceptance/invoices-test.js :
97 | click('button:contains(Ok)');
98 | });
99 | andThen(function(){
------------------------------^
100 | click('button:contains(+ new item)');
101 | waitToAppear('h4:contains(New Pricing Profile)');
requireSpaceAfterLineComment: Missing space after line comment at acceptance/invoices-test.js :
101 | waitToAppear('h4:contains(New Pricing Profile)');
102 | });
103 | //flat fee
-----------------^
104 | andThen(function() {
105 | fillIn('.pricing-profile-name input', '$150 fee');
disallowTrailingWhitespace: Illegal trailing whitespace at acceptance/invoices-test.js :
104 | andThen(function() {
105 | fillIn('.pricing-profile-name input', '$150 fee');
106 | fillIn('.pricing-set-fee input', '150');
------------------------------------------------------^
107 | click('button:contains(Add)');
108 | waitToAppear('button:contains(Ok)');
Log: |
app/models/price-profile.js
Outdated
@@ -19,6 +20,11 @@ export default AbstractModel.extend({ | |||
numericality: { | |||
allowBlank: true | |||
} | |||
} | |||
}, |
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.
Travis is failing tests due to formatting issues. Please run ember test to check the formatting issues locally.
not ok 736 Chrome 53.0 - JSCS - models/price-profile.js: should pass jscs
---
actual: >
false
expected: >
true
stack: >
at Object.<anonymous> (http://localhost:7357/assets/tests.js:25353:12)
at runTest (http://localhost:7357/assets/test-support.js:2995:28)
at Object.run (http://localhost:7357/assets/test-support.js:2980:4)
at http://localhost:7357/assets/test-support.js:3143:11
at process (http://localhost:7357/assets/test-support.js:2819:24)
at begin (http://localhost:7357/assets/test-support.js:2801:2)
message: >
models/price-profile.js should pass jscs.
disallowTrailingWhitespace: Illegal trailing whitespace at models/price-profile.js :
26 | allowBlank: true
27 | }
28 | }
-------------^
29 | }
30 |});
Log: |
# Conflicts: # app/models/invoice.js # app/pricing/profiles/edit/template.hbs
# Conflicts: # app/locales/en/translations.js
@jkleinsc in an effort to close out old PR's, I made some corrections here. If this checks out with Travis, I recommend that we move this PR into master. |
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.
@tangollama thanks for the PR. There are a couple minor tweaks needed to move this forward.
app/locales/en/translations.js
Outdated
@@ -362,6 +362,7 @@ export default { | |||
} | |||
}, | |||
labels: { | |||
currency_symbol: '$', |
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.
this label is unused and also doesn't following naming standards. Should be currencySymbol (if it was used).
app/locales/en/translations.js
Outdated
@@ -385,6 +386,7 @@ export default { | |||
action: 'Action', | |||
notes: 'Notes', | |||
edit: 'Edit', | |||
expense_to: 'Expense To', |
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.
This label should be expenseTo
, not expense_to
app/locales/en/translations.js
Outdated
discountAmount: 'Discount Amount', | ||
discountPercentage: 'Discount Percentage' | ||
}, | ||
messages: { | ||
flatFeeMsg: 'There is a flat fee for patient financial responsibility of {{currency}}{{setFee}}.', |
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.
Looks like currency and currency_symbol are the same thing?
app/models/invoice.js
Outdated
@@ -98,6 +98,26 @@ export default AbstractModel.extend(DateFormat, NumberFormat, { | |||
|
|||
patientResponsibilityTotals: Ember.computed.mapBy('lineItems', 'amountOwed'), | |||
patientResponsibility: Ember.computed.sum('patientResponsibilityTotals'), | |||
finalPatientResponsibility: function() { |
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.
Use
finalPatientResponsibility: computed('patientResponsibility', 'paymentProfile', function() {
...
})
instead of
finalPatientResponsibility: function() {
...
}.property('patientResponsibility', 'paymentProfile'),
@@ -7,5 +7,6 @@ | |||
{{em-input property="name" label="Name" class="required pricing-profile-name"}} | |||
{{number-input property="discountPercentage" label="Discount Percentage" class="pricing-profile-percentage"}} | |||
{{number-input property="discountAmount" label="Discount Amount" class="pricing-profile-discount"}} | |||
{{em-input property="setFee" label="Set Fee" class="pricing-set-fee"}} |
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.
label="Set Fee"
should be localized
# Conflicts: # app/locales/en/translations.js
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.
This has been satisfied. I need someone else to approve.
Addresses #35 by adding a new field to payment profiles, considering it (and amountDiscount) in the calculation of Invoices as well as finally creating some tests for payment profiles.
For your consider mr. @jkleinsc