Skip to content

Bookmark (Lesson / Playlist)#168

Open
Mandvii wants to merge 128 commits into
mainfrom
dev-bookmark
Open

Bookmark (Lesson / Playlist)#168
Mandvii wants to merge 128 commits into
mainfrom
dev-bookmark

Conversation

@Mandvii

@Mandvii Mandvii commented Feb 20, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@Mandvii Mandvii requested a review from mikedawson February 20, 2026 05:25
@Mandvii Mandvii linked an issue Feb 20, 2026 that may be closed by this pull request
Key changes include:
- Updating the `OpdsDataSource` interface and its implementations (`OpdsDataSourceRepository`, `OpdsDataSourceDb`) to handle observing and setting bookmark status.
- Adding a `BookmarkEntity` and `BookmarkDao` to the database schema to persist bookmark state.
- Implementing the bookmark logic in the `LearningUnitDetailViewModel`, which now observes and updates the bookmark status.
- The bookmark button in `LearningUnitDetailScreen` now dynamically changes its icon based on whether the unit is bookmarked.
Introduced the data layer components required for managing publication bookmarks.

This includes:
- A `BookmarkEntity` to store bookmark status and timestamp.
- A `BookmarkDao` with functions to insert/update bookmarks, observe a single bookmark's status, and retrieve all bookmarked publications.
- An implementation in `OpdsDataSourceDb` to get a flow of bookmarked `OpdsPublication` objects.
- An update to `ReadiumLinkEntityDao` to find all links associated with a publication UID.
This commit introduces the functionality to retrieve a list of all bookmarked publications from the local database.

Key changes:

-   Added `getBookmarkedPublications()` to the `OpdsDataSource` interface.
-   Implemented the new function in `OpdsDataSourceRepository` to delegate the call to the local data source.
-   In `OpdsDataSourceDb`, the implementation now fetches bookmarked publication entities from `BookmarkDao` and maps them to `OpdsPublication` models.
-   Updated the `OpdsDataSourceHttp` stub to include the new interface method.
This change enhances the bookmarking functionality by storing the title of the bookmarked item and implicitly tracking when it was last updated.

Key changes include:
- Updating the `setBookmarkStatus` function across the `OpdsDataSource` interface and its implementations (`OpdsDataSourceDb`, `OpdsDataSourceRepository`, `OpdsDataSourceHttp`) to accept a `title` parameter.
- Modifying the `BookmarkDao` to no longer retrieve `OpdsPublicationEntity` objects directly. Instead, a new `observeAllBookmarks` function returns a flow of `BookmarkEntity` objects, ordered by their update timestamp.
- The `LearningUnitDetailViewModel` now passes the lesson title when setting the bookmark status.
- The `OpdsDataSourceDb` was updated to implement these DAO changes and now provides a `getAllBookmarks` function.
This commit introduces the functionality to retrieve a list of all bookmarks.
This commit refactors the bookmarking functionality. Instead of explicitly passing a boolean to set the bookmark status, the `setBookmarkStatus` function now toggles the state. If a bookmark exists, it's removed; otherwise, a new one is created.

This change simplifies the bookmarking logic across the data layers and the view model. Additionally, the bookmark list screen now allows for the removal of bookmarks.
This commit refactors the bookmarking functionality. Instead of explicitly passing a boolean to set the bookmark status, the `setBookmarkStatus` function now toggles the state. If a bookmark exists, it's removed; otherwise, a new one is created.

This change simplifies the bookmarking logic across the data layers and the view model. Additionally, the bookmark list screen now allows for the removal of bookmarks.
This commit refactors the bookmarking functionality. Instead of explicitly passing a boolean to set the bookmark status, the `setBookmarkStatus` function now toggles the state. If a bookmark exists, it's removed; otherwise, a new one is created.

This change simplifies the bookmarking logic across the data layers and the view model. Additionally, the bookmark list screen now allows for the removal of bookmarks.
}


override suspend fun setBookmarkStatus(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Mandvii #yellow violation of patterns: all our datasources follow patterns: there is a store function, not setStatus. Deletion is handled using a StatusEnum. There is a datasource per entity type.

The current main branch OpdsDataSource is not a relevant pattern to follow here: it was read-only, related only to a URL not a user. An update will be merged from my active branch that makes it writable (in preparation to handle playlists): where it follows the same patterns as far as possible.

Please revert all changes to opdsdatasource (and child classes). Also a bookmark is specific to a given user, whilst your bookmark has no reference to user. This needs to be part of the SchoolDataSource.

val personUid = accountManager.activeAccount?.userGuid ?: return@launch

schoolDataSource.bookmarkDataSource
.listAsFlow(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@mikedawson
Instead of using a PagingSource, I’m currently using a Flow of a list because each bookmark has associated app details. With PagingSource, I’m finding it difficult to associate each bookmark with its corresponding app data.

Any suggestions

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mikedawson Instead of using a PagingSource, I’m currently using a Flow of a list because each bookmark has associated app details. With PagingSource, I’m finding it difficult to associate each bookmark with its corresponding app data.

Any suggestions

I think in this case we can go with using a flow because the number of bookmarks for one specific user is very unlikely to exceed the low hundreds. That's not an excessive amount of work for the database/http calls. It's not like a list with 10,000 entries in it.

poojaustad and others added 18 commits March 23, 2026 16:01
*   **002_browse_lessons_test.yaml**: Updated the "Bookmark" tap index from 1 to 0 and commented out several assertion and undo steps.
…functionality for bookmark removal

*   Modify `App.kt` to handle `SnackbarResult` and trigger the snackbar's `onAction` callback.
*   Update `BookmarkListViewModel` to use `SnackBarDispatcher` and provide an "Undo" action when removing a bookmark.
*   Add `remove_bookmark` and `undo` string resources.
@Mandvii Mandvii marked this pull request as ready for review March 24, 2026 09:15
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.

Bookmarks

3 participants