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

Currency validation: Addressing bug found by @ellevargas #27

Open
wants to merge 335 commits into
base: master
Choose a base branch
from

Conversation

Trishthedish
Copy link

After spending lots of time testing out theories about how I could solve the rounding issue.

  • Some found here: https://repl.it/EI4h/3
  • I looked at where the issue was occurring, and how many times it was actually being triggered. I yelped, with joy 🌈 to realize it was only happening 7 times throughout the site.

Found a clever way to achieve MVP by simply removing the offensive data.

I totally realize this is mvp territory && highlighted a gap in my understanding of how to use BigDecimal properly in Ruby.

@Hamled any resources you might recommend on actually solving this problem in a non-hack-mvp type of way?

Also, its late & if this no sense. I can explain tomorrow

ellevargas and others added 30 commits October 20, 2016 19:56
…th div tags around it, will download other pics and update products csv
… separated images into folders in assets by category
…ow pages looking real nice. I will fix any conflicts in the morning after I submit pr
…wn authentication -- both create and destroy methods
…th sessions resources except for login and logout routes - session new and destroy
Trishthedish and others added 30 commits October 26, 2016 23:04
… status, and ship and fulfillment button. Except order details associated with this order item
created a formalized and fun readme
had deleted this because it broke tests.  can fix that.  need this.  …
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants