Add Sync/Share tabbed settings UI and Intro Skipper-backed sharing with local dedupe#24
Conversation
Agent-Logs-Url: https://github.com/intro-skipper/skipme.db-plugin/sessions/14fddb35-f3cb-40fa-b81c-64a0ef157d51 Co-authored-by: AbandonedCart <1173913+AbandonedCart@users.noreply.github.com>
Agent-Logs-Url: https://github.com/intro-skipper/skipme.db-plugin/sessions/14fddb35-f3cb-40fa-b81c-64a0ef157d51 Co-authored-by: AbandonedCart <1173913+AbandonedCart@users.noreply.github.com>
Agent-Logs-Url: https://github.com/intro-skipper/skipme.db-plugin/sessions/14fddb35-f3cb-40fa-b81c-64a0ef157d51 Co-authored-by: AbandonedCart <1173913+AbandonedCart@users.noreply.github.com>
| [Route("SkipMeDb")] | ||
| public sealed class ShareController(ShareSubmissionService shareSubmissionService) : ControllerBase | ||
| { | ||
| private readonly ShareSubmissionService _shareSubmissionService = shareSubmissionService; |
There was a problem hiding this comment.
WARNING: Redundant field assignment with primary constructor
C# 12+ primary constructor parameters are automatically captured. Explicit _shareSubmissionService field declaration creates unnecessary memory allocation and duplication. You can remove the field entirely and use the primary constructor parameter directly.
| public sealed class ShareSubmitRequest | ||
| { | ||
| /// <summary>Gets the filtered series IDs currently visible in the UI.</summary> | ||
| public Collection<string> FilteredSeriesIds { get; } = []; |
There was a problem hiding this comment.
CRITICAL: Mutable request model properties
Getter-only Collection<T> properties are still mutable when deserialized by System.Text.Json. Request objects should be immutable. Use IReadOnlyCollection<T> with [JsonInclude].
| public sealed class ShareSubmitResponse | ||
| { | ||
| /// <summary>Gets or sets a value indicating whether at least one submission request succeeded.</summary> | ||
| public bool Ok { get; set; } |
There was a problem hiding this comment.
WARNING: Mutable response DTO properties
Response objects should be immutable. Use init; instead of set; for all properties.
| /// <param name="DurationMs">Item duration in milliseconds.</param> | ||
| public sealed record SharedUploadFingerprint( | ||
| Guid ItemId, | ||
| string Segment, |
There was a problem hiding this comment.
SUGGESTION: Segment should be strongly typed enum
Using string for known segment types prevents compiler validation, introduces case sensitivity bugs, and allocates unnecessary string copies.
| serviceCollection.AddHttpClient(nameof(SkipMeApiClient)); | ||
| serviceCollection.AddSingleton<SkipMeApiClient>(); | ||
| serviceCollection.AddSingleton<SegmentStore>(); | ||
| serviceCollection.AddSingleton<ShareSubmissionService>(); |
There was a problem hiding this comment.
WARNING: Incorrect service lifetime
ShareSubmissionService depends on scoped/transient services like ILibraryManager but is registered as Singleton. This will cause captured disposed context bugs and memory leaks. Should be registered as Scoped.
Code Review SummaryStatus: 21 Issues Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (8 files)
Reviewed by seed-2-0-pro-260328 · 370,685 tokens |
…w issues Agent-Logs-Url: https://github.com/intro-skipper/skipme.db-plugin/sessions/fe5eed29-24a8-420e-b411-44f5d9866d59 Co-authored-by: AbandonedCart <1173913+AbandonedCart@users.noreply.github.com>
This update introduces a dual-mode settings experience: Sync for existing enable/disable config management and Share for submitting enabled timestamps to
https://db.skipme.workers.dev. Sharing now separates TV and movie payloads by endpoint contract and prevents duplicate re-submission using local history.UI: tabbed Sync/Share workflow
Backend: Share API for filtered enabled items
POST /SkipMeDb/Shareendpoint.POST /v1/submit/seasonPOST /v1/submit/collectionTimestamp source + conversion parity with Intro Skipper
introskipper.db), using the same segment typing/mapping used for Jellyfin-facing segments.Local share-history dedupe in existing plugin DB
skipme-segments.dbschema withSharedUploads.start_ms,end_ms, andduration_msare each within ±1000ms is treated as already shared.New types/services
Example payload shaping (TV vs movies):