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

Plex webhook episode detection for non-episode videos #1126

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

LordFlashmeow
Copy link
Contributor

@LordFlashmeow LordFlashmeow commented May 11, 2024

Use title + episode number (or TVDB season + episode #) for non-Episode type videos.

Use Plex API to fetch the filename of the watched video file (for non-episode type videos).

This should allow the Plex webhook to mark specials and other video types as watched.

There is a corner case where the user has multiple episodes with the same file name in a series, handling that is TBD.

@harshithmohan harshithmohan requested a review from Cazzar May 12, 2024 05:54
Shoko.Server/API/v2/Modules/PlexWebhook.cs Outdated Show resolved Hide resolved
Shoko.Server/API/v2/Modules/PlexWebhook.cs Outdated Show resolved Hide resolved
@LordFlashmeow LordFlashmeow force-pushed the master branch 2 times, most recently from 7209b1d to 0a3f36d Compare May 12, 2024 22:11
@LordFlashmeow LordFlashmeow requested a review from Cazzar May 13, 2024 23:21
@@ -60,20 +62,29 @@ public ActionResult WebhookPost([FromForm] [ModelBinder(BinderType = typeof(Plex
}

_logger.LogTrace($"{payload.Event}: {payload.Metadata?.Guid}");

var user = User;
user ??= RepoFactory.JMMUser.GetAll().FirstOrDefault(u => payload.Account.Title.FindIn(u.GetPlexUsers()));
Copy link
Member

Choose a reason for hiding this comment

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

Instead of passing the user around and having additional parameters, would it be possibly better to have a cached JMMUser property on the payload?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would that be done? (Sorry, I haven't worked with C# before)

Shoko.Server/API/v2/Modules/PlexWebhook.cs Outdated Show resolved Hide resolved
Comment on lines +187 to +190
var key = metadata.Key.Split("/").LastOrDefault(); // '/library/metadata/{key}'

var plexEpisode = (SVR_Episode)GetPlexEpisodeData(key, user);
var episode = plexEpisode?.AnimeEpisode;
Copy link
Member

Choose a reason for hiding this comment

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

Can we add this with a guard against people not having a valid plex connection?

The original design was that the webhook did not require user plex login, so it was entirely working off it's own data.

Happy to add this in, but I think it should be at least guarded to allow for users to not have their own plex config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you clarify the scenario you're describing? The user has plex webhooks enabled, but hasn't linked their plex account to Shoko?

If that's the case, I suppose I could fall back original behavior of only matching episodes.

Shoko.Server/API/v2/Modules/PlexWebhook.cs Outdated Show resolved Hide resolved
Shoko.Server/API/v2/Modules/PlexWebhook.cs Outdated Show resolved Hide resolved
Shoko.Server/API/v2/Modules/PlexWebhook.cs Outdated Show resolved Hide resolved
/// <returns>the AnimeEpisode given the file information</returns>
public SVR_AnimeEpisode GetByFilename(string name)
public SVR_AnimeEpisode GetByFilename(string name, long? size = null)
Copy link
Member

Choose a reason for hiding this comment

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

Consider renaming this, or adding an extra helper to make this clearer.

Copy link
Contributor Author

@LordFlashmeow LordFlashmeow Jun 12, 2024

Choose a reason for hiding this comment

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

I split it into 2 functions, GetByFilename(string name) and GetByFilenameAndSize(string name, long size) is that suitable?

Shoko.Server/Plex/Collection/SVR_PlexLibrary.cs Outdated Show resolved Hide resolved
Shoko.Server/API/v2/Modules/PlexWebhook.cs Outdated Show resolved Hide resolved
@Cazzar
Copy link
Member

Cazzar commented Jun 11, 2024

Sorry for the delay reviewing this in proper, only got internet proper today.

@ElementalCrisis
Copy link
Member

@LordFlashmeow can you update your PR please and resolve conflicts.

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.

3 participants