Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transactions and Donations do not allow decimals #1709

Open
betsydupuis opened this issue Jul 13, 2024 · 9 comments
Open

Transactions and Donations do not allow decimals #1709

betsydupuis opened this issue Jul 13, 2024 · 9 comments

Comments

@betsydupuis
Copy link

Screenshot 2024-07-13 at 4 17 33 PM

Definitely a "bug or feature" question, but since the admin team strives for accuracy, it would be reasonable to support 2 decimal places for financial transactions.

@jessib
Copy link
Collaborator

jessib commented Jul 30, 2024

It looks like the amount column in relationships table is a bigint so this would require a db migration to hold a float. I think the number_field for the Amount in app/views/relationships/edit.html.erb could get passed a step to prevent the front-end validation requiring an int, but that doesn't help with storing, and scrolling up and down by a cent probably isn't a great UI.

@betsydupuis
Copy link
Author

so this would require a db migration to hold a float.

That's unfortunate.

scrolling up and down by a cent probably isn't a great UI.

It might make sense to always round up or down.

@jessib
Copy link
Collaborator

jessib commented Aug 2, 2024

If we want it to step by cent, this change will the front end part:

--- a/app/views/relationships/edit.html.erb
+++ b/app/views/relationships/edit.html.erb
@@ -123,7 +123,7 @@
   <% if [5, 6, 7].include? @relationship.category_id  %>
     <%= edit_entity_form_section(
       f.label(:amount, 'Amount', class: label_class),
-      f.number_field(:amount)
+      f.number_field(:amount, {step: 0.01} )
     ) %>
 

This step change means the up/down arrows now change by cent:
image

However, I'm not entirely clear this is desirable, as not sure donations are actually tracked by cent.

If we do update the db to hold a float, we do don't want to store (or at least display) more than the 2 decimal places.

@aepyornis
Copy link
Contributor

you can use numeric type to round correctly https://www.postgresql.org/docs/16/datatype-numeric.html#DATATYPE-NUMERIC-DECIMAL

another option is https://github.com/RubyMoney/money-rails and store it as cents w/ bigint.

i personally prefer ints to keep it simple since the amounts are often in the thousands and millions :)

@betsydupuis
Copy link
Author

i personally prefer ints to keep it simple since the amounts are often in the thousands and millions :)

Agreed, but sounds like something that can be solved on the frontend.

However, I'm not entirely clear this is desirable, as not sure donations are actually tracked by cent.

Donations on IRS forms often have cents. FEC tracks cents too.

I mostly notice from copy and pasting in forms and finding it annoying that I had one more step before I could move on.

@josephlacey
Copy link
Contributor

@mooninmyname This issue has the conversation about whether to include cents when tracking donations in the database.

@josephlacey
Copy link
Contributor

I mostly notice from copy and pasting in forms and finding it annoying that I had one more step before I could move on.

This is interesting to consider. If we do stick with integers, I wonder if we can add some JavaScript to automatically remove the cents when pasting a number in.

@betsydupuis
Copy link
Author

This is interesting to consider. If we do stick with integers, I wonder if we can add some JavaScript to automatically remove the cents when pasting a number in.

Definitely a chicken or egg situation.

The current implementation seems to have centered this decision on the presentation issue of how the data will look in it's final form, which is something javascript or even the API can solve.

But it's somewhat backfired into supporting this increment input (which I think could be removed if switched to decimals).

Decimals can always be reformatted programmatically, but you can't put data back that isn't there.

If this was the 90s, we might be debating the size of the database on these two decimals, but hopefully this isn't an issue lol.

A switch to add cents would probably require a fix to ensure all the places that monetary values show up are rounded in presentation.

@betsydupuis
Copy link
Author

There's always math.round() and listen for keystrokes on the field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants