From 4fb68f4bbcf9f6247df5491d5c14cc4667da533c Mon Sep 17 00:00:00 2001 From: ponzischeme89 Date: Wed, 12 Aug 2026 15:39:08 +1200 Subject: [PATCH] 0.2.59 - Settings save fixes --- CHANGELOG.md | 6 ++ app/build.gradle.kts | 2 +- .../ponzischeme89/memby/ui/HomeMovieHero.kt | 35 ++++++++-- .../memby/ui/requests/RequestsScreen.kt | 12 ++-- .../memby/ui/requests/RequestsViewModel.kt | 20 ++++-- .../memby/ui/search/SearchScreen.kt | 9 +++ .../memby/ui/HomeMovieHeroTest.kt | 64 +++++++++++++++++++ .../ui/requests/RequestPresentationTest.kt | 5 +- server/internal/api/api.go | 20 ++++++ server/internal/api/hero.go | 20 ++++++ server/internal/api/hero_test.go | 21 ++++++ server/internal/api/images.go | 20 +++++- server/internal/buildinfo/VERSION | 2 +- server/internal/store/preferences.go | 16 +++-- 14 files changed, 227 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e100507..d1f4261 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.2.59 — 2026-08-12 +- Fixed: Moving up through the on-screen keyboard on the Requests page no longer jumps straight to the tabs — only the top row of letters leaves for them. +- Improved: Requests now waits for five letters, and for a short pause in typing, before looking anything up. +- Fixed: The featured cards at the top of Home no longer lead with a film you have already watched. Series are unchanged, so a new season of something you are up to date with still features. +- Fixed: Saving a setting could fail repeatedly with a server error. + ## 0.2.58 — 2026-08-12 - Added: Improved Requests option to the user selector menu. diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 7cb5970..880e420 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -42,7 +42,7 @@ val projectNoticeText = // A release workflow can derive the app version from its Git tag without editing the // source tree. Local builds keep using the checked-in default. -val defaultVersionName = "0.2.58" +val defaultVersionName = "0.2.59" val membyVersionName: String = (project.findProperty("memby.versionName") as String?) ?.trim() diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/HomeMovieHero.kt b/app/src/main/java/com/ponzischeme89/memby/ui/HomeMovieHero.kt index a743306..37bdd01 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/HomeMovieHero.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/HomeMovieHero.kt @@ -132,6 +132,27 @@ private const val LABEL_LIBRARY = "FROM YOUR LIBRARY" /** The `kind` of the server-composed hero row. It is consumed here, never drawn as a row. */ internal const val SERVER_HERO_ROW_KIND = "hero" +/** + * Whether a card is something to lead the launcher with tonight. + * + * A film somebody has already finished is not: the hero exists to be pressed, and the + * answer to "watch this" cannot be a title they watched last week. This holds however the + * card was chosen, and regardless of the *Hide films you have seen* preference — that one + * is about shelves, and a shelf of a hundred cards can afford to carry one somebody has + * seen where a row of four cannot. + * + * **A series is deliberately left alone.** Being marked watched there means somebody is up + * to date, which is exactly who a season premiere is news for — the whole reason the + * gateway can lead with a returning show. + * + * The gateway applies the same rule when it composes the row, so this is the second half of + * one decision rather than a disagreement with it: what it catches is the launcher a + * television draws from its cache before the first refresh lands, and a gateway older than + * the rule. + */ +internal fun heroWorthLeadingWith(item: BaseItem): Boolean = + !(item.isMovie && item.userData?.played == true) + /** * The hero the gateway composed, if it sent one. * @@ -151,6 +172,7 @@ internal fun serverHeroPicks(rows: List): List = .filter { it.kind == SERVER_HERO_ROW_KIND } .flatMap { it.items.asSequence() } .filter { it.membyPlayable } + .filter(::heroWorthLeadingWith) .distinctBy(BaseItem::id) .take(4) .map { item -> @@ -191,17 +213,20 @@ internal fun selectHomeHeroMovies( return words.any(label::contains) } + // Unwatched throughout: see heroWorthLeadingWith. It is applied to each list rather + // than to the rows so the rotation still counts in cards that can actually lead. + fun List.unwatchedMovies() = + flatMap(HomeBrowseRow::items).filter(BaseItem::isMovie).filter(::heroWorthLeadingWith) + val newReleases = rows .filter { it.matches("latest", "recent", "new release", "just added") } - .flatMap(HomeBrowseRow::items) - .filter(BaseItem::isMovie) + .unwatchedMovies() .rotatedBy(day) val popular = rows .filter { it.matches("popular", "trending", "recommended", "top pick") } - .flatMap(HomeBrowseRow::items) - .filter(BaseItem::isMovie) + .unwatchedMovies() .rotatedBy(day) - val everyMovie = rows.flatMap(HomeBrowseRow::items).filter(BaseItem::isMovie).rotatedBy(day) + val everyMovie = rows.unwatchedMovies().rotatedBy(day) fun List.labelled(label: String) = map { HomeHeroPick(it, label) } diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/requests/RequestsScreen.kt b/app/src/main/java/com/ponzischeme89/memby/ui/requests/RequestsScreen.kt index b4b99e5..5c07d84 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/requests/RequestsScreen.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/requests/RequestsScreen.kt @@ -459,11 +459,12 @@ private fun DiscoverPane( QueryLine(state) Spacer(Modifier.height(12.dp)) Box( - Modifier - // The keyboard is the pane's entry point and its top-left control, so - // Down from the tabs lands on it and Up from it goes back. - .focusRequester(paneFocusRequester) - .focusProperties { up = tabsFocusRequester }, + // The keyboard is the pane's entry point, so Down from the tabs lands on it. + // Only the *entry* is declared here: an `up` hung off this Box would be + // inherited by every key underneath it, which is what made Up anywhere in + // the letters leave for the tab strip instead of moving one row up. The + // escape belongs to the top row alone, and the keyboard states it per key. + Modifier.focusRequester(paneFocusRequester), ) { TvKeyboard( navigationFocusRequester = navigationFocusRequester, @@ -476,6 +477,7 @@ private fun DiscoverPane( onCharacter = onAppendToQuery, onBackspace = onBackspace, onClear = onClearQuery, + upTarget = tabsFocusRequester, ) } } diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/requests/RequestsViewModel.kt b/app/src/main/java/com/ponzischeme89/memby/ui/requests/RequestsViewModel.kt index c76a63d..16907de 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/requests/RequestsViewModel.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/requests/RequestsViewModel.kt @@ -267,16 +267,24 @@ class RequestsViewModel(private val repository: EmbyRepository) : ViewModel() { fun dismissNotice() = _state.update { it.copy(notice = null) } companion object { - const val DEBOUNCE_MS = 300L + /** + * Longer than the Search tab's, because the thing at the other end is different. + * Typing on a remote is around half a second a letter, so a 300 ms window let every + * keystroke past the threshold through as its own live Radarr and Sonarr query. + */ + const val DEBOUNCE_MS = 700L /** - * Three characters, not the Search tab's two. + * Five characters, not the Search tab's two. * - * Every keystroke here reaches Radarr and Sonarr rather than a local index, and a - * two-letter prefix returns a hundred films nobody meant while costing two live - * provider queries. The gateway independently refuses under two. + * Every keystroke here reaches Radarr and Sonarr rather than a local index, so a + * viewer typing a title on a remote spends a live provider query per letter — and + * the early ones are worthless: a three-letter prefix returns a hundred films + * nobody meant. Five is roughly where a prefix starts naming something, so the + * lookups begin when they can answer rather than as soon as they are legal. The + * gateway independently refuses under two. */ - const val MIN_QUERY_LENGTH = 3 + const val MIN_QUERY_LENGTH = 5 } } diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt b/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt index 76e3ae7..cf321f3 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt @@ -511,6 +511,14 @@ internal fun TvKeyboard( onCharacter: (String) -> Unit, onBackspace: () -> Unit, onClear: () -> Unit, + /** + * Where Up goes from the **top row of letters**, for a host that has something above the + * keyboard — the Requests page's tab strip. It is stated per key rather than declared on + * a parent, because `focusProperties` is inherited by every descendant: hung off a Box + * around the keyboard it applies to all thirty-nine keys, so Up anywhere in the letters + * leaves for the tabs instead of moving one row up. + */ + upTarget: FocusRequester? = null, ) { Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { KeyboardRows.forEachIndexed { rowIndex, row -> @@ -532,6 +540,7 @@ internal fun TvKeyboard( if (columnIndex == KEYBOARD_COLUMNS - 1) { right = if (hasResultsTarget) resultsEntry else FocusRequester.Cancel } + if (rowIndex == 0) upTarget?.let { up = it } } .then( if (index == 0) Modifier.focusRequester(keyboardEntry) else Modifier, diff --git a/app/src/test/java/com/ponzischeme89/memby/ui/HomeMovieHeroTest.kt b/app/src/test/java/com/ponzischeme89/memby/ui/HomeMovieHeroTest.kt index 522684c..e58a595 100644 --- a/app/src/test/java/com/ponzischeme89/memby/ui/HomeMovieHeroTest.kt +++ b/app/src/test/java/com/ponzischeme89/memby/ui/HomeMovieHeroTest.kt @@ -5,6 +5,7 @@ import com.ponzischeme89.memby.data.localEpochDay import com.ponzischeme89.memby.data.millisUntilNextLocalDay import com.ponzischeme89.memby.data.model.BaseItem import com.ponzischeme89.memby.data.model.HomeRow +import com.ponzischeme89.memby.data.model.UserItemData import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test @@ -259,6 +260,58 @@ class HomeMovieHeroTest { } } + // --- Watched films ------------------------------------------------------------- + + /** + * The hero is four cards answering "watch this tonight". A film already finished is + * not an answer to that, however new it is or however well it was reviewed. + */ + @Test + fun `a film already watched never leads the launcher`() { + val rows = listOf( + HomeBrowseRow( + id = "latest-movies", + title = "Recently Added Movies", + items = listOf( + movie("seen", watched = true), + movie("partway", positionTicks = 6_000_000_000L), + movie("unseen"), + ), + kind = MediaRowKind.MOVIES, + emptyMessage = "", + ), + ) + + assertEquals( + listOf("partway", "unseen"), + selectHomeHeroMovies(rows).map { it.item.id }, + ) + } + + /** + * The gateway applies the same rule, so this is the launcher a television draws from + * its cache before the first refresh — and any gateway older than the rule. + */ + @Test + fun `a watched film is dropped from the row the gateway sent`() { + val row = heroRow( + movie("seen", watched = true).copy(membyHeroLabel = "NEW RELEASE"), + movie("unseen").copy(membyHeroLabel = "NEW RELEASE"), + ) + + assertEquals(listOf("unseen"), serverHeroPicks(listOf(row)).map { it.item.id }) + } + + /** A show marked watched is somebody up to date — exactly who a new season is for. */ + @Test + fun `a watched series still leads`() { + val row = heroRow( + movie("show", watched = true).copy(type = "Series", membyHeroLabel = "NEW SEASON"), + ) + + assertEquals(listOf("show"), serverHeroPicks(listOf(row)).map { it.item.id }) + } + // --- Midnight ---------------------------------------------------------------- /** Local, not UTC: the day must turn over at the viewer's midnight. */ @@ -308,6 +361,17 @@ class HomeMovieHeroTest { const val DAY_MS = 24L * HOUR_MS } + private fun movie( + id: String, + watched: Boolean = false, + positionTicks: Long = 0L, + ) = BaseItem( + id = id, + name = id, + type = "Movie", + userData = UserItemData(played = watched, playbackPositionTicks = positionTicks), + ) + private fun heroItem(id: String, label: String?, reason: String?) = BaseItem( id = id, name = id, diff --git a/app/src/test/java/com/ponzischeme89/memby/ui/requests/RequestPresentationTest.kt b/app/src/test/java/com/ponzischeme89/memby/ui/requests/RequestPresentationTest.kt index 039db52..c795ee9 100644 --- a/app/src/test/java/com/ponzischeme89/memby/ui/requests/RequestPresentationTest.kt +++ b/app/src/test/java/com/ponzischeme89/memby/ui/requests/RequestPresentationTest.kt @@ -95,9 +95,10 @@ class RequestPresentationTest { fun `the lookup threshold is stricter than the library search's`() { // Every keystroke here reaches Radarr and Sonarr rather than a local index. assertFalse(shouldLookup("du")) - assertTrue(shouldLookup("dun")) + assertFalse(shouldLookup("dune")) + assertTrue(shouldLookup("dunes")) assertFalse(shouldLookup(" a ")) - assertTrue(shouldLookup(" dune ")) + assertTrue(shouldLookup(" dune part two ")) } @Test diff --git a/server/internal/api/api.go b/server/internal/api/api.go index 909d78a..5d1fccd 100644 --- a/server/internal/api/api.go +++ b/server/internal/api/api.go @@ -414,6 +414,10 @@ func clientLogValue(value string) string { // visible regardless of path. func requestLogLevel(path string, status int) slog.Level { switch { + // A request nobody is waiting for any more is not a failure of anything. It is only + // ever answered this way deliberately, so it never hides a fault. + case status == statusClientClosedRequest: + return slog.LevelDebug case status >= http.StatusInternalServerError: return slog.LevelError case status >= http.StatusBadRequest: @@ -557,6 +561,11 @@ func writeRaw(w http.ResponseWriter, status int, body []byte) { _, _ = w.Write(body) } +// statusClientClosedRequest is nginx's 499. Go has no constant for it because it is not +// in the RFC — it exists to say "this was not answered, and that is nobody's fault", +// which is a distinction a log is read for and a 5xx destroys. +const statusClientClosedRequest = 499 + func writeError(w http.ResponseWriter, status int, message string) { writeJSON(w, status, map[string]string{"error": message}) } @@ -566,6 +575,17 @@ func writeError(w http.ResponseWriter, status int, message string) { func (s *Server) writeUpstreamError( ctx context.Context, w http.ResponseWriter, err error, message string, ) { + // The television having navigated on is not a fault, and it is the ordinary case here: + // artwork loaders abandon requests as cards leave the screen, and a detail page warmed + // on focus is cancelled the moment the D-pad moves. Reported as 502 it filled the + // operator's log with errors describing a launcher working exactly as designed, and + // buried the ones that meant something. Nobody is left to read the answer, so it goes + // out as 499 — nginx's "client closed request" — and is recorded at DEBUG. + if clientGaveUp(ctx, err) { + s.loggerFor(ctx).Debug("abandoned before the answer", "detail", message, "error", err) + writeError(w, statusClientClosedRequest, "the request was abandoned") + return + } var apiErr *emby.APIError if errors.As(err, &apiErr) { switch { diff --git a/server/internal/api/hero.go b/server/internal/api/hero.go index a8946a6..48a2182 100644 --- a/server/internal/api/hero.go +++ b/server/internal/api/hero.go @@ -452,6 +452,10 @@ type heroItemFacts struct { Type string Premiere time.Time Playable bool + + // Watched is Emby's own answer for this viewer. The rows are fetched per person and + // carry their user data, so it costs nothing to read. + Watched bool } func heroFactsOf(raw json.RawMessage) (heroItemFacts, bool) { @@ -462,6 +466,9 @@ func heroFactsOf(raw json.RawMessage) (heroItemFacts, bool) { PremiereDate string `json:"PremiereDate"` Source string `json:"MembySource"` Playable *bool `json:"MembyPlayable"` + UserData struct { + Played bool `json:"Played"` + } `json:"UserData"` } if json.Unmarshal(raw, &payload) != nil || strings.TrimSpace(payload.ID) == "" { return heroItemFacts{}, false @@ -474,6 +481,7 @@ func heroFactsOf(raw json.RawMessage) (heroItemFacts, bool) { // Anything from Emby carries neither field, and is. Playable: strings.TrimSpace(payload.Source) == "" && (payload.Playable == nil || *payload.Playable), + Watched: payload.UserData.Played, } if parsed, err := parseEmbyDate(payload.PremiereDate); err == nil { facts.Premiere = parsed @@ -837,6 +845,18 @@ func heroMovieCandidates(rows []recommend.Row) ([]heroCandidate, map[string]hero !strings.EqualFold(fact.Type, "Movie") { continue } + // A film somebody has already seen is not something to lead the launcher + // with: the hero exists to be pressed, and the answer to "watch this + // tonight" cannot be a film that finished last week. It is only ever the + // whole title here — a film halfway through belongs to Continue Watching, + // which is not a candidate row at all. + // + // Series are deliberately not filtered this way. A show marked watched is + // one somebody is up to date with, which is exactly who a season premiere + // is news for, and premieres come from Sonarr rather than from here. + if fact.Watched { + continue + } seen[fact.ID] = true facts[fact.ID] = fact rating, rated := heroRatingOf(raw) diff --git a/server/internal/api/hero_test.go b/server/internal/api/hero_test.go index f5e1b5c..d6fa0b8 100644 --- a/server/internal/api/hero_test.go +++ b/server/internal/api/hero_test.go @@ -357,6 +357,27 @@ func TestHeroMovieCandidatesSkipUnpressableRows(t *testing.T) { } } +// A film somebody has finished is not an answer to "watch this tonight", however new or +// well reviewed it is. A series is left alone: watched there means up to date, which is +// exactly who a returning season is news for. +func TestHeroMovieCandidatesSkipWatchedFilms(t *testing.T) { + watched, _ := json.Marshal(map[string]any{ + "Id": "seen", "Name": "Seen", "Type": "Movie", + "UserData": map[string]any{"Played": true}, + }) + partway, _ := json.Marshal(map[string]any{ + "Id": "partway", "Name": "Partway", "Type": "Movie", + "UserData": map[string]any{"Played": false, "PlaybackPositionTicks": 6_000_000_000}, + }) + rows := []recommend.Row{{ID: "latest", Kind: "latest", Items: []json.RawMessage{ + watched, partway, heroItem("unseen", "Unseen", "Movie"), + }}} + candidates, _ := heroMovieCandidates(rows) + if ids := strings.Join(heroIDs(candidates), ","); ids != "partway,unseen" { + t.Fatalf("expected the unfinished and unseen films only, got %s", ids) + } +} + // Emby writes dates in more than one shape, and one it will not parse is unknown rather // than fatal. func TestParseEmbyDate(t *testing.T) { diff --git a/server/internal/api/images.go b/server/internal/api/images.go index e188537..d641f56 100644 --- a/server/internal/api/images.go +++ b/server/internal/api/images.go @@ -110,6 +110,10 @@ func (s *Server) handleRadarrImage(w http.ResponseWriter, r *http.Request, itemI writeError(w, http.StatusNotFound, "image not found") return } + if clientGaveUp(r.Context(), err) { + writeError(w, statusClientClosedRequest, "the request was abandoned") + return + } s.log.Warn("radarr image failed", "movie_id", movieID, "type", coverType, "error", err) writeError(w, http.StatusBadGateway, "could not load the image") return @@ -154,6 +158,10 @@ func (s *Server) handleSonarrImage(w http.ResponseWriter, r *http.Request, itemI writeError(w, http.StatusNotFound, "image not found") return } + if clientGaveUp(r.Context(), err) { + writeError(w, statusClientClosedRequest, "the request was abandoned") + return + } s.log.Warn("sonarr image failed", "series_id", seriesID, "type", coverType, "error", err) writeError(w, http.StatusBadGateway, "could not load the image") return @@ -192,7 +200,17 @@ func copyImage( } func expectedClientDisconnect(r *http.Request, err error) bool { - if r.Context().Err() != nil || + return clientGaveUp(r.Context(), err) +} + +// clientGaveUp reports whether a failure is the television having walked away rather than +// anything being wrong here. +// +// A cancellation is the *only* thing it treats as such. A deadline is our own patience +// running out, which is a real failure with a real cause; conflating the two would hide +// exactly the timeouts worth seeing. +func clientGaveUp(ctx context.Context, err error) bool { + if errors.Is(ctx.Err(), context.Canceled) || errors.Is(err, context.Canceled) || errors.Is(err, net.ErrClosed) || errors.Is(err, syscall.EPIPE) || diff --git a/server/internal/buildinfo/VERSION b/server/internal/buildinfo/VERSION index 9dd1793..a52e041 100644 --- a/server/internal/buildinfo/VERSION +++ b/server/internal/buildinfo/VERSION @@ -1 +1 @@ -0.1.34 +0.1.35 diff --git a/server/internal/store/preferences.go b/server/internal/store/preferences.go index 6990359..a173b4c 100644 --- a/server/internal/store/preferences.go +++ b/server/internal/store/preferences.go @@ -171,10 +171,18 @@ func (s *Store) SetUserPreferences( // Pruned here rather than on a schedule: this is the only writer, so it is the only // place the table can grow, and the acks of a revision nobody can see any more are // dead weight with it. + // + // The cutoff is worked out in Go rather than as `$2 - $3` in the statement. Postgres + // has to infer a type for every placeholder, and two of them either side of an + // operator give it nothing to infer from — it answers `operator is not unique: + // unknown - unknown`, which failed the whole transaction and returned 500 to every + // television trying to save a setting. Arithmetic on two parameters belongs on this + // side of the wire. + pruneBelow := next.Revision - preferenceHistoryLimit if _, err := tx.Exec(ctx, ` DELETE FROM user_preference_revisions - WHERE emby_user_id = $1 AND revision <= $2 - $3`, - userID, next.Revision, preferenceHistoryLimit, + WHERE emby_user_id = $1 AND revision <= $2`, + userID, pruneBelow, ); err != nil { return UserPreferences{}, fmt.Errorf("store: prune preference history: %w", err) } @@ -183,13 +191,13 @@ func (s *Store) SetUserPreferences( // described as "on revision 12" rather than as one that has never checked in. if _, err := tx.Exec(ctx, ` DELETE FROM user_preference_acks stale - WHERE stale.emby_user_id = $1 AND stale.revision <= $2 - $3 + WHERE stale.emby_user_id = $1 AND stale.revision <= $2 AND EXISTS ( SELECT 1 FROM user_preference_acks newer WHERE newer.emby_user_id = stale.emby_user_id AND newer.device_id = stale.device_id AND newer.revision > stale.revision)`, - userID, next.Revision, preferenceHistoryLimit, + userID, pruneBelow, ); err != nil { return UserPreferences{}, fmt.Errorf("store: prune preference acks: %w", err) }