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.

Sign up for GitHub

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

Jump to bottom

Fixes, Round 5 #253

Merged
merged 4 commits into from
Aug 4, 2016
Merged

Fixes, Round 5 #253

merged 4 commits into from
Aug 4, 2016

Conversation

mistergone
Copy link
Member

@mistergone mistergone commented Aug 4, 2016 •
edited
Loading

This PR fixes two issues:

  • Fixes a bug where the expenses view failed to update when the financial numbers were changed
  • Fixes the "overborrowing" value when a user has borrowed more than the costs

Additions

  • local recalculation of overborrowing to include tuition payment plans
  • a fixed call to expensesView.updateView() in financialView

Changes

  • updates student-debt-calc to 2.5.8
  • Changes the borrowing error message to use the "overborrowing" value, which has been fixed in student-debt-calc to 2.5.8

Testing

  • Passes unit tests
  • Fails some functional tests (awaiting calculation confirmations before fix)

Review

Todos

  • Updated related functional tests

Checklist

  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the front end playbook
  • Passes all existing automated tests
  • New functions include new tests
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged
  • Visually tested in supported browsers and devices
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)

@mistergone
This commit fixes a bug with the expenses section failing to update a…
2506c24 
…fter a

recalculation by adding a call to expensesView.updateView() in the appropriate
place.
@mistergone
This commit does the following:
1f88bf5 
- update student-debt-calc to 2.5.8 to fix overborrowing issue
- adds internal fix to add tuition payment plans to overborrowing
- re-shrinkwraps
@mistergone
Vulnerability patched
0556a92
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0556a92 on fixes5 into 6542742 on master.

@marteki
Copy link
Member

marteki commented Aug 4, 2016 •
edited
Loading

Before review: I will remove my assignment to the overborrowing task, and delete the code I was actively working on, then. 😆

EDIT: Never mind, this isn't addressing the broken error messaging to the user about overborrowing. I'll carry on!

@mistergone
Copy link
Member Author

Sorry - I didn't realize I didn't self-assign, and I can't connect to the internal issues list right now. 😢

@marteki
Copy link
Member

marteki commented Aug 4, 2016

Check my edit. S'all good.

@marteki
Copy link
Member

marteki commented Aug 4, 2016

Fixes a bug where the expenses view failed to update when the financial numbers were changed

What's the issue that this is addressing? I'm not sure how to verify that it's working properly. Post-grad budget worksheet?

marteki
marteki reviewed Aug 4, 2016
View reviewed changes
src/disclosures/js/models/financial-model.js
@@ -47,6 +47,7 @@ var financialModel = {
this.values = recalculate( this.values );
this.sumTotals();
this.roundValues();
console.log( this.values );
Copy link
Member

Choose a reason for hiding this comment

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

Hello.

@mistergone
Copy link
Member Author

@marteki - Internally, the bug is "Post Grad Budget Calculation"

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 852377f on fixes5 into 6542742 on master.

@higs4281
Copy link
Member

higs4281 commented Aug 4, 2016

👍 over-borrowing content looks good

@mistergone
Copy link
Member Author

@marteki - I pushed a patch for the console.log() of shame and a few other linting issues.

@marteki
Copy link
Member

marteki commented Aug 4, 2016

👍

@mistergone
Copy link
Member Author

Merging!

@mistergone mistergone merged commit 51c12a3 into master Aug 4, 2016
@mistergone mistergone deleted the fixes5 branch August 5, 2016 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mistergone @coveralls @marteki @higs4281