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

Add html templates #1377

Merged
merged 4 commits into from Jul 1, 2017
Merged

Conversation

DanielePalombo
Copy link
Contributor

@mamhoff
Copy link
Contributor

mamhoff commented Aug 5, 2016

LGTM. 👍

<tr>
<td><%= item.variant.sku %></td>
<td><%= raw(item.variant.product.name) %></td>
<td><%= raw(item.variant.options_text) -%></td>
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe that these two should use raw. It is user input, so I don't think it should be trusted.

Copy link
Member

Choose a reason for hiding this comment

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

If text formatting is the reason for using raw, we probably should use sanitize here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used raw because i copied from

<%= raw(item.variant.product.name) %>

Like in other html templates, i don't use sanitize, the only thing to do is escape the text.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Valid changes I would like to merge. Could you please fix the conflicts and add a CHANGELOG entry? Thanks

@DanielePalombo
Copy link
Contributor Author

Conflicts are fixed. CHANGELOG updated.

@@ -21,13 +21,13 @@
<% @reimbursement.return_items.exchange_requested.each do |return_item| %>
<tr>
<td>
<%= return_item.variant.sku %> <%= raw(return_item.variant.name) %> <%= "(#{raw(return_item.variant.options_text)})" if return_item.variant.options_text.present? %>
<%= return_item.variant.sku %> <%=return_item.variant.name %> <%= "(#{return_item.variant.options_text})" if return_item.variant.options_text.present? %>
Copy link
Member

Choose a reason for hiding this comment

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

missing space

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thanks for updating.

DanielePalombo and others added 2 commits June 13, 2017 11:31
The product informations in email templates was not escaped because was used `raw`
method.
@DanielePalombo
Copy link
Contributor Author

Updated. Thanks!

Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

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

:shipit:

@gmacdougall gmacdougall merged commit 682f6ae into solidusio:master Jul 1, 2017
gmacdougall pushed a commit that referenced this pull request Jul 1, 2017
Resolves #1377

Conflicts:
	CHANGELOG.md
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.

None yet

4 participants