Page MenuHomePhabricator

Make Currency a more formal type
ClosedPublic

Authored by epriestley on Oct 4 2014, 8:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 15, 12:49 AM
Unknown Object (File)
Sat, Apr 6, 6:00 PM
Unknown Object (File)
Tue, Apr 2, 5:16 PM
Unknown Object (File)
Mon, Apr 1, 8:17 PM
Unknown Object (File)
Sun, Mar 24, 5:21 AM
Unknown Object (File)
Sun, Mar 24, 5:07 AM
Unknown Object (File)
Feb 9 2024, 11:30 AM
Unknown Object (File)
Feb 8 2024, 5:49 PM
Subscribers

Details

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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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.
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
2–5

I previously removed this because it's not used by Harbormaster, but it is used by a unit test.

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.

This revision was automatically updated to reflect the committed changes.