[Feature] Add clickable metadata links when using the Telegram connector. (IMDb / Trakt / TVDb, TVMaze) #6613

qTipTip · 2024-03-09T13:14:07Z

Description

Adds a checkbox and a dropdown allowing you to select whether to add a clickable link to your Telegram notification, and which database you want a link to.

Questions:

Which events should this be added to. Currently only implemented for series added and deleted.
Should it be added for episode-specific events, like episode grabbed?
Should we only use the dropdown and have a None or Disabled-option instead? Probably easier to reason about for the user.

I'd appreciate some input on wording. The setting-labels are not very enlightening at the moment.

Screenshots for UI Changes

Issues Fixed or Closed by this PR

Closes Add IMDB Link to Telegram Notifications #5342

Add metadata links to series when using Telegram connector.

qTipTip · 2024-03-09T13:14:45Z

src/NzbDrone.Api.Test/Sonarr.Api.Test.csproj

@@ -1,6 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>net6.0</TargetFrameworks>
+    <Platforms>AnyCPU;x86</Platforms>


I'll drop the changes in the .csproj-files. Not sure how I managed to commit them.

Please do, the Sonarr.sln one as well.

qTipTip · 2024-03-09T13:15:25Z

src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

        private readonly ITelegramProxy _proxy;
+        private readonly Dictionary<MetadataLinkType, Func<string, string, string>> _formatLinkFromIdMethods;
+
+        private string FormatImdbLinkFromId(string message, string id)


Could this be encapsulated in a utility class somehow? Might be useful for other notication providers?

Potentially, but it depends a lot on the format something else might need, so for now I wouldn't worry about it.

This whole group of methods can be cleaned up and combined with FormatMessageWithLink

Something like to cut down on the functions and only add links when they will be correctly formatted:

private string FormatMessageWithLink(string message, Series series) { var linkType = (MetadataLinkType)Settings.MetadataLinkType; if (MetadataLinkType.None) { return message; } if (MetadataLinkType.Imdbb && series.ImdbId.IsNotNullOrWhiteSpace()) { return $"[{message}](https://www.imdb.com/title/{series.ImdbId})"; } ... return message; }

No need to throw if the ID is unavailable or the enum value is unexpected since we don't want the notification to be skipped, it'll just be missing that info.

Also move these private functions to the bottom of the class.

src/NzbDrone.Core/Notifications/Telegram/TelegramSettings.cs

stevietv

nice first contribution :)
I was wondering why the choice for 4 different methods for formatting the string? Couldnt it be done in a single method with a case switch if you pass in the MedatdataLinkType?

How have you handled cases where there isn't an imdb, trakt or tvmaze Id associated with the show, but user has selected on?

qTipTip · 2024-03-09T14:51:52Z

nice first contribution :) I was wondering why the choice for 4 different methods for formatting the string? Couldnt it be done in a single method with a case switch if you pass in the MedatdataLinkType?

How have you handled cases where there isn't an imdb, trakt or tvmaze Id associated with the show, but user has selected on?

Thanks for the input!

I'll try to create a switch.

And no, I haven't considered that fact!
Would that materialize as a null-value for the ID? If so I can make adding the link a no-op in that case.

markus101

Thanks for contributing! Left quite a few comments which should help clean things up a lot and reduce the number of changes to make it easier to follow.

markus101 · 2024-03-10T04:55:16Z

src/NzbDrone.Api.Test/Sonarr.Api.Test.csproj

@@ -1,6 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>net6.0</TargetFrameworks>
+    <Platforms>AnyCPU;x86</Platforms>


Please do, the Sonarr.sln one as well.

markus101 · 2024-03-10T05:00:16Z

src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

        private readonly ITelegramProxy _proxy;
+        private readonly Dictionary<MetadataLinkType, Func<string, string, string>> _formatLinkFromIdMethods;
+
+        private string FormatImdbLinkFromId(string message, string id)


Potentially, but it depends a lot on the format something else might need, so for now I wouldn't worry about it.

This whole group of methods can be cleaned up and combined with FormatMessageWithLink

Something like to cut down on the functions and only add links when they will be correctly formatted:

private string FormatMessageWithLink(string message, Series series) { var linkType = (MetadataLinkType)Settings.MetadataLinkType; if (MetadataLinkType.None) { return message; } if (MetadataLinkType.Imdbb && series.ImdbId.IsNotNullOrWhiteSpace()) { return $"[{message}](https://www.imdb.com/title/{series.ImdbId})"; } ... return message; }

No need to throw if the ID is unavailable or the enum value is unexpected since we don't want the notification to be skipped, it'll just be missing that info.

