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

Remove unused currency from calculators #10318

Conversation

dacook
Copy link
Member

@dacook dacook commented Jan 25, 2023

What? Why?

Capture d’écran 2023-01-18 à 16 43 22

Capture d’écran 2023-01-18 à 16 43 57

What should we test?

As in #10272 (comment) . For example:

  • As an admin, when editing fees that include method, or fees that include calculator is Amount (flatrate, ...) you should not see Currency input and therefore you should be able to edit it.

Release notes

Changelog Category: User facing change

Documentation updates

The user guide already doesn't mention this field (eg https://guide.openfoodnetwork.org/basic-features/shopfront/shipping-methods#fee-calculators )

However it's already a bit confusing that you have to save/update before you can access the calculator values, so I've submitted a PR to update the user guide: openfoodfoundation/ofn-UserGuide#39

Copy link
Contributor

@jibees jibees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very simple and clever. Thanks a lot.

@jibees
Copy link
Contributor

jibees commented Jan 25, 2023

Shouldn't we clean the spree_preferences table via a db migration? Just thinking...

dacook added a commit to dacook/openfoodnetwork that referenced this pull request Jan 27, 2023
@dacook
Copy link
Member Author

dacook commented Jan 27, 2023

Ah, good question, I didn't think of that!

I tried reading the Spree documentation to get an overview and understand how it is stored, but I think it just confused me more. Instead I browsed the database and it became clearer:

open_food_network_dev=# select id, key, value_type, substr(value, 0, 32) from spree_preferences where key like '/calculator%/currency/%';
  id   |                 key                  | value_type | substr
-------+--------------------------------------+------------+---------
 15646 | /calculator/flat_rate/currency/1826  | string     | --- AUD+
       |                                      |            | ...    +
       |                                      |            |
 15644 | /calculator/per_item/currency/2009   | string     | --- AUD+
       |                                      |            | ...    +
       |                                      |            |
   317 | /calculator/flat_rate/currency/357   | string     | AUD
   337 | /calculator/per_item/currency/369    | string     | AUD
...
(533 rows)

(it looks like older ones are stored in plain text, newer in a yaml format. doesn't matter either way).

I don't know if Spree will automatically clean these up upon next save. And it probably doesn't matter. But now that I've come this far I will try cleaning up with a new PR.

@filipefurtad0 filipefurtad0 self-assigned this Feb 1, 2023
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Feb 1, 2023
@filipefurtad0
Copy link
Contributor

filipefurtad0 commented Feb 1, 2023

Hey @dacook ,

I've checked the enterprise fees page before (left - we can see the currency field) and after staging the PR (right - we can see the currency field is removed):

image
image

Also, for payment methods:

image

For shipping methods, I believe this was removed elsewhere? (no difference between before and after staging this PR):

image

Edit: this is correct, it only appears in some calculators. Updating the testing notes on the next comment.

Good to go! Merging.

@filipefurtad0 filipefurtad0 merged commit 1e3f754 into openfoodfoundation:master Feb 1, 2023
@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Feb 1, 2023
@filipefurtad0
Copy link
Contributor

An addenda to the previous testing notes: on the edit pages for shipping and payment methods, and for the respective calculators, we can compare the removal from those in which they've previously appeared.

For shipping methods:

image

image

image

image

image

For payment methods:

image

image

image

image

image

Again - good to go.

dacook added a commit to dacook/openfoodnetwork that referenced this pull request Feb 1, 2023
dacook added a commit to dacook/openfoodnetwork that referenced this pull request Feb 9, 2023
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