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

NEXT-36122 - Allow empty string in order customer comment #3710

Conversation

M-arcus
Copy link
Contributor

@M-arcus M-arcus commented May 13, 2024

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

  • I have rebased my changes to remove merge conflicts
  • I have written tests and verified that they fail without my change
  • 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
  • I have read the contribution requirements and fulfil them.

@mstegmeyer
Copy link
Contributor

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?

@Bird87ZA Bird87ZA self-assigned this May 14, 2024
@M-arcus M-arcus force-pushed the next-36122/allow-html-in-order-customer-comment branch from c3eb2f3 to 4eaf978 Compare May 15, 2024 06:55
@M-arcus
Copy link
Contributor Author

M-arcus commented May 15, 2024

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

@Bird87ZA
Copy link

Bird87ZA commented May 15, 2024

@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.

@M-arcus M-arcus force-pushed the next-36122/allow-html-in-order-customer-comment branch from 4eaf978 to dd63609 Compare May 17, 2024 09:46
@M-arcus M-arcus changed the title NEXT-36122 - Allow Html in order customer comment NEXT-36122 - Allow empty string in order customer comment May 17, 2024
@M-arcus
Copy link
Contributor Author

M-arcus commented May 17, 2024

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.

@Bird87ZA
Copy link

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.

@M-arcus M-arcus force-pushed the next-36122/allow-html-in-order-customer-comment branch from dd63609 to da3dd54 Compare May 21, 2024 09:59
@M-arcus
Copy link
Contributor Author

M-arcus commented May 21, 2024

@shopware-github-importer
Copy link

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.

@Bird87ZA
Copy link

Your PR has been merged! Thanks for your contribution 💙

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants