shopware icon indicating copy to clipboard operation
shopware copied to clipboard

NEXT-36122 - Allow empty string in order customer comment

Open M-arcus opened this issue 1 year ago • 6 comments

1. Why is this change necessary?

https://issues.shopware.com/issues/NEXT-36122

2. What does this change do, exactly?

Add AllowHtml() flag to definition of field customerComment in OrderDefinition to allow sanitized HTML in customer comments.

3. Describe each step to reproduce the issue or behavior.

Before pressing buy on the checkout page, enter "<3" as the customer comment. An exception is thrown. Similar behavior in the API.

4. Please link to the relevant issues (if any).

https://issues.shopware.com/issues/NEXT-36122

5. Checklist

  • [x] I have rebased my changes to remove merge conflicts
  • [ ] I have written tests and verified that they fail without my change
  • [x] I have created a changelog file with all necessary information about my changes
  • [ ] I have written or adjusted the documentation according to my changes
  • [ ] This change has comments for package types, values, functions, and non-obvious lines of code
  • [x] I have read the contribution requirements and fulfil them.

M-arcus avatar May 13 '24 22:05 M-arcus

I think, this would not be a good solution, we don't want to have HTML content in the DB.

How about the flag AllowEmptyString instead?

mstegmeyer avatar May 14 '24 11:05 mstegmeyer

@mstegmeyer Good idea, but that will mean, that comments like '<3 my comment about the delivery' will be saved as empty string, right?

M-arcus avatar May 15 '24 07:05 M-arcus

@mstegmeyer Good idea, but that will mean, that comments like '<3 my comment about the delivery' will be saved as empty string, right?

No. It will be saved as my comment about the delivery.

When a field does not have AllowHtml(), it will run strip_tags on the input field. Because it strips tags, <3 is removed, and the input now contains an empty string, which causes the exception to be thrown.

Bird87ZA avatar May 15 '24 15:05 Bird87ZA

I have changed the flag to AllowEmptyString() and tested it with the comment. Sadly, the comment still gets removed, because strip_tags removes the entire part after <3

image

But at least it saves the order now.

M-arcus avatar May 17 '24 09:05 M-arcus

Ah I see. This is unfortunately how PHP's strip_tags work. If you have an alternative, you can make a change here.

If you are fine with this limitation, then just fix the PHP Lint failure.

Bird87ZA avatar May 21 '24 09:05 Bird87ZA

@Bird87ZA Fixed. No idea why https://github.com/shopware/shopware/actions/runs/9172715717/job/25219862488?pr=3710 fails.

M-arcus avatar May 21 '24 10:05 M-arcus

Hello,

thank you for creating this pull request. I have opened an issue on our Issue Tracker for you. See the issue link: https://issues.shopware.com/issues/NEXT-36122

Please use this issue to track the state of your pull request.

Your PR has been merged! Thanks for your contribution 💙

Bird87ZA avatar May 22 '24 06:05 Bird87ZA

https://issues.shopware.com/issues/NEXT-36122 is still open @Bird87ZA

M-arcus avatar Jun 15 '24 15:06 M-arcus