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

Bring some js and css code from spree_backend needs in the general configurations #4617

Closed
wants to merge 4 commits into from

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Dec 19, 2019

What? Why?

Continues #4595

What should we test?

Image settings:

  • no need to test the actual working of the setting, just the form
  • verify you can add a new style and remove it and also that you can add a new s3 header (removing a s3 header is broken, we can open a new issue if we want to fix this)

States:

  • Editing the list of states of a country, when you change from one country to another, the link to create a new state should be updated. Add a state to a country and then change to another country and try to add a state to that new country. The second state should be added to the second country.

Calculator Fields:

  • Edit a shipping method and make sure that when you change the calculator, you can see the warning: "If you are changing the calculator type, you must save first before you can edit the calculator settings"

Zones:

  • Verify the edit zone page and make sure you can switch between country based to state based and the panel on the right switches accordingly and works correctly.

Release notes

Changelog Category: Changed

Bring code from spree_backend related to general configurations so that we can make OFN independent of Spree.

@luisramos0
Copy link
Contributor Author

rebased and tested deployment process. all good, ready for testing.

@lin-d-hop lin-d-hop added the pr-staged-uk staging.openfoodnetwork.org.uk label Jan 31, 2020
@lin-d-hop
Copy link
Contributor

Image settings. The form appears to work. When I click new the form offers me 2 new options and I have to remove one to save.

Screenshot from 2020-01-31 11-45-51

Screenshot from 2020-01-31 11-47-09

States - adding states to different countries persists as expected

Calculators - warning shows correctly in Shipping and Payment methods

Zones - Works correctly but when I go to add a new zone I am shown 2 additional zones fields instead of one. Same problem as above.

Screenshot from 2020-01-31 11-45-30

As this double up of fields is happening in multiple places I fear that we might find this popping up all over which will create an additional support demand for new instances as we have to explain that to save you must delete on of the additional fields.

Not sure if this will also flow through to user facing areas.

So despite it being small and easily worked around I suggest we just fix this here and now.

Back to In Dev.

@lin-d-hop lin-d-hop removed the pr-staged-uk staging.openfoodnetwork.org.uk label Jan 31, 2020
@luisramos0
Copy link
Contributor Author

ah, the code is duplicated between spree and OFN now, that's why we see two entries...
This will go away when we remove spree_backend

"we might find this popping up all over " - we are only changing these pages, nothing else. There's no risk of seeing this in other places.

Agree, we shouldnt put this in produiction.
But I think I'll leave this for when I remove spree_backend, that way this will just work as is.

@luisramos0
Copy link
Contributor Author

Closing, this is going in as part of #4621

@luisramos0 luisramos0 closed this Feb 5, 2020
@luisramos0 luisramos0 deleted the settings_js branch February 5, 2020 13:58
@luisramos0 luisramos0 mentioned this pull request Feb 6, 2020
18 tasks
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.

4 participants