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

Fix export CSV bug (#4399) #4439

Merged
merged 7 commits into from
May 4, 2017
Merged

Fix export CSV bug (#4399) #4439

merged 7 commits into from
May 4, 2017

Conversation

rohitpaulk
Copy link
Contributor

@rohitpaulk rohitpaulk commented May 3, 2017

#4399 (Export as CSV yields blank file)

This is a regression from the move to Gratipay 2.0. We moved all internal money-moving logic to the payments table instead of transfers, but when history is fetched - only the transfers table is hit.

This affects the yearly balances calculation, exporting giving and exporting taking. I've solved the first 2 as part of this PR (since they are actually used in the UI right now). Added a note about fixing taking - that can be a separate PR.

@rohitpaulk rohitpaulk force-pushed the fix-export-csv-bug branch from 8e96650 to bf6bd4d Compare May 3, 2017 10:50
@rohitpaulk rohitpaulk force-pushed the fix-export-csv-bug branch from bf6bd4d to 7172922 Compare May 3, 2017 10:52
@rohitpaulk
Copy link
Contributor Author

Screenshot of docs:

screen shot 2017-05-03 at 4 22 45 pm

This was an undocumented feature, we were always using
mode=aggregate in links.

Less to maintain, the better!
@rohitpaulk rohitpaulk force-pushed the fix-export-csv-bug branch 3 times, most recently from 3da2b9a to a6d3477 Compare May 3, 2017 17:56
@rohitpaulk rohitpaulk force-pushed the fix-export-csv-bug branch from a6d3477 to 73e16c2 Compare May 3, 2017 18:02
@rohitpaulk
Copy link
Contributor Author

Looking good... going to perform a smoke test against the prod DB before marking as ready for review

@chadwhitacre
Copy link
Contributor

@rohitpaulk Does this relate to #4274?

@rohitpaulk
Copy link
Contributor Author

Aha! Was wondering how no one discovered that 😛

Yes, this will fix both #4274 and #4399

@rohitpaulk
Copy link
Contributor Author

DB backup imported - I can replicate both issues (export csv returns empty, balances are wrong) using my own profile. Now to see if the patch fixes it...

@rohitpaulk
Copy link
Contributor Author

Blank export CSV is fixed!

Balances still seem to be a problem though...

@rohitpaulk
Copy link
Contributor Author

Balances still seem to be a problem though...

Ah! I didn't run branch.sql :P Done now, looking good.

@rohitpaulk
Copy link
Contributor Author

Balances lining up perfectly:

screen shot 2017-05-04 at 9 22 02 am

@rohitpaulk
Copy link
Contributor Author

rohitpaulk commented May 4, 2017

I'm seeing a small bug on the penultimate payday in 2016 though... It shows 2 masspay entries and a negative balance, where it should only be one..

@rohitpaulk
Copy link
Contributor Author

rohitpaulk commented May 4, 2017

Aha! Not a bug, we actually ran two masspays - gratipay/inside.gratipay.com#951 (comment)

@rohitpaulk
Copy link
Contributor Author

Okay! I think this is good to go.

DB Schema
---------

is_suspipicous on participant can be None, True or False. It represents unknown,
Copy link
Contributor

Choose a reason for hiding this comment

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

THIS MAKES ME SO HAPPY TO FIX THIS TYPO!!!!!!!!!!!! 😻

@chadwhitacre chadwhitacre merged commit 18d308e into master May 4, 2017
@chadwhitacre chadwhitacre deleted the fix-export-csv-bug branch May 4, 2017 14:03
@mattbk
Copy link
Contributor

mattbk commented May 4, 2017

👍 Works for me!

@rohitpaulk
Copy link
Contributor Author

Running!

screen shot 2017-05-05 at 8 28 38 pm

@chadwhitacre
Copy link
Contributor

Wrong ticket? 😜

@rohitpaulk
Copy link
Contributor Author

Haha yes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants