Copy link Copy Markdown

Contributor

       resolves 1581

Copy link Copy Markdown

Contributor Author

This is ready for review. Thank you!

		def hat_html_label(hat)
		hl = hat.link.present? && hat.link.match(/^https?:\/\//)

		h = "<span class=\"hat " \

Copy link Copy Markdown

Member

Choose a reason for hiding this comment

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

I should've been clearer in my bug report. One of the things I'd like out of this refactoring is to get rid of all this unsafe string concatenation. If you look at the other helpers, you'll see use of link_to and content_tag to generate HTML without needing to use .html_safe. Can you continue the refactor to that safer approach?

Copy link Copy Markdown

Contributor Author

Choose a reason for hiding this comment

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

Got it! I'll make the changes.

Copy link Copy Markdown

Contributor Author

Choose a reason for hiding this comment

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

Done making the changes.

Copy link Copy Markdown

Member

This looks great, thank you! It'll be much easier to edit this safely. You've cleaned up the last place an ActiveRecord model produced HTML for the frontend.

I do hope you'll choose another issue to contribute to, feel free to comment or email if you'd like help picking one out. :)