Remove unneeded link conversions in chat integrations

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Work on this issue
• Close this issue

Summary

The following discussion from https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/2031 should be addressed:

• @toupeira started a discussion:

  note: The format method is used in a few places to convert Markdown/HTML links to Slack syntax, but this is already done automatically for the message and attachments (:text property) through middleware in the slack-messenger gem: https://gitlab.com/gitlab-org/slack-notifier#middleware

  So I think we can remove some format calls and simplify the code a bit, but I'd also like to do this in a public follow-up.

The following discussion from https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/2031 should be addressed:

• @toupeira started a discussion:

  note: I figured we could do this in a (public) follow-up, in case we do need HTML somewhere after all.

Improvements

• Disable link conversion for HTML links:
  • https://gitlab.com/gitlab-org/security/gitlab/blob/a40bd0a27fde8b04d67164510099ee66895bc685/app/models/concerns/integrations/slack_mattermost_notifier.rb#L13
  • https://gitlab.com/gitlab-org/security/gitlab/blob/a40bd0a27fde8b04d67164510099ee66895bc685/app/models/integrations/chat_message/base_message.rb#L73
  • It doesn't seem like we use HTML links anywhere, so this shouldn't break anything.
• Remove manual link conversion where it's already handled through the middleware:
  • https://gitlab.com/gitlab-org/security/gitlab/blob/a40bd0a27fde8b04d67164510099ee66895bc685/app/models/integrations/chat_message/base_message.rb#L47
  • Possibly other places (only the text: attributes of the message and attachments are converted automatically)

Testing

We have to make sure the changes don't break the formatting for any of the chat message types in any of the chat integrations. Ideally we should manually test all of them.