Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Fix fake payday depth #2988

Merged
merged 8 commits into from
Dec 5, 2014
Merged

Fix fake payday depth #2988

merged 8 commits into from
Dec 5, 2014

Conversation

rohitpaulk
Copy link
Contributor

No description provided.

@techtonik
Copy link
Contributor

Is there a human friendly description of the problem that this PR is aimed to solve?

@rohitpaulk
Copy link
Contributor Author

hey @techtonik - Sorry about that. This is just something that we forgot to implement in #2914. The problem we're trying to solve is #2664.

funded_tips = self.db.all("SELECT amount FROM tips WHERE is_funded ORDER BY id")
assert funded_tips == [3, 6, 1, 4, 10, 5]
assert funded_tips == [50, 10, 30, 70, 51, 5]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you change so many things in this test ? From reading the diff I can't tell what you're trying to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test was kinda messy because we had added stuff over time to check for various situations. I didn't want to add one more participant into the equation, so I spent some time on restructuring to make sure that we check for all the following cases:

  • tips from previously failing CCs are not funded
  • payday goes beyond 2 steps
  • team takes are included in the calculations
  • make sure funds are not transferred to closed accounts

@Changaco
Copy link
Contributor

Changaco commented Dec 5, 2014

@rohitpaulk I've reverted your changes to the test and then fixed it again but this time without changing all the tip amounts, the diff is a lot smaller. Does it look okay to you ?

@rohitpaulk
Copy link
Contributor Author

@Changaco - This doesn't check whether the depth is > 2. Replace the first tip loop with the old statement

UPDATE temp_tips t SET is_funded = true WHERE is_funded IS NOT true;

and the test will pass, but it shouldn't.

@Changaco
Copy link
Contributor

Changaco commented Dec 5, 2014

@rohitpaulk Your version had the same problem. I've added a separate test for depth.

@rohitpaulk
Copy link
Contributor Author

Cool, going to merge this in :)

rohitpaulk added a commit that referenced this pull request Dec 5, 2014
@rohitpaulk rohitpaulk merged commit 0e49279 into master Dec 5, 2014
@Changaco Changaco deleted the fix-fake-payday-depth branch December 5, 2014 17:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants