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

Fix giving and receiving amounts #2718

Merged
merged 19 commits into from
Sep 23, 2014
Merged

Fix giving and receiving amounts #2718

merged 19 commits into from
Sep 23, 2014

Conversation

Changaco
Copy link
Contributor

@Changaco Changaco commented Sep 2, 2014

Fixes #1127, and a few other bugs (including the 4th item of #2630).

@Changaco Changaco mentioned this pull request Sep 2, 2014
6 tasks
@Changaco Changaco force-pushed the fix-receiving-amounts branch 2 times, most recently from 12dfb39 to a887ba2 Compare September 3, 2014 11:33
@Changaco
Copy link
Contributor Author

Changaco commented Sep 3, 2014

Fixed a bug and improved the tests.

@Changaco
Copy link
Contributor Author

Changaco commented Sep 8, 2014

Added a test and a fix for the computation of actual team takes that is used in the /%team/members/ pages.

@chadwhitacre
Copy link
Contributor

Rebased on master.

FROM tips
ORDER BY tipper, tippee, mtime DESC;

-- Allow updating is_funding via the current_tips view for convenience
Copy link
Contributor

Choose a reason for hiding this comment

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

s/is_funding/is_funded

@Changaco Changaco force-pushed the fix-receiving-amounts branch from 6a6d169 to a06884b Compare September 10, 2014 15:12
@chadwhitacre
Copy link
Contributor

Okay, so after a first pass through, it looks like basically what we're doing here is pushing is_funded down from the temp tables used during payday into the tips table itself. Yes?

@Changaco Changaco force-pushed the fix-receiving-amounts branch from fb5abef to 6d5ab04 Compare September 10, 2014 18:17
@Changaco
Copy link
Contributor Author

Ready for final review.

@chadwhitacre
Copy link
Contributor

IRC

@chadwhitacre
Copy link
Contributor

What if we had is_funded in its own table and modified the current_tips view to join on that?

@Changaco
Copy link
Contributor Author

I don't see how that would solve anything, nor why we're still talking about this.

@chadwhitacre
Copy link
Contributor

@Changaco It would solve the problem of having information with an undefined meaning in our database. We're still talking about this because having undefined information in our database does not seem good to me.

@chadwhitacre
Copy link
Contributor

I'm also not settled on using a fake payday during the real payday.

@chadwhitacre
Copy link
Contributor

If we relaxed the null constraint and maintained true/false only on the most recent tip, with the rest being null, I'd be satisfied with that.

@chadwhitacre
Copy link
Contributor

I mean, that seems like a potential source of bugs to me. I guess we could add a self-check for that.

@Changaco
Copy link
Contributor Author

NULL values are a source of subtle SQL bugs, which is why I added the NOT NULL constraint after you suggested it.

@chadwhitacre
Copy link
Contributor

Rebased on master.

chadwhitacre added a commit that referenced this pull request Sep 23, 2014
@chadwhitacre chadwhitacre merged commit 4449d15 into master Sep 23, 2014
@chadwhitacre chadwhitacre deleted the fix-receiving-amounts branch September 23, 2014 20:15
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.

compute "receiving" more accurately
2 participants