Uh oh!
There was an error while loading. Please reload this page.
fix(settings): carry mediaSegmentActions in the sync profile - #253
Open
ken-r-smith wants to merge 1 commit into
Open
fix(settings): carry mediaSegmentActions in the sync profile#253ken-r-smith wants to merge 1 commit into
ken-r-smith wants to merge 1 commit into
Conversation
MoonfinSettingsProfile names every field it will accept and has no JsonExtensionData, so a key it does not declare is dropped during deserialisation. mediaSegmentActions was never declared, which meant the client could push the value but the profile would never store or return it. Its two neighbours, mediaSegmentCountdown and mediaSegmentAutoHide, are already carried, so the setting that decides whether a skip prompt appears was the only one of the three that did not follow the user between clients. Added to the Jellyfin and Emby profiles alike, as a nullable string matching the client's comma separated `type:action` value. No merge logic references these properties by name, so declaring them is all that is required. Pairs with Moonfin-Core: per type media segment actions (#821).
There was a problem hiding this comment.
Pull request overview
This PR fixes settings sync persistence by adding a missing mediaSegmentActions field to the server-side MoonfinSettingsProfile models, preventing the value from being dropped during JSON deserialization and ensuring it round-trips between clients.
Changes:
- Added
mediaSegmentActions(string?) to the JellyfinMoonfinSettingsProfilemodel with the correctJsonPropertyName. - Added the same
mediaSegmentActionsproperty to the EmbyMoonfinSettingsProfilemodel, matching the existing single-line property style.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
Jellyfin/backend/Models/MoonfinSettingsProfile.cs | Adds mediaSegmentActions JSON-mapped property so the Jellyfin backend persists and returns the synced value. |
Emby/Emby.Plugins.Moonfin/Models/MoonfinSettingsProfile.cs | Adds the same mediaSegmentActions JSON-mapped property for the Emby plugin backend to persist and return the value. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
MoonfinSettingsProfiledeclares every field it will accept and carries no[JsonExtensionData], so any key it does not name is dropped duringdeserialisation.
mediaSegmentActionswas never declared, which means a clientcan push the value but the profile will not store or return it.
Its two neighbours are already carried:
So the countdown shown on a skip prompt, and how long the button lingers, both
follow the user between clients — while the setting that decides whether the
prompt appears at all does not. That looks like an oversight rather than a
decision, which is what this fixes.
Related Issues
Companion to the Moonfin-Core PR adding per media segment type actions. That one
registers
mediaSegmentActionsinsyncedFields; without the property here, theclient pushes the value and the server discards it. The client change is useful
on its own — this is what makes the choice follow the account instead of being
re-set on every device.
Type of Change
Changes Made
mediaSegmentActionsas a nullable string to the Jellyfin profile model,beside
mediaSegmentCountdown.property style.
The client stores this as a comma separated
type:actionstring(
intro:askToSkip,recap:askToSkip,outro:skip), sostring?matches its twoneighbours. No merge or override logic references these properties by name, so
declaring them is all that is needed.
Platform
Testing
I do not have a .NET toolchain set up for this repository, so this has not been
compiled. It is two property declarations with no logic, mirroring the
mediaSegmentCountdownlines directly above them in both files. Happy to have CIor a maintainer confirm the build, or to test a plugin build against a live
Jellyfin 10.11.11 server if that is useful.
Test Steps
Screenshots
Not applicable, no user facing surface.
Checklist