markus101 · 2024-03-10T05:00:30Z

src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

        private readonly ITelegramProxy _proxy;
+        private readonly Dictionary<MetadataLinkType, Func<string, string, string>> _formatLinkFromIdMethods;
+
+        private string FormatImdbLinkFromId(string message, string id)


Also move these private functions to the bottom of the class.

markus101 · 2024-03-10T05:01:08Z

src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

+
+        private string FormatMessageWithLink(string message, Series series)
+        {
+            if (Settings.SendMetadataLink && _formatLinkFromIdMethods.TryGetValue(Settings.MetadataLinkType, out var formatMethod))


Settings should validate that the stored MetadataLinkType is valid.

src/NzbDrone.Core/Notifications/Telegram/TelegramSettings.cs

markus101 · 2024-03-10T05:16:41Z

Would that materialize as a null-value for the ID? If so I can make adding the link a no-op in that case.

We'll have a TVDB ID 100% of the time, IMDB should be null (off chance of an empty string) and everything else will be 0 if not set.

qTipTip · 2024-03-11T18:22:57Z

src/NzbDrone.Core/Notifications/Telegram/TelegramProxy.cs


            var requestBuilder = new HttpRequestBuilder(URL).Resource("bot{token}/sendmessage").Post();

            var request = requestBuilder.SetSegment("token", settings.BotToken)
                                        .AddFormParameter("chat_id", settings.ChatId)
-                                        .AddFormParameter("parse_mode", "HTML")
+                                        .AddFormParameter("parse_mode", "MarkdownV2")


I changed the parser to MarkdownV2 to have the link render properly, as I think the HTML hrefs are being escaped in HTML-mode when just added to the message. However, this breaks the SeriesDeleteMessage as it contains a reserved character -.

Not quite sure how to handle this.

Which part of the message? A dash could also be in the series. We could escape the -, possibly with just \-, but what about * and other special characters in markdown? _ could cause issues, especially if there are two in the title.

Still need to determine the impact of this

Yes, apologies for the lack of activity here.

I'll try to mess around with reverting back to HTML.
Been unable to find a clean solution.

The problem I had initially with the HTML parser was that the markup would render as raw strings in the message, instead of formatting the message.

This is because HttpUtility.HtmlEncode is being called on the body text. The message body with the link can't be passed into SendNotification, the link needs to be passed in separately and then built in the proxy, which is better because it hides that implementation detail from Telegram.cs

qTipTip · 2024-03-11T18:23:47Z

src/NzbDrone.Core/Notifications/Telegram/TelegramSettings.cs

+    public enum MetadataLinkType
+    {
+        None = 0,
+        [FieldOption(Label = "IMDb")]


Should I add FieldOption to None and Trakt as well, where the Enum key aligns with the Label?

I think leaving them off as fine as long as the UI is rendering them correctly.

This reverts commit 4e8dbe7.

qTipTip · 2024-03-11T18:35:37Z

Thanks for contributing! Left quite a few comments which should help clean things up a lot and reduce the number of changes to make it easier to follow.

Thanks for the thorough review @markus101 ! I've cleaned up stuff, and it's a lot more readable now, so thanks for that! 🔥

How can I have None set as the default option in TelegramSettings.cs? Does that happen just by virtue of the = 0 in the enum?

markus101

Great work on the clean up, much easier to review now, left some more comments and questions.

src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

markus101 · 2024-03-14T04:23:51Z

src/NzbDrone.Core/Notifications/Telegram/TelegramProxy.cs

@@ -35,13 +35,13 @@ public TelegramProxy(IHttpClient httpClient, ILocalizationService localizationSe
        public void SendNotification(string title, string message, TelegramSettings settings)
        {
            // Format text to add the title before and bold using markdown
-            var text = $"<b>{HttpUtility.HtmlEncode(title)}</b>\n{HttpUtility.HtmlEncode(message)}";
+            var text = $"*{HttpUtility.HtmlEncode(title)}*\n{HttpUtility.HtmlEncode(message)}";


If this isn't sending HTML, do we still need to encode it for HTML?

This should send HTML still, this works when the link is passed in:

var text = new StringBuilder($"<b>{HttpUtility.HtmlEncode(title)}</b>\n"); if (link.IsNullOrWhiteSpace()) { text.Append($"{{HttpUtility.HtmlEncode(message)}}"); } else { text.Append($"<a href=\"{link}\">{HttpUtility.HtmlEncode(message)}</a>"); }

For test notifications something like the following inside Test:

var link = settings.MetadataLink == (int)MetadataLinkType.None ? null : "https://sonarr.tv"; SendNotification(settings.IncludeAppNameInTitle ? brandedTitle : title, body, link, settings);

This would allow people to see that the link works.

markus101 · 2024-03-14T04:24:27Z

src/NzbDrone.Core/Notifications/Telegram/TelegramSettings.cs

@@ -32,9 +39,27 @@ public class TelegramSettings : IProviderConfig
        [FieldDefinition(3, Label = "NotificationsTelegramSettingsSendSilently", Type = FieldType.Checkbox, HelpText = "NotificationsTelegramSettingsSendSilentlyHelpText")]
        public bool SendSilently { get; set; }

+        public bool SendMetadataLink { get; set; }


No longer needed

This still needs to be removed.

markus101 · 2024-03-14T04:24:54Z

src/NzbDrone.Core/Notifications/Telegram/TelegramSettings.cs

+    public enum MetadataLinkType
+    {
+        None = 0,
+        [FieldOption(Label = "IMDb")]


I think leaving them off as fine as long as the UI is rendering them correctly.

markus101 · 2024-03-14T04:29:53Z

src/NzbDrone.Core/Notifications/Telegram/TelegramProxy.cs


            var requestBuilder = new HttpRequestBuilder(URL).Resource("bot{token}/sendmessage").Post();

            var request = requestBuilder.SetSegment("token", settings.BotToken)
                                        .AddFormParameter("chat_id", settings.ChatId)
-                                        .AddFormParameter("parse_mode", "HTML")
+                                        .AddFormParameter("parse_mode", "MarkdownV2")


Which part of the message? A dash could also be in the series. We could escape the -, possibly with just \-, but what about * and other special characters in markdown? _ could cause issues, especially if there are two in the title.

src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

Co-authored-by: Mark McDowall <markus.mcd5@gmail.com>

markus101

I've tested this locally and added some additional comments and suggestions on how to make this functional.

I do wonder if a slightly different approach would be better, where individual links are added for each selected Metadata type, instead of turning the body of the message into a single link.

Let me know what you think about that and whether you'll be able to pick up the rest of the fixes here.

markus101 · 2024-04-11T23:58:25Z

src/NzbDrone.Core/Localization/Core/en.json

+  "NotificationsTelegramSettingsSendMetadataLink": "Add a link to the series metadata",
+  "NotificationsTelegramSettingsSendMetadataLinkHelpText": "Add a link to the series metadata when sending the notifications",


Suggested change

"NotificationsTelegramSettingsSendMetadataLink": "Add a link to the series metadata",

"NotificationsTelegramSettingsSendMetadataLinkHelpText": "Add a link to the series metadata when sending the notifications",

"NotificationsTelegramSettingsMetadataLink": "MetadataLink",

"NotificationsTelegramSettingsMetadataLinkHelpText": "Add a link to the series metadata when sending notifications",

markus101 · 2024-04-12T00:00:01Z

src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

@@ -33,12 +34,16 @@ public override void OnEpisodeFileDelete(EpisodeDeleteMessage deleteMessage)

        public override void OnSeriesAdd(SeriesAddMessage message)
        {
-            _proxy.SendNotification(SERIES_ADDED_TITLE, message.Message, Settings);
+            var text = FormatMessageWithLink(message.Message, message.Series);


These links should also be included for OnGrab, OnDownload and OnEpisodeFileDelete

markus101 · 2024-04-12T00:17:26Z

src/NzbDrone.Core/Notifications/Telegram/TelegramProxy.cs


            var requestBuilder = new HttpRequestBuilder(URL).Resource("bot{token}/sendmessage").Post();

            var request = requestBuilder.SetSegment("token", settings.BotToken)
                                        .AddFormParameter("chat_id", settings.ChatId)
-                                        .AddFormParameter("parse_mode", "HTML")
+                                        .AddFormParameter("parse_mode", "MarkdownV2")


This is because HttpUtility.HtmlEncode is being called on the body text. The message body with the link can't be passed into SendNotification, the link needs to be passed in separately and then built in the proxy, which is better because it hides that implementation detail from Telegram.cs

markus101 · 2024-04-12T00:17:53Z

src/NzbDrone.Core/Notifications/Telegram/TelegramSettings.cs

@@ -32,9 +39,27 @@ public class TelegramSettings : IProviderConfig
        [FieldDefinition(3, Label = "NotificationsTelegramSettingsSendSilently", Type = FieldType.Checkbox, HelpText = "NotificationsTelegramSettingsSendSilentlyHelpText")]
        public bool SendSilently { get; set; }

+        public bool SendMetadataLink { get; set; }


This still needs to be removed.

markus101 · 2024-04-12T00:18:03Z

src/NzbDrone.Core/Notifications/Telegram/TelegramSettings.cs

@@ -32,9 +39,27 @@ public class TelegramSettings : IProviderConfig
        [FieldDefinition(3, Label = "NotificationsTelegramSettingsSendSilently", Type = FieldType.Checkbox, HelpText = "NotificationsTelegramSettingsSendSilentlyHelpText")]
        public bool SendSilently { get; set; }

+        public bool SendMetadataLink { get; set; }
+        [FieldDefinition(4, Label = "NotificationsTelegramSettingsMetadataLinkType", Type = FieldType.Select, SelectOptions = typeof(MetadataLinkType), HelpText = "NotificationsTelegramSettingsMetadataLinkType")]


Suggested change

[FieldDefinition(4, Label = "NotificationsTelegramSettingsMetadataLinkType", Type = FieldType.Select, SelectOptions = typeof(MetadataLinkType), HelpText = "NotificationsTelegramSettingsMetadataLinkType")]

[FieldDefinition(4, Label = "NotificationsTelegramSettingsMetadataLink", Type = FieldType.Select, SelectOptions = typeof(MetadataLinkType), HelpText = "NotificationsTelegramSettingsMetadataLink")]

markus101 · 2024-04-12T00:18:14Z

src/NzbDrone.Core/Notifications/Telegram/TelegramSettings.cs

@@ -32,9 +39,27 @@ public class TelegramSettings : IProviderConfig
        [FieldDefinition(3, Label = "NotificationsTelegramSettingsSendSilently", Type = FieldType.Checkbox, HelpText = "NotificationsTelegramSettingsSendSilentlyHelpText")]
        public bool SendSilently { get; set; }

+        public bool SendMetadataLink { get; set; }
+        [FieldDefinition(4, Label = "NotificationsTelegramSettingsMetadataLinkType", Type = FieldType.Select, SelectOptions = typeof(MetadataLinkType), HelpText = "NotificationsTelegramSettingsMetadataLinkType")]
+        public MetadataLinkType MetadataLinkType { get; set; }


Suggested change

public MetadataLinkType MetadataLinkType { get; set; }

public int MetadataLink { get; set; }

markus101 · 2024-04-12T00:20:48Z

src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

@@ -69,5 +74,37 @@ public override ValidationResult Test()

            return new ValidationResult(failures);
        }
+
+        private string FormatMessageWithLink(string message, Series series)


This should be renamed to GetLink and just return the link, which gets passed to SendNotification and the message is formatted there.

markus101 · 2024-04-12T00:22:41Z

src/NzbDrone.Core/Notifications/Telegram/TelegramProxy.cs

@@ -35,13 +35,13 @@ public TelegramProxy(IHttpClient httpClient, ILocalizationService localizationSe
        public void SendNotification(string title, string message, TelegramSettings settings)
        {
            // Format text to add the title before and bold using markdown
-            var text = $"<b>{HttpUtility.HtmlEncode(title)}</b>\n{HttpUtility.HtmlEncode(message)}";
+            var text = $"*{HttpUtility.HtmlEncode(title)}*\n{HttpUtility.HtmlEncode(message)}";


This should send HTML still, this works when the link is passed in:

var text = new StringBuilder($"<b>{HttpUtility.HtmlEncode(title)}</b>\n"); if (link.IsNullOrWhiteSpace()) { text.Append($"{{HttpUtility.HtmlEncode(message)}}"); } else { text.Append($"<a href=\"{link}\">{HttpUtility.HtmlEncode(message)}</a>"); }

markus101 · 2024-04-12T00:24:45Z

src/NzbDrone.Core/Notifications/Telegram/TelegramProxy.cs

@@ -35,13 +35,13 @@ public TelegramProxy(IHttpClient httpClient, ILocalizationService localizationSe
        public void SendNotification(string title, string message, TelegramSettings settings)
        {
            // Format text to add the title before and bold using markdown
-            var text = $"<b>{HttpUtility.HtmlEncode(title)}</b>\n{HttpUtility.HtmlEncode(message)}";
+            var text = $"*{HttpUtility.HtmlEncode(title)}*\n{HttpUtility.HtmlEncode(message)}";


For test notifications something like the following inside Test:

var link = settings.MetadataLink == (int)MetadataLinkType.None ? null : "https://sonarr.tv"; SendNotification(settings.IncludeAppNameInTitle ? brandedTitle : title, body, link, settings);

This would allow people to see that the link works.

qTipTip · 2024-04-29T10:51:22Z

I've tested this locally and added some additional comments and suggestions on how to make this functional.

I do wonder if a slightly different approach would be better, where individual links are added for each selected Metadata type, instead of turning the body of the message into a single link.

Let me know what you think about that and whether you'll be able to pick up the rest of the fixes here.

Hey!

I'll have a look at this sometime this week!

qTipTip added 9 commits March 9, 2024 00:16

Add imdb-link to series.

bd64c70

Add a selectable enum

b58d099

use is

f1b4ea7

Conditionally send only if sendmetadatalink is set.

86e4900

Add trakt, maze, tvdb.

0f37586

Use constant url formats, and get the ID by the type of link.

c35e6f8

add to deleteEvent

32ba0e7

Fix typo

8024db2

Merge pull request #1 from qTipTip/add-imdb-link-to-telegram

ac75cce

Add metadata links to series when using Telegram connector.

github-actions bot added the connection label Mar 9, 2024

qTipTip commented Mar 9, 2024

View reviewed changes

qTipTip changed the title ~~Add clickable metadata links when using the Telegram connector. (IMDb / Trakt / TVDb, TVMaze)~~ [Feature] Add clickable metadata links when using the Telegram connector. (IMDb / Trakt / TVDb, TVMaze) Mar 9, 2024

stevietv reviewed Mar 9, 2024

View reviewed changes

src/NzbDrone.Core/Notifications/Telegram/TelegramSettings.cs Outdated Show resolved Hide resolved

stevietv reviewed Mar 9, 2024

View reviewed changes

markus101 requested changes Mar 10, 2024

View reviewed changes

qTipTip added 10 commits March 11, 2024 18:25

checkout csproj and Sonarr.sln from upstream

30999b9

Remove redundant setting, sanitize the enum values and add none.

72a59f9

Use sanitized enum vals

a4b524a

Move enum to bottom of file

b3dc231

Use FieldOption for labels

2718182

Add localization info

1ab9c78

Simplify FormatMessageWithLink

510f780

Move private method to bottom

54540c3

Add enum validation for MetaDataLink

f7addbf

Remove unused directive

130fbd4

qTipTip commented Mar 11, 2024

View reviewed changes

qTipTip added 2 commits March 11, 2024 19:26

Remove stray directives

4e8dbe7

Revert "Remove stray directives"

e7514f9

This reverts commit 4e8dbe7.

Remove directive and format

ebc0391

qTipTip requested review from markus101 and stevietv March 11, 2024 18:37

markus101 requested changes Mar 14, 2024

View reviewed changes

qTipTip and others added 3 commits March 21, 2024 16:34

Update src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

c81b8ec

Co-authored-by: Mark McDowall <markus.mcd5@gmail.com>

Update src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

6c5650a

Co-authored-by: Mark McDowall <markus.mcd5@gmail.com>

Update src/NzbDrone.Core/Notifications/Telegram/Telegram.cs

521fd25

Co-authored-by: Mark McDowall <markus.mcd5@gmail.com>

markus101 requested changes Apr 12, 2024

View reviewed changes

github-actions bot added the merge-conflict PR has merge conflicts that need to be resolved before it can be merged label Apr 16, 2024

		"NotificationsTelegramSettingsSendMetadataLink": "Add a link to the series metadata",
		"NotificationsTelegramSettingsSendMetadataLinkHelpText": "Add a link to the series metadata when sending the notifications",

	[FieldDefinition(4, Label = "NotificationsTelegramSettingsMetadataLinkType", Type = FieldType.Select, SelectOptions = typeof(MetadataLinkType), HelpText = "NotificationsTelegramSettingsMetadataLinkType")]
	[FieldDefinition(4, Label = "NotificationsTelegramSettingsMetadataLink", Type = FieldType.Select, SelectOptions = typeof(MetadataLinkType), HelpText = "NotificationsTelegramSettingsMetadataLink")]

	public MetadataLinkType MetadataLinkType { get; set; }
	public int MetadataLink { get; set; }

[Feature] Add clickable metadata links when using the Telegram connector. (IMDb / Trakt / TVDb, TVMaze) #6613

Are you sure you want to change the base?

[Feature] Add clickable metadata links when using the Telegram connector. (IMDb / Trakt / TVDb, TVMaze) #6613

Conversation

qTipTip commented Mar 9, 2024

Description

Screenshots for UI Changes

Issues Fixed or Closed by this PR

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus101 Mar 10, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevietv left a comment

Choose a reason for hiding this comment

qTipTip commented Mar 9, 2024

markus101 left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus101 Mar 10, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus101 commented Mar 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qTipTip commented Mar 11, 2024

markus101 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus101 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qTipTip commented Apr 29, 2024

markus101 Mar 10, 2024 •

edited

markus101 left a comment •

edited

markus101 Mar 10, 2024 •

edited