[openstreetmap/openstreetmap-website] Refactor render_social_share_buttons helper (Issue #5415)

[Andy Allan]

The implementation of render_social_share_buttons can be refactored in a number of ways:

* `html_safe` **must not be used**. It does not convert strings to a safe representation, it is an assertion that the string is always safe given every possible input. This method is probably too complex for us to assert this today, and certainly not to ensure that everything involved is perfectly safe when features are added or removed from the helper in future. See e.g. `opengraph_tags` helper for the use of `safe_join` to combine tags in loop.
* No need for a `render_` prefix in helper methods. See e.g. `opengraph_tags` helper
* There are only two `opts` ever used in the method, `:title` and `:url`. Since there are only two, it would be better to call the method with these explicitly, ideally as keyword arguments so that the order doesn't matter.
* The `filter_allowed_sites` method behaves surprisingly. If you pass it some sites, it will only return a subset of those sites. However, if you pass an empty set, instead of an empty list you instead get a full list of sites that were not requested. However, it's probably unlikely to be important, because...
* The `filter_allowed_sites` method is unnecessary and unused code. It should be removed, since unnecessary and used code is both a source of bugs, and also makes it harder for new developers in future, who need to read and understand it. Personally I've spent time trying to review how it works and what it's trying to do, before realising that it's not used anywhere.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5415
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/issues/5415 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20241218/829f025f/attachment-0001.htm>