openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.12k stars 724 forks source link

Non-numerical values (ex.: using commas) trigger snail when setting up payment and shipping method Calculator #7841

Open filipefurtad0 opened 3 years ago

filipefurtad0 commented 3 years ago

Description

The Calculator section, when setting up payment or shipping methods, can easily break the app displaying a visible snail to the user, if non-numerical values are typed. Some examples are shown on the pic below.

It should be noted that currently, most of the backoffice of the app does not take commas as decimal separators. So, setting a Flat Rate per Order of 2,5 will trigger the error, ex:

https://app.bugsnag.com/yaycode/openfoodnetwork-uk/errors/60d5d1b3c0835200099efcf3?event_id=60d5d1b3007f61c7fb470000&i=sk&m=nw

This relates the "commas as decimal separators" discussion, and as such PRs #7758 and #7719.

Expected Behavior

If an invalid input is added, clicking Update should warn the user so the input can be corrected; it should not cause an error 500.

Update PR #8144 introduces an informative banner to the user: it is expected that this banner is displayed when non-numerical values are introduced.

Actual Behaviour

If an invalid input is added, clicking Update causes an error 500 and displays the snail to the user.

Steps to Reproduce

As an Admin:

  1. Edit a payment method.
  2. On the Calculator section choose a Calculator
  3. Type in an invalid value (some examples below)
  4. Click Update -> :snail:

Animated Gif/Screenshot

Some examples of "invalid" inputs: image

Peek 2021-06-25 13-57

Workaround

Navigate back from the snail and type a valid input.

Severity

bug-s3: a feature is broken but there is a workaround

Your Environment

Possible Fix

filipefurtad0 commented 3 years ago

See also these different scenarios, as described here. In addition to these, the Weight calculator should be considered when fixing this issue.

drummer83 commented 2 years ago

Summary

Payment methods have been fixed in #8204, only weight calculator is still triggering the snail. Shipping methods are covered in #8249 and have been addressed in #8312, but incomplete - should be reopened. Enterprise fees are covered in #8250 and have been addressed in #8308, but incomplete - should be reopened.

Things left to do in this issue

Only the weight calculator still needs fixing for payment methods!

Steps to Reproduce As an Admin:

drummer83 commented 2 years ago

Update

Payment methods have been fixed in #8204, weight calculator has been removed for payment methods at some point!? --> Was this intentional? I can't find a discussion around this, @RachL? Leftover to be addressed in #9861. Shipping methods are covered in #8249 and have been addressed in #8312 and #9756. Leftovers to be addressed in #9836. Enterprise fees are covered in #8250 and have been addressed in #8308 and #9791. Leftovers to be addressed in #9860.

Things left to do in this issue

Nothing, unless weight calculator should be reintroduced for payment methods.