Make Currency a more formal type
ClosedPublic
Actions

Authored by epriestley on Oct 4 2014, 8:35 PM.

Details

Reviewers

btrahan

Maniphest Tasks

T2787: Build Phortune into a real accounts/billing application

Commits

Restricted Diffusion Commit
rPf86f9dc51283: Make Currency a more formal type

Summary

Ref T2787. Phortune currently stores a bunch of stuff as ...inUSDCents. This ends up being pretty cumbersome and I worry it will create a huge headache down the road (and possibly not that far off if we do Coinbase/Bitcoin soon). Even now, it's more of a pain than I figured it would be.

Instead:

Provide an application-level serialization mechanism.
Provide currency serialization.
Store currency in an abstract way (currently, as "1.23 USD") that can handle currencies in the future.
Change all ...inUSDCents to ..asCurrency.
This generally simplifies all the application code.
Also remove some columns which don't make sense or don't make sense anymore. Notably, Product is going to get more abstract and mostly be provided by applications.

Test Plan

Created a new product.
Purchased a product.
Backed an initiative.
Ran unit tests.

Diff Detail

Repository

rP Phabricator

Branch

fund1

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 2743
Build 2747: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley updated this revision to Diff 25540.Oct 4 2014, 8:35 PM

epriestley retitled this revision from to Make Currency a more formal type.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: btrahan.

epriestley added a task: T2787: Build Phortune into a real accounts/billing application.

Herald added a subscriber: epriestley. · View Herald TranscriptOct 4 2014, 8:35 PM

Harbormaster completed remote builds in B2743: Diff 25540.Oct 4 2014, 8:35 PM

epriestley added inline comments.Oct 4 2014, 8:36 PM

resources/sql/autopatches/20140902.almanacdevice.1.sql
4–13	This isn't precisely related, but these patches will currently fail on old MySQL by emitting `COLLATE binary`.
resources/sql/autopatches/20141004.harborliskcounter.sql
1–4	I previously removed this because it's not used by Harbormaster, but it is used by a unit test.

btrahan accepted this revision.Oct 6 2014, 5:18 PM

btrahan edited edge metadata.

btrahan added inline comments.

resources/sql/autopatches/20140902.almanacdevice.1.sql
4–13	T6251 fix instead?

This revision is now accepted and ready to land.Oct 6 2014, 5:18 PM

I think it's slightly more subtle for columns than databases, because the binary collation is a virtual collation that changes the column type. Some variant of that fix may come here eventually, but for now I'm just not using the new stuff since it doesn't matter in the long run and is less likely to cause a problem.

Closed by commit rPf86f9dc51283: Make Currency a more formal type (authored by epriestley, committed by epriestley).Oct 6 2014, 5:27 PM

This revision was automatically updated to reflect the committed changes.