Skip to content

fix(playlist): make catalogue refresh reachable, and stop it wiping watch history - #415

Open
Pierroons wants to merge 4 commits into
oxyroid:masterfrom
Pierroons:fix/unreachable-refresh
Open

fix(playlist): make catalogue refresh reachable, and stop it wiping watch history#415
Pierroons wants to merge 4 commits into
oxyroid:masterfrom
Pierroons:fix/unreachable-refresh

Conversation

@Pierroons

Copy link
Copy Markdown
Contributor

Updating a playlist currently means subscribing to it again — retyping the
address and the credentials — even though a working refresh has been wired all
along. Three commits, each standing on its own.

1. Screen actions the top bar never renders

PlaylistScreen publishes a sort and a refresh into Metadata.actions, wired
down to SubscriptionWorker. Neither has ever been reachable.

Those actions are only rendered by the TopAppBar, and App.kt renders it only
when shouldShowContextualTopBar holds — isRootPlaylistConfiguration || isNestedDetailVisible. On the playlist screen both are false and the search bar
takes its place, so the actions were declared, wired, and impossible to invoke.

The search bar already drew an overflow icon, and that icon did nothing at all:
a bare Icon, no IconButton, no onClick. It now opens a menu built from
Metadata.actions, so any screen publishing actions gets them with no further
change there. Hidden while the search is expanded, and absent entirely when a
screen publishes none rather than opening an empty menu.

2. Watch history and favourites survive a refresh

A refresh deletes every channel and imports them anew, so rows came back with
seen reset and the Continue watching row emptied. Favourites survived only
under PlaylistStrategy.KEEP, which is not the default.

What the viewer built up is now captured before the delete and handed to the
rows replacing them, matched on relation_id — or on the URL, for an M3U
playlist without tvg-id. Only rows differing from a fresh import are carried:
a catalogue runs to tens of thousands of channels, of which a handful were ever
watched, and taking the rest would mean holding it all in memory to restore
nothing.

This also fixes a crash that the unreachable button had kept hidden.
WorkManager calls getForegroundInfo before doWork, and doWork was where
the notification channel got created, so the first refresh of an install handed
startForeground a notification pointing at a channel that did not exist:

RemoteServiceException: Bad notification for startForeground:
invalid channel for service notification

That killed the process mid-import. It went unnoticed because the channel
survives once created.

data/.gitignore excluded /src/test, so the module could not carry unit
tests at all; the entry is dropped and seven come with this change.

3. Refresh every playlist from the home screen

Refreshing acts on one playlist, so keeping a few sources current meant opening
each in turn — from the screen that already lists them side by side. The home
screen gets a refresh of its own, skipping sources that cannot be refreshed.

Verification

Commits 1 and 2 were exercised on device (Android 9, arm64) against a 40 971
channel Xtream catalogue: the menu opens with Sort and Refresh, a full refresh
runs without re-entering credentials, and afterwards the channel counts return
identical, the 22 watched channels keep their timestamps, and Continue watching
still shows the same title. Commit 3 is covered by unit tests and a build only.

110 unit tests pass.

Pierroons and others added 3 commits August 9, 2026 08:57
The playlist screen publishes a sort and a refresh into Metadata.actions,
fully wired down to SubscriptionWorker. Neither has ever been reachable.

Those actions are only rendered by the TopAppBar, and App.kt renders it
only when shouldShowContextualTopBar holds — that is,
isRootPlaylistConfiguration || isNestedDetailVisible. On the playlist
screen both are false and the search bar takes its place, so the actions
were declared, wired, and impossible to invoke. Updating a catalogue meant
subscribing to it again, retyping the address and credentials.

The search bar already drew an overflow icon, and that icon did nothing at
all: a bare Icon, no IconButton, no onClick. It now opens a menu built from
Metadata.actions — generic, so any screen publishing actions gets them with
no further change here, and the sort comes back along with the refresh.

Hidden when the search is expanded, since the actions belong to the screen
underneath, and absent entirely when a screen publishes none rather than
opening an empty menu.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Refreshing a catalogue deletes every channel and imports it anew, so rows
came back with seen reset and the Continue watching row emptied. Favourites
survived only under PlaylistStrategy.KEEP, and KEEP is not the default.

What a viewer built up is now captured before the delete and handed to the
rows that replace them, matched on relation_id — or on the URL, for an M3U
playlist without tvg-id, which has no stable id to match on.

Only rows that differ from a fresh import are carried: a catalogue runs to
tens of thousands of channels, of which a handful were ever watched. Taking
the rest would mean holding the whole thing in memory to restore nothing.

Also fixes a crash this made reachable for the first time. WorkManager
calls getForegroundInfo before doWork, and doWork was where the
notification channel got created, so the first refresh of an install handed
startForeground a notification pointing at a channel that did not exist:

  RemoteServiceException: Bad notification for startForeground:
  invalid channel for service notification

That killed the process mid-import. It went unnoticed because the channel
survives once created — and because nothing on screen could trigger a
refresh to begin with.

Verified on device, 40 971 channels: after a full refresh the counts return
identical, the 22 watched channels keep their timestamps, Continue watching
still shows the same title, and the 28 154 cached descriptions are intact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Refreshing lives on the playlist screen and acts on the one playlist it
shows, so keeping a few sources up to date means opening each of them in
turn — from the very screen that already lists them all side by side.

The home screen now offers a refresh of its own, reusing
PlaylistRepository.refresh for each source. Playlists that cannot be
refreshed at all, such as one imported from a local file, are skipped
rather than reported as failing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@oxyroid oxyroid left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I just left a few comments. Not all of them necessarily need changes — I’m happy to discuss any of them.

viewModelScope.launch {
playlistRepository.getAll()
.filter { playlist -> playlist.refreshable }
.forEach { playlist -> playlistRepository.refresh(playlist.url) }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Maybe it would be better to do these in a single database transaction?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question, but I don't think there's anything to wrap here — please tell me if I misread you.

The loop doesn't write to the database. PlaylistRepository.refresh enqueues a SubscriptionWorker per playlist and returns; the only read is the initial getAll(). A transaction would enclose no writes, and holding one open across enqueue calls would keep it open for the WorkManager round-trips.

If your concern was that a playlist added while the loop runs could be missed — that's real, and a snapshot read would fix it. Happy to do that if it's what you had in mind.

*/
@Composable
private fun ScreenActionsMenu(modifier: Modifier = Modifier) {
val actions = Metadata.actions

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I’m considering refactoring this because having seemingly static, non-composable parameters trigger recompositions may be confusing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, and it bothered me too. The composable takes no parameters yet recomposes, because it reads Metadata.actions — a global mutableStateOf — from inside. Nothing in its signature says so.

I'll pass the actions in from AppImpl, which already reads that state, so the dependency is visible and the composable becomes a function of its arguments again.

Let me know if you'd rather refactor Metadata itself — I'd leave that to you, since it's used well beyond this screen.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 8580bb8 — the caller passes the list in, so what makes it recompose is in the signature now.

…data

The composable took no parameters and recomposed anyway, because it read
Metadata.actions from inside — a global mutableStateOf. Nothing in its
signature said so, which is confusing for a reader trying to work out what
makes it redraw.

The caller already sits where that state is read, so it passes the list in.
The composable is a function of its arguments again, and stays just as
generic: any screen publishing actions still gets them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants