openfoodfoundation / openfoodnetwork

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

Allow return or line break in shipping / payment method description #12465

Closed MrBowmanXD closed 1 week ago

MrBowmanXD commented 3 weeks ago

What? Why?

When setting up a shipping or payment method description, users are allowed to return to line or add line breaks in the admin side.

However this is not done is the shopfront where everything is only separated with a space. If description are long this can be quite ugly.

(copied from the issue)

Simply added the simple_format method in order to introduce line breaks or return in the shipping method description. (checkout/_details.html.haml: line98)

What should we test?

  1. Login as admin, go to administration.
  2. Add shipping method description with line breaks / return
  3. Checkout and observe the same description in shopfront (access checkout details page threw shopping cart)

Note: Check security issues but simple_format sanitizes the html by default according to the documentation.

Release notes

Changelog Category (reviewers may add a label for the release notes):

The title of the pull request will be included in the release notes.

MrBowmanXD commented 2 weeks ago

Yes i can.

MrBowmanXD commented 2 weeks ago

I think we still need to test in order to check if the order of the methods is right, but in essence the solution might look like the above commit shows.

MrBowmanXD commented 2 weeks ago

We could make a test that simulates adding mailicious html to check if html_escape is working or it's redundant?

rioug commented 2 weeks ago

We could make a test that simulates adding mailicious html to check if html_escape is working or it's redundant?

It's not necessary, html_escape is part of Rails so it should be tested by Rails. If we can't trust Rails then we are in trouble :smile:

MrBowmanXD commented 1 week ago

Need to delete this last commit in github. This last commit introduces code that should not be there.

MrBowmanXD commented 1 week ago

It's good now, thanks for your help @MrBowmanXD 🙏

You last commit will be sorted out once we merge into master. Note for next time we prefer to rebase the branch instead of merging master.

Sorry for the late response but this last commit introduces code that should not be there. I can make another pull request if necessary.

MrBowmanXD commented 1 week ago

Had to remove the code according to #12443

filipefurtad0 commented 1 week ago

Hey @MrBowmanXD ,

Thanks for another PR :muscle:

Before staging it, we can see the bug (left, admin section; right, shopfront):

image

After staging your PR we can see the line breaks are displayed correctly:

image

Awesome!! :tada: Merging.

filipefurtad0 commented 1 week ago

Ohh, I just noticed there are some merge conflicts. Can you fix these @MrBowmanXD ?

Thanks!

MrBowmanXD commented 1 week ago

Check the shipping method description (in checkout details) and the payment method description (in checkout payment).