From b021194096bc687b513d2a26b85d455138c4c569 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Fri, 3 Jan 2025 17:11:40 +0200 Subject: [PATCH 01/23] Add basic functions and icons for selected Added a pickup of the possibly excisting selected Datetime into the OBDS feed and parser, so it can be used to change what user sees. Added handling for the selectedURI available in AuthenticationDocument. Current returns the loans. Added test button to load selected from settings page. Added the select and unselect functions to the controller. Added an icon that changes depending on if the user has the book selected or not to the catalog views. Needs to be added still: Button for single book view 401 handling Move seeing selected books to a different place Add little toast to tell that a book has been added or deleted Fix some error handling Ref: EKIR-88 --- .../accounts/api/AccountProvider.kt | 2 + .../accounts/api/AccountProviderType.kt | 6 + .../accounts/json/AccountProvidersJSON.kt | 4 + .../ekirjasto/AccountProviderResolution.kt | 1 + .../ekirjasto/EkirjastoAccountFallback.kt | 1 + .../controller/api/BooksControllerType.kt | 22 +++ .../books/controller/BookSelectTask.kt | 144 ++++++++++++++++++ .../books/controller/BookSyncTask.kt | 50 +++++- .../books/controller/BookUnselectTask.kt | 137 +++++++++++++++++ .../simplified/books/controller/Controller.kt | 45 ++++++ .../books/controller/SelectTaskException.kt | 7 + .../nypl/simplified/feeds/api/FeedLoader.kt | 2 +- .../main/MainFragmentListenerDelegate.kt | 19 ++- .../api/AuthenticationDocument.kt | 3 + .../opds/core/OPDSAcquisitionFeedEntry.java | 25 +++ .../OPDSAcquisitionFeedEntryBuilderType.java | 8 + .../core/OPDSAcquisitionFeedEntryParser.java | 1 + .../nypl/simplified/opds/core/OPDSAtom.java | 16 ++ .../simplified/opds/core/OPDSJSONParser.java | 2 + .../ui/accounts/AccountDetailEvent.kt | 5 + .../ekirjasto/EKirjastoAccountFragment.kt | 16 ++ .../src/main/res/layout/account_ekirjasto.xml | 9 ++ .../ui/catalog/CatalogBookDetailViewModel.kt | 11 ++ .../ui/catalog/CatalogFeedViewModel.kt | 14 ++ .../ui/catalog/CatalogPagedViewHolder.kt | 20 +++ .../ui/catalog/CatalogPagedViewListener.kt | 5 + .../drawable/outline_bookmark_border_24.xml | 10 ++ .../drawable/round_add_circle_outline_24.xml | 10 ++ .../src/main/res/layout/book_cell_idle.xml | 16 +- .../ui/settings/SettingsDebugViewModel.kt | 1 + 30 files changed, 606 insertions(+), 6 deletions(-) create mode 100644 simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt create mode 100644 simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt create mode 100644 simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/SelectTaskException.kt create mode 100644 simplified-ui-catalog/src/main/res/drawable/outline_bookmark_border_24.xml create mode 100644 simplified-ui-catalog/src/main/res/drawable/round_add_circle_outline_24.xml diff --git a/simplified-accounts-api/src/main/java/org/nypl/simplified/accounts/api/AccountProvider.kt b/simplified-accounts-api/src/main/java/org/nypl/simplified/accounts/api/AccountProvider.kt index 4d91ecd38..ff19cea5f 100644 --- a/simplified-accounts-api/src/main/java/org/nypl/simplified/accounts/api/AccountProvider.kt +++ b/simplified-accounts-api/src/main/java/org/nypl/simplified/accounts/api/AccountProvider.kt @@ -17,6 +17,7 @@ data class AccountProvider( override val authenticationAlternatives: List, override val supportsReservations: Boolean, override val loansURI: URI?, + override val selectedURI: URI?, override val cardCreatorURI: URI?, override val authenticationDocumentURI: URI?, override val catalogURI: URI, @@ -56,6 +57,7 @@ data class AccountProvider( authenticationDocumentURI = other.authenticationDocumentURI, cardCreatorURI = other.cardCreatorURI, catalogURI = other.catalogURI, + selectedURI = other.selectedURI, description = other.description, displayName = other.displayName, eula = other.eula, diff --git a/simplified-accounts-api/src/main/java/org/nypl/simplified/accounts/api/AccountProviderType.kt b/simplified-accounts-api/src/main/java/org/nypl/simplified/accounts/api/AccountProviderType.kt index c285ad79f..90a3261ff 100644 --- a/simplified-accounts-api/src/main/java/org/nypl/simplified/accounts/api/AccountProviderType.kt +++ b/simplified-accounts-api/src/main/java/org/nypl/simplified/accounts/api/AccountProviderType.kt @@ -83,6 +83,12 @@ interface AccountProviderType : Comparable { val loansURI: URI? + /** + * @return The URI of the user selected feed, if supported + */ + + val selectedURI: URI? + /** * @return The URI to reset the user's password, if any */ diff --git a/simplified-accounts-json/src/main/java/org/nypl/simplified/accounts/json/AccountProvidersJSON.kt b/simplified-accounts-json/src/main/java/org/nypl/simplified/accounts/json/AccountProvidersJSON.kt index 088af4e15..63377048a 100644 --- a/simplified-accounts-json/src/main/java/org/nypl/simplified/accounts/json/AccountProvidersJSON.kt +++ b/simplified-accounts-json/src/main/java/org/nypl/simplified/accounts/json/AccountProvidersJSON.kt @@ -78,6 +78,7 @@ object AccountProvidersJSON { this.putConditionally(node, "eula", provider.eula) this.putConditionally(node, "license", provider.license) this.putConditionally(node, "loansURI", provider.loansURI) + this.putConditionally(node, "selectedURI", provider.selectedURI) this.putConditionally(node, "logo", provider.logo) this.putConditionally(node, "patronSettingsURI", provider.patronSettingsURI) this.putConditionally(node, "privacyPolicy", provider.privacyPolicy) @@ -275,6 +276,8 @@ object AccountProvidersJSON { JSONParserUtilities.getURIOrNull(obj, "logo") val loansURI = JSONParserUtilities.getURIOrNull(obj, "loansURI") + val selectedURI = + JSONParserUtilities.getURIOrNull(obj, "loansURI")//Wrong URI val patronSettingsURI = JSONParserUtilities.getURIOrNull(obj, "patronSettingsURI") val privacyPolicy = @@ -318,6 +321,7 @@ object AccountProvidersJSON { isProduction = isProduction, license = license, loansURI = loansURI, + selectedURI = selectedURI, location = location, logo = logo, mainColor = mainColor, diff --git a/simplified-accounts-source-ekirjasto/src/main/java/org/nypl/simplified/accounts/source/ekirjasto/AccountProviderResolution.kt b/simplified-accounts-source-ekirjasto/src/main/java/org/nypl/simplified/accounts/source/ekirjasto/AccountProviderResolution.kt index 6e8eb06dd..2afd90dbc 100644 --- a/simplified-accounts-source-ekirjasto/src/main/java/org/nypl/simplified/accounts/source/ekirjasto/AccountProviderResolution.kt +++ b/simplified-accounts-source-ekirjasto/src/main/java/org/nypl/simplified/accounts/source/ekirjasto/AccountProviderResolution.kt @@ -125,6 +125,7 @@ class AccountProviderResolution( isProduction = this.description.isProduction, license = authDocument?.licenseURI, loansURI = authDocument?.loansURI, + selectedURI = authDocument?.selectedURI, logo = authDocument?.logoURI ?: this.description.logoURI?.hrefURI, mainColor = authDocument?.mainColor ?: "red", patronSettingsURI = authDocument?.patronSettingsURI, diff --git a/simplified-app-ekirjasto/src/main/java/fi/kansalliskirjasto/ekirjasto/EkirjastoAccountFallback.kt b/simplified-app-ekirjasto/src/main/java/fi/kansalliskirjasto/ekirjasto/EkirjastoAccountFallback.kt index d229ba5f9..aaf17ccbf 100644 --- a/simplified-app-ekirjasto/src/main/java/fi/kansalliskirjasto/ekirjasto/EkirjastoAccountFallback.kt +++ b/simplified-app-ekirjasto/src/main/java/fi/kansalliskirjasto/ekirjasto/EkirjastoAccountFallback.kt @@ -66,6 +66,7 @@ class EkirjastoAccountFallback : AccountProviderFallbackType { isProduction = true, license = null, loansURI = null, + selectedURI = null, logo = null, mainColor = "orange", patronSettingsURI = null, diff --git a/simplified-books-controller-api/src/main/java/org/nypl/simplified/books/controller/api/BooksControllerType.kt b/simplified-books-controller-api/src/main/java/org/nypl/simplified/books/controller/api/BooksControllerType.kt index 65773b8df..04347229e 100644 --- a/simplified-books-controller-api/src/main/java/org/nypl/simplified/books/controller/api/BooksControllerType.kt +++ b/simplified-books-controller-api/src/main/java/org/nypl/simplified/books/controller/api/BooksControllerType.kt @@ -114,4 +114,26 @@ interface BooksControllerType { accountID: AccountID, bookID: BookID ): FluentFuture> + + /** + * Add the chosen book to a list of selected books. + * + * @param accountID The account that selected the book + * @param bookID The ID of the book + */ + fun bookAddToSelected( + accountID: AccountID, + feedEntry: FeedEntry.FeedEntryOPDS + ) : FluentFuture> + + /** + * Remove the chosen book to a list of selected books. + * + * @param accountID The account that removed the book + * @param bookID The ID of the book + */ + fun bookRemoveFromSelected( + accountID: AccountID, + feedEntry: FeedEntry.FeedEntryOPDS + ) : FluentFuture> } diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt new file mode 100644 index 000000000..e58230bf3 --- /dev/null +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt @@ -0,0 +1,144 @@ +package org.nypl.simplified.books.controller + +import one.irradia.mime.api.MIMECompatibility +import org.librarysimplified.http.api.LSHTTPClientType +import org.librarysimplified.http.api.LSHTTPRequestBuilderType +import org.librarysimplified.http.api.LSHTTPResponseStatus +import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP +import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP.addCredentialsToProperties +import org.nypl.simplified.accounts.database.api.AccountType +import org.nypl.simplified.opds.core.OPDSAcquisitionFeedEntry +import org.nypl.simplified.opds.core.getOrNull +import org.nypl.simplified.taskrecorder.api.TaskRecorder +import org.nypl.simplified.taskrecorder.api.TaskRecorderType +import org.nypl.simplified.taskrecorder.api.TaskResult +import org.slf4j.LoggerFactory +import java.net.URI + +class BookSelectTask ( + private val HTTPClient: LSHTTPClientType, + private val account: AccountType, + private val feedEntry: OPDSAcquisitionFeedEntry +) { + + private lateinit var taskRecorder: TaskRecorderType + + private val logger = + LoggerFactory.getLogger(BookSelectTask::class.java) + + fun execute() : TaskResult<*> { + this.taskRecorder = TaskRecorder.create() + taskRecorder.beginNewStep("Creating an OPDS Select...") + + try { + //Using the alternate link as it seems to be the same basis as the ref "self" + //FIXFIX + // See if better solution to get the link + val baseURI = feedEntry.alternate.getOrNull() + if (baseURI == null) { + logger.debug("No alternate link to use as basis for request") + return taskRecorder.finishFailure() + } + + //Form the select URI by adding the desired path + val currentURI = URI.create(baseURI.toString().plus("/select_book")) + + taskRecorder.beginNewStep("Using $currentURI to select a book...") + taskRecorder.addAttribute("Select URI", currentURI.toString()) + + //Use the credentials available on current profile + val credentials = account.loginState.credentials + + //Create the authorization (bearer) based on current credentials + val auth = + AccountAuthenticatedHTTP.createAuthorizationIfPresent(credentials) + + //Use the POST method to ask server to add the book to selected with selected auth + val request = + HTTPClient.newRequest(currentURI!!) + .setMethod( + LSHTTPRequestBuilderType.Method.Post( + ByteArray(0), + MIMECompatibility.applicationOctetStream + ) + ) + .setAuthorization(auth) + .addCredentialsToProperties(credentials) + .build() + + request.execute().use { response -> + when (val status = response.status) { + is LSHTTPResponseStatus.Responded.OK -> { + //If successful, answer with success, handle feed refresh elsewhere + this.handleOKRequest(currentURI, status) + return taskRecorder.finishSuccess("Success") + } + is LSHTTPResponseStatus.Responded.Error -> { + this.handleHTTPError(status) + return taskRecorder.finishFailure() + } + is LSHTTPResponseStatus.Failed -> { + this.handleHTTPFailure(status) + return taskRecorder.finishFailure() + } + } + } + } catch (e: SelectTaskException.SelectAccessTokenExpired) { + //Catch a special error for access token expiration + throw e + } + } + + private fun handleHTTPFailure( + status: LSHTTPResponseStatus.Failed + ) { + taskRecorder.currentStepFailed( + message = status.exception.message ?: "Exception raised during connection attempt.", + errorCode = "SelectingFailed", + exception = status.exception + ) + throw SelectTaskException.SelectTaskFailed() + } + + private fun handleHTTPError( + status: LSHTTPResponseStatus.Responded.Error + ) { + val report = status.properties.problemReport + if (report != null) { + taskRecorder.addAttributes(report.toMap()) + + //FIXFIX + //Report type for already selected book? + if (report.type == "http://librarysimplified.org/terms/problem/book-already-selected") { + taskRecorder.currentStepSucceeded("It turns out we already selected this book.") + return + } + } + + taskRecorder.currentStepFailed( + message = "HTTP request failed: ${status.properties.originalStatus} ${status.properties.message}", + errorCode = "SelectedError", + exception = null + ) + + if (report?.type == "http://librarysimplified.org/terms/problem/select-limit-reached") { + //FIXFIX + //DO we have special error for list being full? set to 50? + throw SelectTaskException.SelectTaskFailed() + } + + //Catch 401 with special handling in controller + if (report?.type == "http://librarysimplified.org/terms/problem/credentials-invalid") { + throw SelectTaskException.SelectAccessTokenExpired() + } + + throw SelectTaskException.SelectTaskFailed() + } + + private fun handleOKRequest( + uri: URI, + status: LSHTTPResponseStatus.Responded.OK + ) { + taskRecorder.currentStepSucceeded("Book selected successfully") + } +} diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt index da10d3d8e..ab917f6fc 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt @@ -100,7 +100,52 @@ class BookSyncTask( .build() val response = request.execute() - return when (val status = response.status) { + when (val status = response.status) { + is LSHTTPResponseStatus.Responded.OK -> { + this.onHTTPOK( + stream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)), + provider = provider, + account = account, + accessToken = status.getAccessToken() + ) + } + is LSHTTPResponseStatus.Responded.Error -> { + val recovered = this.onHTTPError(status, account) + + if (recovered) { + //this.taskRecorder.finishSuccess(Unit) + } else { + val message = String.format("%s: %d: %s", provider.loansURI, status.properties.status, status.properties.message) + val exception = IOException(message) + this.taskRecorder.currentStepFailed( + message = message, + errorCode = "syncFailed", + exception = exception + ) + throw TaskFailedHandled(exception) + } + } + is LSHTTPResponseStatus.Failed -> + throw IOException(status.exception) + } + + //Do the same for selected + //FIXFIX + //Check if this is needed + val selectedURI = provider.selectedURI + if (selectedURI == null) { + this.logger.debug("no selected URI, aborting!") + return this.taskRecorder.finishSuccess(Unit) + } + + val selectedRequest = + this.http.newRequest(selectedURI) + .setAuthorization(AccountAuthenticatedHTTP.createAuthorization(credentials)) + .addCredentialsToProperties(credentials) + .build() + + val selectedResponse = selectedRequest.execute() + when (val status =selectedResponse.status) { is LSHTTPResponseStatus.Responded.OK -> { this.onHTTPOK( stream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)), @@ -116,7 +161,7 @@ class BookSyncTask( if (recovered) { this.taskRecorder.finishSuccess(Unit) } else { - val message = String.format("%s: %d: %s", provider.loansURI, status.properties.status, status.properties.message) + val message = String.format("%s: %d: %s", provider.selectedURI, status.properties.status, status.properties.message) val exception = IOException(message) this.taskRecorder.currentStepFailed( message = message, @@ -129,6 +174,7 @@ class BookSyncTask( is LSHTTPResponseStatus.Failed -> throw IOException(status.exception) } + return this.taskRecorder.finishSuccess(Unit) } private fun fetchPatronUserProfile( diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt new file mode 100644 index 000000000..9543fa091 --- /dev/null +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt @@ -0,0 +1,137 @@ +package org.nypl.simplified.books.controller + +import one.irradia.mime.api.MIMECompatibility +import org.librarysimplified.http.api.LSHTTPClientType +import org.librarysimplified.http.api.LSHTTPRequestBuilderType +import org.librarysimplified.http.api.LSHTTPResponseStatus +import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP +import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP.addCredentialsToProperties +import org.nypl.simplified.accounts.database.api.AccountType +import org.nypl.simplified.opds.core.OPDSAcquisitionFeedEntry +import org.nypl.simplified.opds.core.getOrNull +import org.nypl.simplified.taskrecorder.api.TaskRecorder +import org.nypl.simplified.taskrecorder.api.TaskRecorderType +import org.nypl.simplified.taskrecorder.api.TaskResult +import org.slf4j.LoggerFactory +import java.net.URI + +class BookUnselectTask ( + private val HTTPClient: LSHTTPClientType, + private val account: AccountType, + private val feedEntry: OPDSAcquisitionFeedEntry +) { + + private lateinit var taskRecorder: TaskRecorderType + + private val logger = + LoggerFactory.getLogger(BookUnselectTask::class.java) + + fun execute() : TaskResult<*> { + this.taskRecorder = TaskRecorder.create() + taskRecorder.beginNewStep("Creating an OPDS Unselect...") + + try { + //Using the alternate link as it seems to be the same basis as the ref "self" + //FIXFIX + // See if better solution to get the link + val baseURI = feedEntry.alternate.getOrNull() + if (baseURI == null) { + logger.debug("No link to form") + return taskRecorder.finishFailure() + } + //Form the select URI by adding the desired path + val currentURI = URI.create(baseURI.toString().plus("/unselect_book")) + + taskRecorder.beginNewStep("Using $currentURI to unselect a book...") + taskRecorder.addAttribute("Unselect URI", currentURI.toString()) + + //Use the credentials available on current profile + val credentials = account.loginState.credentials + + //Create the authorization (bearer) based on current credentials + val auth = + AccountAuthenticatedHTTP.createAuthorizationIfPresent(credentials) + + //Use the DELETE method to ask server to remove the book to selected with auth + val request = + HTTPClient.newRequest(currentURI!!) + .setMethod( + LSHTTPRequestBuilderType.Method.Delete( + ByteArray(0), + MIMECompatibility.applicationOctetStream + ) + ) + .setAuthorization(auth) + .addCredentialsToProperties(credentials) + .build() + + request.execute().use { response -> + when (val status = response.status) { + is LSHTTPResponseStatus.Responded.OK -> { + //If successful, answer with success, handle feed refresh elsewhere + this.handleOKRequest(currentURI, status) + return taskRecorder.finishSuccess("Success") + } + is LSHTTPResponseStatus.Responded.Error -> { + this.handleHTTPError(status) + return taskRecorder.finishFailure() + } + is LSHTTPResponseStatus.Failed -> { + this.handleHTTPFailure(status) + return taskRecorder.finishFailure() + } + } + } + } catch (e: SelectTaskException.SelectAccessTokenExpired) { + //Catch a special error for access token expiration + throw e + } + } + + private fun handleHTTPFailure( + status: LSHTTPResponseStatus.Failed + ) { + taskRecorder.currentStepFailed( + message = status.exception.message ?: "Exception raised during connection attempt.", + errorCode = "SelectingFailed", + exception = status.exception + ) + throw SelectTaskException.UnselectTaskFailed() + } + + private fun handleHTTPError( + status: LSHTTPResponseStatus.Responded.Error + ) { + val report = status.properties.problemReport + if (report != null) { + taskRecorder.addAttributes(report.toMap()) + + //FIXFIX + //Report type for already unselected book? + if (report.type == "http://librarysimplified.org/terms/problem/selection-already-removed") { + taskRecorder.currentStepSucceeded("It turns out we already didn't have this book selected.") + return + } + } + + taskRecorder.currentStepFailed( + message = "HTTP request failed: ${status.properties.originalStatus} ${status.properties.message}", + errorCode = "UnselectError", + exception = null + ) + + //Catch 401 with special handling in controller + if (report?.type == "http://librarysimplified.org/terms/problem/credentials-invalid") { + throw SelectTaskException.SelectAccessTokenExpired() + } + + throw SelectTaskException.UnselectTaskFailed() + } + + private fun handleOKRequest( + uri: URI, + status: LSHTTPResponseStatus.Responded.OK + ) { + taskRecorder.currentStepSucceeded("Book selected successfully") + } +} diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt index 5f02f03d4..1a2009b49 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt @@ -731,6 +731,51 @@ class Controller private constructor( ) } + override fun bookAddToSelected( + accountID: AccountID, + feedEntry: FeedEntry.FeedEntryOPDS + + ) : FluentFuture> { + val profile = this.profileCurrent() + val account = profile.account(accountID) + + //FIXFIXFIX + //Add 401 handling + //Add successful reload? + return this.submitTask( + Callable >{ + val bookSelectTask = BookSelectTask( + HTTPClient = this.lsHttp, + account = account, + feedEntry =feedEntry.feedEntry + ) + bookSelectTask.execute() + } + ) + } + + override fun bookRemoveFromSelected( + accountID: AccountID, + feedEntry: FeedEntry.FeedEntryOPDS + ): FluentFuture> { + val profile = this.profileCurrent() + val account = profile.account(accountID) + + //FIXFIXFIX + //Add 401 handling + //Add successful reload? + return this.submitTask( + Callable >{ + val bookUnselectTask = BookUnselectTask( + HTTPClient = this.lsHttp, + account = account, + feedEntry =feedEntry.feedEntry + ) + bookUnselectTask.execute() + } + ) + } + override fun profileAnyIsCurrent(): Boolean = this.profiles.currentProfile().isSome diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/SelectTaskException.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/SelectTaskException.kt new file mode 100644 index 000000000..19c352d9a --- /dev/null +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/SelectTaskException.kt @@ -0,0 +1,7 @@ +package org.nypl.simplified.books.controller + +sealed class SelectTaskException : Exception() { + class SelectTaskFailed : SelectTaskException() + class UnselectTaskFailed : SelectTaskException() + class SelectAccessTokenExpired : SelectTaskException() +} diff --git a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedLoader.kt b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedLoader.kt index 25bc60c20..31c152e71 100644 --- a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedLoader.kt +++ b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedLoader.kt @@ -186,7 +186,7 @@ class FeedLoader private constructor( accountId: AccountID, uri: URI ): FeedLoaderResult { - val streamMaybe = this.contentResolver.openInputStream(uri) + val streamMaybe = this.contentResolver.openInputStream(uri)// return if (streamMaybe != null) { streamMaybe.use { stream -> FeedLoaderSuccess( diff --git a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt index e893dd80f..6a3a3ef35 100644 --- a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt +++ b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt @@ -1,7 +1,9 @@ package org.librarysimplified.main +import android.net.Uri import androidx.activity.addCallback import androidx.appcompat.app.AppCompatActivity +import androidx.core.net.toUri import androidx.fragment.app.Fragment import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleObserver @@ -14,6 +16,7 @@ import org.librarysimplified.ui.catalog.CatalogBookDetailFragmentParameters import org.librarysimplified.ui.catalog.CatalogFeedArguments import org.librarysimplified.ui.catalog.CatalogFeedEvent import org.librarysimplified.ui.catalog.CatalogFeedFragment +import org.librarysimplified.ui.catalog.CatalogFeedOwnership import org.librarysimplified.ui.catalog.saml20.CatalogSAML20Event import org.librarysimplified.ui.login.LoginMainFragment import org.librarysimplified.ui.navigation.tabs.TabbedNavigator @@ -60,6 +63,7 @@ import org.nypl.simplified.ui.settings.SettingsMainEvent import org.nypl.simplified.viewer.api.Viewers import org.nypl.simplified.viewer.spi.ViewerPreferences import org.slf4j.LoggerFactory +import java.net.URI import java.net.URL internal class MainFragmentListenerDelegate( @@ -377,7 +381,6 @@ internal class MainFragmentListenerDelegate( this.navigator.popBackStack() MainFragmentState.EmptyState } - else -> { state } @@ -427,6 +430,20 @@ internal class MainFragmentListenerDelegate( this.openDependentPage() state } + + //FIXFIX + is AccountDetailEvent.OpenSelectedFragment -> { + val accountID = profilesController.profileCurrent().mostRecentAccount().id + val selectedURI = profilesController.profileCurrent().mostRecentAccount().provider.selectedURI!! + val arguments = CatalogFeedArguments.CatalogFeedArgumentsRemote( + title = "Selected", + ownership = CatalogFeedOwnership.OwnedByAccount(accountID), + feedURI = selectedURI, + isSearchResults = false + ) + this.openFeed(arguments) + state + } } } diff --git a/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt b/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt index 5df792f8d..638d4d388 100644 --- a/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt +++ b/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt @@ -72,6 +72,9 @@ data class AuthenticationDocument( val loansURI: URI? = this.links.find { link -> link.relation == "http://opds-spec.org/shelf" }?.hrefURI + val selectedURI: URI? = //FIXFIX change to correct + this.links.find { link -> link.relation == "http://opds-spec.org/shelf" }?.hrefURI + val cardCreatorURI: URI? = this.links.find { link -> link.relation == "register" }?.hrefURI diff --git a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntry.java b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntry.java index 2a54e12b9..145d33656 100644 --- a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntry.java +++ b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntry.java @@ -37,6 +37,7 @@ public final class OPDSAcquisitionFeedEntry implements Serializable { private final OptionType issues; private final OptionType related; private final OptionType published; + private final OptionType selected; private final OptionType publisher; private final OptionType language; private final String distribution; @@ -71,6 +72,7 @@ private OPDSAcquisitionFeedEntry( final String in_summary, final List in_narrators, final OptionType in_published, + final OptionType in_selected, final OptionType in_publisher, final String in_distribution, final List in_categories, @@ -95,6 +97,7 @@ private OPDSAcquisitionFeedEntry( this.title = NullCheck.notNull(in_title); this.thumbnail = NullCheck.notNull(in_thumbnail); this.timeTrackingUri = NullCheck.notNull(in_time_tracking_uri); + this.selected = NullCheck.notNull(in_selected); this.updated = NullCheck.notNull(in_updated); this.summary = NullCheck.notNull(in_summary); this.narrators = NullCheck.notNull(in_narrators); @@ -175,6 +178,7 @@ public static OPDSAcquisitionFeedEntryBuilderType newBuilderFrom( b.setAlternateOption(e.getAlternate()); b.setAnalyticsOption(e.getAnalytics()); b.setLicensorOption(e.getLicensor()); + b.setSelectedOption(e.getSelected()); { final String summary = e.getSummary(); @@ -225,6 +229,7 @@ public boolean equals( && this.published.equals(other.published) && this.publisher.equals(other.publisher) && this.licensor.equals(other.licensor) + && this.selected.equals(other.selected) && this.distribution.equals(other.distribution); } @@ -414,6 +419,13 @@ public OptionType getLicensor() { return this.licensor; } + /** + * @return Selected + */ + public OptionType getSelected() { + return this.selected; + } + /** * @return The title */ @@ -480,6 +492,7 @@ public int hashCode() { result = (prime * result) + this.published.hashCode(); result = (prime * result) + this.publisher.hashCode(); result = (prime * result) + this.distribution.hashCode(); + result = (prime * result) + this.selected.hashCode(); result = (prime * result) + this.licensor.hashCode(); return result; } @@ -531,6 +544,8 @@ public String toString() { b.append(this.updated); b.append(", licensor="); b.append(this.licensor); + b.append(", selected="); + b.append(this.selected); b.append("]"); return NullCheck.notNull(b.toString()); } @@ -564,6 +579,7 @@ private static final class Builder implements OPDSAcquisitionFeedEntryBuilderTyp private OptionType thumbnail; private OptionType timeTrackingUri; private OptionType licensor; + private OptionType selected; private OptionType duration; private Builder( @@ -578,6 +594,7 @@ private Builder( this.title = NullCheck.notNull(in_title); this.updated = NullCheck.notNull(in_updated); this.availability = NullCheck.notNull(in_availability); + this.selected = Option.none(); this.summary = ""; this.narrators = new ArrayList(); this.copyright = Option.none(); @@ -659,6 +676,7 @@ public OPDSAcquisitionFeedEntry build() { this.summary, this.narrators, this.published, + this.selected, this.publisher, this.distribution, this.categories, @@ -810,6 +828,13 @@ public OPDSAcquisitionFeedEntryBuilderType setLicensorOption( return this; } + @Override + public OPDSAcquisitionFeedEntryBuilderType setSelectedOption( + final OptionType sel) { + this.selected = NullCheck.notNull(sel); + return this; + } + @Override public OPDSAcquisitionFeedEntryBuilderType setDurationOption(OptionType duration) { this.duration = NullCheck.notNull(duration); diff --git a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntryBuilderType.java b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntryBuilderType.java index 46d0a6b62..cf51afa9c 100644 --- a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntryBuilderType.java +++ b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntryBuilderType.java @@ -106,6 +106,14 @@ OPDSAcquisitionFeedEntryBuilderType addPreviewAcquisition( OPDSAcquisitionFeedEntryBuilderType setAnnotationsOption( OptionType uri); + /** + * Set selected + * + * @param selected Selected DateTime or not + */ + OPDSAcquisitionFeedEntryBuilderType setSelectedOption( + OptionType selected); + /** * @param uri The alternate URI */ diff --git a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntryParser.java b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntryParser.java index a486c13fe..7f97002fc 100644 --- a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntryParser.java +++ b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntryParser.java @@ -249,6 +249,7 @@ private OPDSAcquisitionFeedEntry parseAcquisitionEntry( entry_builder.setPublisherOption(findPublisher(element)); entry_builder.setDistribution(findDistribution(element)); entry_builder.setPublishedOption(OPDSAtom.findPublished(element)); + entry_builder.setSelectedOption(OPDSAtom.findSelected(element)); entry_builder.setSummaryOption( OPDSXML.getFirstChildElementTextWithNameOptional(element, ATOM_URI, "summary")); entry_builder.setLanguageOption(findLanguage(element)); diff --git a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAtom.java b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAtom.java index 64c85d676..a3b0fba9c 100644 --- a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAtom.java +++ b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAtom.java @@ -60,4 +60,20 @@ static DateTime findUpdated( OPDSXML.getFirstChildElementTextWithName(e, OPDSFeedConstants.ATOM_URI, "updated"); return OPDSDateParsers.dateTimeParser().parseDateTime(e_updated_raw); } + + static OptionType findSelected( + final Element e) + throws DOMException, ParseException + { + final OptionType e_opt = + OPDSXML.getFirstChildElementWithNameOptional( + e, OPDSFeedConstants.ATOM_URI, "selected"); + + return e_opt.mapPartial( + (PartialFunctionType) er -> { + final String text = er.getTextContent(); + final String trimmed = text.trim(); + return OPDSDateParsers.dateTimeParser().parseDateTime(trimmed); + }); + } } diff --git a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSJSONParser.java b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSJSONParser.java index b18287a02..a0bef8d75 100644 --- a/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSJSONParser.java +++ b/simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSJSONParser.java @@ -513,6 +513,8 @@ public Unit call( JSONParserUtilities.getString(s, "distribution")); fb.setSummaryOption( JSONParserUtilities.getStringOptional(s, "summary")); + fb.setSelectedOption( + JSONParserUtilities.getTimestampOptional(s, "selected")); return fb.build(); } catch (final JSONParseException e) { throw new OPDSParseException(e); diff --git a/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/AccountDetailEvent.kt b/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/AccountDetailEvent.kt index af564c021..2071c5616 100644 --- a/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/AccountDetailEvent.kt +++ b/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/AccountDetailEvent.kt @@ -75,4 +75,9 @@ sealed class AccountDetailEvent { * Open the dependents view. */ object OpenDependentInvite : AccountDetailEvent() + + /** + * FIXFIX Open selectedfragment + */ + object OpenSelectedFragment : AccountDetailEvent() } diff --git a/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/ekirjasto/EKirjastoAccountFragment.kt b/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/ekirjasto/EKirjastoAccountFragment.kt index 6b12427e7..09bc93220 100644 --- a/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/ekirjasto/EKirjastoAccountFragment.kt +++ b/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/ekirjasto/EKirjastoAccountFragment.kt @@ -80,6 +80,9 @@ class EKirjastoAccountFragment : Fragment(R.layout.account_ekirjasto){ private lateinit var bookmarkStatement: TextView private lateinit var buttonPreferences: Button + //FIXFIX + private lateinit var buttonSelected: Button + //inherited elements private lateinit var toolbar: PalaceToolbar @@ -119,6 +122,7 @@ class EKirjastoAccountFragment : Fragment(R.layout.account_ekirjasto){ this.versionText = view.findViewById(R.id.appVersion) this.bookmarkSyncCheck = view.findViewById(R.id.accountSyncBookmarksCheck) this.buttonPreferences = view.findViewById(R.id.buttonPreferences) + this.buttonSelected = view.findViewById(R.id.buttonSelected) this.toolbar = @@ -200,6 +204,16 @@ class EKirjastoAccountFragment : Fragment(R.layout.account_ekirjasto){ } } + private fun configureSelectedButton(button: Button) { + //Clicking the button opens the dependents fragment + button.setOnClickListener { + logger.debug("Selected clicked") + this.listener.post( + AccountDetailEvent.OpenSelectedFragment + ) + } + } + private fun configureDocViewButton(button: Button, document: DocumentType?){ button.isEnabled = document != null if (document != null) { @@ -267,6 +281,8 @@ class EKirjastoAccountFragment : Fragment(R.layout.account_ekirjasto){ */ configureDependentsButton(buttonDependents) + configureSelectedButton(buttonSelected) + //Set version text that's shown on the bottom of the page val versionString = this.requireContext().getString(R.string.app_version_string, this.viewModel.appVersion) this.versionText.text = versionString diff --git a/simplified-ui-accounts/src/main/res/layout/account_ekirjasto.xml b/simplified-ui-accounts/src/main/res/layout/account_ekirjasto.xml index d407e5211..be4c601c3 100644 --- a/simplified-ui-accounts/src/main/res/layout/account_ekirjasto.xml +++ b/simplified-ui-accounts/src/main/res/layout/account_ekirjasto.xml @@ -70,6 +70,15 @@ android:layout_height="wrap_content" android:text="@string/invite_a_dependent" /> + + (R.id.bookCellIdleAuthor)!! private val idleButtons = this.idle.findViewById(R.id.bookCellIdleButtons)!! + private val idleSelectedButton = + this.idle.findViewById(R.id.bookCellIdleSelect)!! private val progressProgress = this.parent.findViewById(R.id.bookCellInProgressBar)!! @@ -151,6 +155,22 @@ class CatalogPagedViewHolder( context.getString(R.string.catalogBookFormatPDF) null -> "" } + //If there is a selected date, the book is selected + if (item.feedEntry.selected is Some) { + //Set the drawable as the "checked" version + this.idleSelectedButton.setImageDrawable( + ContextCompat.getDrawable(context,R.drawable.outline_bookmark_border_24) + ) + this.idleSelectedButton.setOnClickListener{ + //Remove book from selected + this.listener.unselectBook(item) + } + } else { + this.idleSelectedButton.setOnClickListener { + //Add book to selected + this.listener.selectBook(item) + } + } val targetHeight = this.parent.resources.getDimensionPixelSize( diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewListener.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewListener.kt index 665f2a598..85330fc99 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewListener.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewListener.kt @@ -27,6 +27,11 @@ interface CatalogPagedViewListener { fun delete(feedEntry: FeedEntry.FeedEntryOPDS) fun cancelDownload(feedEntry: FeedEntry.FeedEntryOPDS) + + fun selectBook(feedEntry: FeedEntry.FeedEntryOPDS) + + fun unselectBook(feedEntry: FeedEntry.FeedEntryOPDS) + fun borrowMaybeAuthenticated(book: Book) fun resetInitialBookStatus(feedEntry: FeedEntry.FeedEntryOPDS) diff --git a/simplified-ui-catalog/src/main/res/drawable/outline_bookmark_border_24.xml b/simplified-ui-catalog/src/main/res/drawable/outline_bookmark_border_24.xml new file mode 100644 index 000000000..e6ec374b0 --- /dev/null +++ b/simplified-ui-catalog/src/main/res/drawable/outline_bookmark_border_24.xml @@ -0,0 +1,10 @@ + + + + + diff --git a/simplified-ui-catalog/src/main/res/drawable/round_add_circle_outline_24.xml b/simplified-ui-catalog/src/main/res/drawable/round_add_circle_outline_24.xml new file mode 100644 index 000000000..0d9769fb3 --- /dev/null +++ b/simplified-ui-catalog/src/main/res/drawable/round_add_circle_outline_24.xml @@ -0,0 +1,10 @@ + + + + + diff --git a/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml b/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml index 605841095..7aefffca5 100644 --- a/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml +++ b/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml @@ -43,18 +43,30 @@ android:layout_height="wrap_content" android:layout_marginStart="8dp" android:layout_marginTop="8dp" - android:layout_marginEnd="16dp" + android:layout_marginEnd="8dp" android:ellipsize="end" android:includeFontPadding="false" android:maxLines="2" android:textSize="16sp" android:textStyle="bold" - app:layout_constraintEnd_toEndOf="parent" + app:layout_constraintEnd_toStartOf="@id/bookCellIdleSelect" app:layout_constraintHorizontal_bias="0.0" app:layout_constraintStart_toEndOf="@id/bookCellIdleCover" app:layout_constraintTop_toTopOf="parent" tools:text="The Modern Prometheus" /> + Date: Tue, 7 Jan 2025 14:29:10 +0200 Subject: [PATCH 02/23] Add selection button to single book view Added the same button into the single page view and added the functionality. --- .../api/AuthenticationDocument.kt | 2 +- .../ui/catalog/CatalogBookDetailFragment.kt | 29 +++++++++++++++++++ .../ui/catalog/CatalogBookDetailViewModel.kt | 14 ++++++--- .../CatalogBookDetailViewModelFactory.kt | 4 +++ .../ui/catalog/CatalogPagedViewHolder.kt | 8 +++++ .../src/main/res/layout/book_cell_idle.xml | 1 + .../src/main/res/layout/book_detail.xml | 16 +++++++++- .../src/main/res/values/stringsCatalog.xml | 2 ++ 8 files changed, 70 insertions(+), 6 deletions(-) diff --git a/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt b/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt index 638d4d388..5e356c9a1 100644 --- a/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt +++ b/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt @@ -72,7 +72,7 @@ data class AuthenticationDocument( val loansURI: URI? = this.links.find { link -> link.relation == "http://opds-spec.org/shelf" }?.hrefURI - val selectedURI: URI? = //FIXFIX change to correct + val selectedURI: URI? = //FIXFIX change to correct : http://opds-spec.org/shelf/selected_books this.links.find { link -> link.relation == "http://opds-spec.org/shelf" }?.hrefURI val cardCreatorURI: URI? = diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt index 65a9727f4..7fde8323e 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt @@ -12,6 +12,7 @@ import android.widget.LinearLayout import android.widget.ProgressBar import android.widget.TextView import androidx.appcompat.app.AlertDialog +import androidx.core.content.ContextCompat import com.google.android.material.dialog.MaterialAlertDialogBuilder import androidx.core.os.bundleOf import androidx.fragment.app.Fragment @@ -135,6 +136,7 @@ class CatalogBookDetailFragment : Fragment(R.layout.book_detail) { private lateinit var summary: TextView private lateinit var title: TextView private lateinit var type: TextView + private lateinit var selected: ImageView private lateinit var toolbar: PalaceToolbar private val dateYearFormatter = @@ -217,6 +219,8 @@ class CatalogBookDetailFragment : Fragment(R.layout.book_detail) { view.findViewById(R.id.feedLoading) this.report = view.findViewById(R.id.bookDetailReport) + this.selected = + view.findViewById(R.id.bookDetailSelect) this.debugStatus = view.findViewById(R.id.bookDetailDebugStatus) @@ -537,6 +541,31 @@ class CatalogBookDetailFragment : Fragment(R.layout.book_detail) { private fun reconfigureUI(book: BookWithStatus, bookPreviewStatus: BookPreviewStatus) { this.debugStatus.text = book.javaClass.simpleName + //If there is a selected date, the book is selected + if (book.book.entry.selected is Some) { + //Set the drawable as the "checked" version + this.selected.setImageDrawable( + ContextCompat.getDrawable(this.requireContext(), R.drawable.outline_bookmark_border_24) + ) + //Add the audio description + this.selected.contentDescription = getString(R.string.catalogAccessibilityBookUnselect) + + this.selected.setOnClickListener { + //Set the button click to unselect the book + this.viewModel.unselectBook(this.parameters.feedEntry) + } + }else { + //Set the "unchecked" icon version + this.selected.setImageDrawable( + ContextCompat.getDrawable(this.requireContext(),R.drawable.round_add_circle_outline_24) + ) + //Add audio description + this.selected.contentDescription = getString(R.string.catalogAccessibilityBookSelect) + this.selected.setOnClickListener { + //Add book to selected + this.viewModel.selectBook(this.parameters.feedEntry) + } + } when (val status = book.status) { is BookStatus.Held -> { this.onBookStatusHeld(status, bookPreviewStatus) diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt index 760e70be4..4b5f78f87 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt @@ -21,6 +21,7 @@ import org.nypl.simplified.books.book_registry.BookRegistryType import org.nypl.simplified.books.book_registry.BookStatus import org.nypl.simplified.books.book_registry.BookStatusEvent import org.nypl.simplified.books.book_registry.BookWithStatus +import org.nypl.simplified.books.controller.api.BooksControllerType import org.nypl.simplified.buildconfig.api.BuildConfigurationServiceType import org.nypl.simplified.feeds.api.Feed import org.nypl.simplified.feeds.api.FeedEntry @@ -50,6 +51,7 @@ class CatalogBookDetailViewModel( private val bookRegistry: BookRegistryType, private val buildConfiguration: BuildConfigurationServiceType, private val borrowViewModel: CatalogBorrowViewModel, + private val booksController: BooksControllerType, private val parameters: CatalogBookDetailFragmentParameters, private val listener: FragmentListenerType ) : ViewModel(), CatalogPagedViewListener { @@ -297,13 +299,17 @@ class CatalogBookDetailViewModel( } override fun selectBook(feedEntry: FeedEntry.FeedEntryOPDS) { - //NOT MADE BUTTON HERE YET - TODO("Not yet implemented") + booksController.bookAddToSelected( + accountID = profilesController.profileCurrent().mostRecentAccount().id, + feedEntry = feedEntry + ) } override fun unselectBook(feedEntry: FeedEntry.FeedEntryOPDS) { - //NOT MADE BUTTON HERE YET - TODO("Not yet implemented") + booksController.bookAddToSelected( + accountID = profilesController.profileCurrent().mostRecentAccount().id, + feedEntry = feedEntry + ) } override fun resetInitialBookStatus(feedEntry: FeedEntry.FeedEntryOPDS) { diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModelFactory.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModelFactory.kt index c80ed3bc3..5d596cda9 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModelFactory.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModelFactory.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import org.librarysimplified.services.api.ServiceDirectoryType import org.nypl.simplified.books.book_registry.BookRegistryType +import org.nypl.simplified.books.controller.api.BooksControllerType import org.nypl.simplified.buildconfig.api.BuildConfigurationServiceType import org.nypl.simplified.feeds.api.FeedLoaderType import org.nypl.simplified.listeners.api.FragmentListenerType @@ -31,6 +32,8 @@ class CatalogBookDetailViewModelFactory( services.requireService(ProfilesControllerType::class.java) val bookRegistry = services.requireService(BookRegistryType::class.java) + val booksController = + this.services.requireService(BooksControllerType::class.java) val configurationService = services.requireService(BuildConfigurationServiceType::class.java) @@ -40,6 +43,7 @@ class CatalogBookDetailViewModelFactory( bookRegistry, configurationService, this.borrowViewModel, + booksController, parameters, listener ) as T diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt index c50f60c76..aec5998d6 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt @@ -161,11 +161,19 @@ class CatalogPagedViewHolder( this.idleSelectedButton.setImageDrawable( ContextCompat.getDrawable(context,R.drawable.outline_bookmark_border_24) ) + //Set audio description to the button + this.idleSelectedButton.contentDescription = context.getString(R.string.catalogAccessibilityBookSelect) this.idleSelectedButton.setOnClickListener{ //Remove book from selected this.listener.unselectBook(item) } } else { + //Set the "unchecked" icon version + this.idleSelectedButton.setImageDrawable( + ContextCompat.getDrawable(context,R.drawable.round_add_circle_outline_24) + ) + //Add audio description + this.idleSelectedButton.contentDescription = context.getString(R.string.catalogAccessibilityBookSelect) this.idleSelectedButton.setOnClickListener { //Add book to selected this.listener.selectBook(item) diff --git a/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml b/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml index 7aefffca5..248914c08 100644 --- a/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml +++ b/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml @@ -61,6 +61,7 @@ android:layout_height="wrap_content" android:layout_marginTop="8dp" android:layout_marginEnd="16dp" + android:layout_marginStart="16dp" android:gravity="right" android:src="@drawable/round_add_circle_outline_24" app:layout_constraintStart_toEndOf="@id/bookCellIdleTitle" diff --git a/simplified-ui-catalog/src/main/res/layout/book_detail.xml b/simplified-ui-catalog/src/main/res/layout/book_detail.xml index d435c9ca7..2edfaf28b 100644 --- a/simplified-ui-catalog/src/main/res/layout/book_detail.xml +++ b/simplified-ui-catalog/src/main/res/layout/book_detail.xml @@ -51,11 +51,25 @@ android:layout_marginEnd="16dp" android:textSize="18sp" android:textStyle="bold" - app:layout_constraintEnd_toEndOf="parent" + app:layout_constraintEnd_toStartOf="@id/bookDetailSelect" app:layout_constraintStart_toEndOf="@id/bookDetailCover" app:layout_constraintTop_toTopOf="parent" tools:text="The Modern Prometheus" /> + + Get Delete Download + Add book to your selected books + Remove book from your selected books Cancel download Details Dismiss From 5820da8227ff2a1817187f50e5c3bdcc1cbd82e1 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Tue, 7 Jan 2025 15:10:27 +0200 Subject: [PATCH 03/23] Change the selected icon --- .../ui/catalog/CatalogBookDetailFragment.kt | 2 +- .../ui/catalog/CatalogPagedViewHolder.kt | 2 +- .../src/main/res/drawable/baseline_check_circle_24.xml | 5 +++++ .../main/res/drawable/outline_bookmark_border_24.xml | 10 ---------- 4 files changed, 7 insertions(+), 12 deletions(-) create mode 100644 simplified-ui-catalog/src/main/res/drawable/baseline_check_circle_24.xml delete mode 100644 simplified-ui-catalog/src/main/res/drawable/outline_bookmark_border_24.xml diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt index 7fde8323e..f6b21aff5 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt @@ -545,7 +545,7 @@ class CatalogBookDetailFragment : Fragment(R.layout.book_detail) { if (book.book.entry.selected is Some) { //Set the drawable as the "checked" version this.selected.setImageDrawable( - ContextCompat.getDrawable(this.requireContext(), R.drawable.outline_bookmark_border_24) + ContextCompat.getDrawable(this.requireContext(), R.drawable.baseline_check_circle_24) ) //Add the audio description this.selected.contentDescription = getString(R.string.catalogAccessibilityBookUnselect) diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt index aec5998d6..e2629c5b4 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt @@ -159,7 +159,7 @@ class CatalogPagedViewHolder( if (item.feedEntry.selected is Some) { //Set the drawable as the "checked" version this.idleSelectedButton.setImageDrawable( - ContextCompat.getDrawable(context,R.drawable.outline_bookmark_border_24) + ContextCompat.getDrawable(context,R.drawable.baseline_check_circle_24) ) //Set audio description to the button this.idleSelectedButton.contentDescription = context.getString(R.string.catalogAccessibilityBookSelect) diff --git a/simplified-ui-catalog/src/main/res/drawable/baseline_check_circle_24.xml b/simplified-ui-catalog/src/main/res/drawable/baseline_check_circle_24.xml new file mode 100644 index 000000000..9a125bcdb --- /dev/null +++ b/simplified-ui-catalog/src/main/res/drawable/baseline_check_circle_24.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/simplified-ui-catalog/src/main/res/drawable/outline_bookmark_border_24.xml b/simplified-ui-catalog/src/main/res/drawable/outline_bookmark_border_24.xml deleted file mode 100644 index e6ec374b0..000000000 --- a/simplified-ui-catalog/src/main/res/drawable/outline_bookmark_border_24.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - - From 54b777ecd549a66c5fd4e5536b1df8d306541830 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Wed, 22 Jan 2025 13:55:55 +0200 Subject: [PATCH 04/23] Ensure correct functioning of tasks and visuals Fixed both the select and unselect tasks to handle the database and book register correctly, as well as added comments to the code. Added a toast that informs user about selecting and unselecting a book being successful. Added functions that handle the toasts based on book status. Added functions to reset a book status after the toast has been showed. Added accessibility strings and descriptions, though unsure if they are used. Also added conditions to when a book is shown as revokes in the accessibility section. TODO: - Change the location of select button in app + rename - Reload selected on remove from the list - Making into local feed - Handling of 401 - Show selected only in selected and book info page --- .../accessibility/AccessibilityService.kt | 18 +- .../accessibility/AccessibilityStrings.kt | 6 + .../accessibility/AccessibilityStringsType.kt | 2 + .../src/main/res/values/strings.xml | 2 + .../simplified/books/borrowing/BorrowTask.kt | 19 +- .../controller/api/BooksControllerType.kt | 2 +- simplified-books-controller/build.gradle.kts | 1 + .../books/controller/BookSelectTask.kt | 150 +++++++++++-- .../books/controller/BookSyncTask.kt | 201 ++++++++++++++++-- .../books/controller/BookUnselectTask.kt | 102 +++++++-- .../simplified/books/controller/Controller.kt | 15 +- .../books/controller/ProfileFeedTask.kt | 4 + .../books/book_registry/BookStatus.kt | 26 +++ .../BookStatusPriorityOrdering.java | 10 + .../librarysimplified/main/MainFragment.kt | 2 + .../api/AuthenticationDocument.kt | 4 +- .../catalog/CatalogBookAvailabilityStrings.kt | 4 + .../ui/catalog/CatalogBookDetailFragment.kt | 21 ++ .../ui/catalog/CatalogBookDetailViewModel.kt | 28 ++- .../ui/catalog/CatalogFeedViewModel.kt | 29 +++ .../ui/catalog/CatalogPagedViewHolder.kt | 29 ++- .../ui/catalog/CatalogPagedViewListener.kt | 4 + .../src/main/res/values/stringsCatalog.xml | 7 +- 23 files changed, 610 insertions(+), 76 deletions(-) diff --git a/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityService.kt b/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityService.kt index 6d403a215..df6a3e8dc 100644 --- a/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityService.kt +++ b/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityService.kt @@ -84,7 +84,9 @@ class AccessibilityService private constructor( is BookStatus.Loanable -> { val notHoldable = this.previousStatusIsNot(event, BookStatus.Holdable::class.java) val notLoanable = this.previousStatusIsNot(event, BookStatus.Loanable::class.java) - if (notHoldable && notLoanable) { + val notUnselected = this.previousStatusIsNot(event, BookStatus.Unselected::class.java) + val notSelected = this.previousStatusIsNot(event, BookStatus.Selected::class.java) + if (notHoldable && notLoanable && notUnselected && notSelected) { this.speak(this.strings.bookReturned(book.entry.title)) } else { // Nothing to do @@ -126,6 +128,20 @@ class AccessibilityService private constructor( is BookStatus.RequestingDownload, is BookStatus.DownloadWaitingForExternalAuthentication, is BookStatus.DownloadExternalAuthenticationInProgress, + is BookStatus.Selected -> { + if (this.previousStatusIsNot(event, BookStatus.Selected::class.java)) { + this.speak(this.strings.bookSelected(book.entry.title)) + } else { + // Nothing to do + } + } + is BookStatus.Unselected -> { + if (this.previousStatusIsNot(event, BookStatus.Unselected::class.java)) { + this.speak(this.strings.bookUnselected(book.entry.title)) + } else { + // Nothing to do + } + } is BookStatus.Revoked -> { } null -> { diff --git a/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityStrings.kt b/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityStrings.kt index b49568347..c79593c78 100644 --- a/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityStrings.kt +++ b/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityStrings.kt @@ -33,4 +33,10 @@ class AccessibilityStrings( override fun bookLoanLimitReached(): String = this.resources.getString(R.string.reachedLoanLimit) + + override fun bookSelected(title: String): String = + this.resources.getString(R.string.bookSelected, title) + + override fun bookUnselected(title: String): String = + this.resources.getString(R.string.bookUnselected, title) } diff --git a/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityStringsType.kt b/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityStringsType.kt index f7194ecf3..6ab17cbab 100644 --- a/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityStringsType.kt +++ b/simplified-accessibility/src/main/java/org/nypl/simplified/accessibility/AccessibilityStringsType.kt @@ -13,4 +13,6 @@ interface AccessibilityStringsType { fun bookFailedLoan(title: String): String fun bookFailedDownload(title: String): String fun bookLoanLimitReached(): String + fun bookSelected(title: String): String + fun bookUnselected(title: String): String } diff --git a/simplified-accessibility/src/main/res/values/strings.xml b/simplified-accessibility/src/main/res/values/strings.xml index 6cbc1f1b2..bbc59e8b4 100644 --- a/simplified-accessibility/src/main/res/values/strings.xml +++ b/simplified-accessibility/src/main/res/values/strings.xml @@ -9,4 +9,6 @@ A reservation has been placed for the book \'%1$s\' The book \'%1$s\' has been successfully returned You have reached your loan limit + The book \'%1$s\' has been added to favorites + The book \'%1$s\' has been removed from favorites diff --git a/simplified-books-borrowing/src/main/java/org/nypl/simplified/books/borrowing/BorrowTask.kt b/simplified-books-borrowing/src/main/java/org/nypl/simplified/books/borrowing/BorrowTask.kt index ac0b1e4fe..cf4ce2d4b 100644 --- a/simplified-books-borrowing/src/main/java/org/nypl/simplified/books/borrowing/BorrowTask.kt +++ b/simplified-books-borrowing/src/main/java/org/nypl/simplified/books/borrowing/BorrowTask.kt @@ -300,10 +300,23 @@ class BorrowTask private constructor( try { val database = this.account.bookDatabase - val dbEntry = database.createOrUpdate(book.id, entry) - this.databaseEntry = dbEntry + + //If the book is already in the database, it might be selected + //And just overwriting the existing entry with the new one would not carry that information over + if (database.books().contains(book.id)) { + //If book already in db, get the entry and insert the selected information already available + val dbEntry = database.entry(book.id) + //Build a new feed entry + val adjustedEntry = OPDSAcquisitionFeedEntry.newBuilderFrom(entry) + .setSelectedOption(dbEntry.book.entry.selected) + .build() + //Create the database entry + this.databaseEntry = database.createOrUpdate(book.id, adjustedEntry) + } else { + this.databaseEntry = database.createOrUpdate(book.id, entry) + } this.taskRecorder.currentStepSucceeded("Book database updated.") - return dbEntry.book + return this.databaseEntry!!.book } catch (e: Exception) { this.error("[{}]: failed to set up book database: ", book.id.brief(), e) this.taskRecorder.currentStepFailed( diff --git a/simplified-books-controller-api/src/main/java/org/nypl/simplified/books/controller/api/BooksControllerType.kt b/simplified-books-controller-api/src/main/java/org/nypl/simplified/books/controller/api/BooksControllerType.kt index 04347229e..980127598 100644 --- a/simplified-books-controller-api/src/main/java/org/nypl/simplified/books/controller/api/BooksControllerType.kt +++ b/simplified-books-controller-api/src/main/java/org/nypl/simplified/books/controller/api/BooksControllerType.kt @@ -119,7 +119,7 @@ interface BooksControllerType { * Add the chosen book to a list of selected books. * * @param accountID The account that selected the book - * @param bookID The ID of the book + * @param feedEntry The FeedEntry for the book */ fun bookAddToSelected( accountID: AccountID, diff --git a/simplified-books-controller/build.gradle.kts b/simplified-books-controller/build.gradle.kts index 4cfa9e9a1..70fbba426 100644 --- a/simplified-books-controller/build.gradle.kts +++ b/simplified-books-controller/build.gradle.kts @@ -30,6 +30,7 @@ dependencies { implementation(project(":simplified-parser-api")) implementation(project(":simplified-patron-api")) implementation(project(":simplified-presentableerror-api")) + implementation(project(":simplified-futures")) implementation(project(":simplified-profiles-api")) implementation(project(":simplified-profiles-controller-api")) implementation(project(":simplified-services-api")) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt index e58230bf3..d4734fed6 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt @@ -7,18 +7,28 @@ import org.librarysimplified.http.api.LSHTTPResponseStatus import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP.addCredentialsToProperties import org.nypl.simplified.accounts.database.api.AccountType +import org.nypl.simplified.books.api.Book +import org.nypl.simplified.books.api.BookIDs +import org.nypl.simplified.books.book_registry.BookRegistryType +import org.nypl.simplified.books.book_registry.BookStatus +import org.nypl.simplified.books.book_registry.BookWithStatus import org.nypl.simplified.opds.core.OPDSAcquisitionFeedEntry +import org.nypl.simplified.opds.core.OPDSAcquisitionFeedEntryParser +import org.nypl.simplified.opds.core.OPDSParseException import org.nypl.simplified.opds.core.getOrNull import org.nypl.simplified.taskrecorder.api.TaskRecorder import org.nypl.simplified.taskrecorder.api.TaskRecorderType import org.nypl.simplified.taskrecorder.api.TaskResult import org.slf4j.LoggerFactory +import java.io.ByteArrayInputStream +import java.io.InputStream import java.net.URI class BookSelectTask ( private val HTTPClient: LSHTTPClientType, private val account: AccountType, - private val feedEntry: OPDSAcquisitionFeedEntry + private val feedEntry: OPDSAcquisitionFeedEntry, + private val bookRegistry: BookRegistryType ) { private lateinit var taskRecorder: TaskRecorderType @@ -28,12 +38,32 @@ class BookSelectTask ( fun execute() : TaskResult<*> { this.taskRecorder = TaskRecorder.create() - taskRecorder.beginNewStep("Creating an OPDS Select...") + //First create database entry, so we have something we can post status to + + this.taskRecorder.beginNewStep("Create database entry") + //ID of the book, can be new + val bookID = BookIDs.newFromOPDSEntry(this.feedEntry) + + val bookInitial = + Book( + id = bookID, + account = account.id, + cover = null, + thumbnail = null, + entry = feedEntry, + formats = listOf() + ) + + // Get either the book that already was in database, or initialize a new database entry + val book = this.getOrCreateBookDatabaseEntry(bookInitial, feedEntry) + + this.taskRecorder.currentStepSucceeded("Initial database entry successful") + + //Try to add the selection into the database entry + taskRecorder.beginNewStep("Creating an OPDS Select...") try { - //Using the alternate link as it seems to be the same basis as the ref "self" - //FIXFIX - // See if better solution to get the link + //Using the alternate link as the basis for the request val baseURI = feedEntry.alternate.getOrNull() if (baseURI == null) { logger.debug("No alternate link to use as basis for request") @@ -69,8 +99,8 @@ class BookSelectTask ( request.execute().use { response -> when (val status = response.status) { is LSHTTPResponseStatus.Responded.OK -> { - //If successful, answer with success, handle feed refresh elsewhere - this.handleOKRequest(currentURI, status) + // Parse the answer and store the new value to the database and book register + this.handleOKRequest(currentURI, status, book) return taskRecorder.finishSuccess("Success") } is LSHTTPResponseStatus.Responded.Error -> { @@ -89,6 +119,35 @@ class BookSelectTask ( } } + private fun getOrCreateBookDatabaseEntry( + book: Book, + entry: OPDSAcquisitionFeedEntry + ): Book { + this.taskRecorder.beginNewStep("Setting up a book database entry...") + + try { + val database = this.account.bookDatabase + + //If the book is already in the database, use that book instead of the generated one + //If the book is not, we initialize a db entry with the given book + if (!database.books().contains(book.id)) { + database.createOrUpdate(book.id, entry) + } + //Get the entry + val dbEntry = database.entry(book.id) + this.taskRecorder.currentStepSucceeded("Book database initialized for select") + return dbEntry.book + } catch (e: Exception) { + logger.error("[{}]: failed to set up book database: ", book.id.brief(), e) + this.taskRecorder.currentStepFailed( + message = "Could not set up the book database entry.", + errorCode = "Error", + exception = e + ) + throw SelectTaskException.SelectTaskFailed() + } + } + private fun handleHTTPFailure( status: LSHTTPResponseStatus.Failed ) { @@ -106,13 +165,6 @@ class BookSelectTask ( val report = status.properties.problemReport if (report != null) { taskRecorder.addAttributes(report.toMap()) - - //FIXFIX - //Report type for already selected book? - if (report.type == "http://librarysimplified.org/terms/problem/book-already-selected") { - taskRecorder.currentStepSucceeded("It turns out we already selected this book.") - return - } } taskRecorder.currentStepFailed( @@ -121,12 +173,6 @@ class BookSelectTask ( exception = null ) - if (report?.type == "http://librarysimplified.org/terms/problem/select-limit-reached") { - //FIXFIX - //DO we have special error for list being full? set to 50? - throw SelectTaskException.SelectTaskFailed() - } - //Catch 401 with special handling in controller if (report?.type == "http://librarysimplified.org/terms/problem/credentials-invalid") { throw SelectTaskException.SelectAccessTokenExpired() @@ -137,8 +183,70 @@ class BookSelectTask ( private fun handleOKRequest( uri: URI, - status: LSHTTPResponseStatus.Responded.OK + status: LSHTTPResponseStatus.Responded.OK, + book: Book ) { + //Get the server response + val inputStream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)) + + // new entry created from ONLY response + val entry = this.parseOPDSFeedEntry(inputStream, uri) + + //get database + val database = this.account.bookDatabase + + //The version in database might be carrying loan information + //And just overwriting the existing entry with the new one would not carry that information over + if (database.books().contains(book.id)) { + //If book already in db, get the entry and insert the availability information already available + val dbEntry = database.entry(book.id) + //Build a new feed entry + val adjustedEntry = OPDSAcquisitionFeedEntry.newBuilderFrom(entry) + .setAvailability(dbEntry.book.entry.availability) + .build() + //Create the database entry + logger.debug("Book was in database with availability:") + logger.debug(dbEntry.book.entry.availability.toString()) + database.createOrUpdate(book.id, adjustedEntry) + } else { + //otherwise just add the response entry + database.createOrUpdate(book.id, entry) + } + + val updatedEntry = database.entry(book.id) + + //If book has previous status, we want to add it to the status update so it can be reset + val oldBookStatus = this.bookRegistry.bookStatusOrNull(book.id) + logger.debug("OLD BOOK STATUS :{}",oldBookStatus) + //If successful, update the state of the book in database to selected, so other things happen + this.bookRegistry.updateIfStatusIsMoreImportant( + BookWithStatus( + book = updatedEntry.book, + status = BookStatus.Selected(updatedEntry.book.id, oldBookStatus) + ) + ) + taskRecorder.currentStepSucceeded("Book selected successfully") } + private fun parseOPDSFeedEntry( + inputStream: InputStream, + uri: URI + ): OPDSAcquisitionFeedEntry { + taskRecorder.beginNewStep("Parsing the OPDS feed entry...") + val parser = OPDSAcquisitionFeedEntryParser.newParser() + + return try { + inputStream.use { + parser.parseEntryStream(uri, it) + } + } catch (e: OPDSParseException) { + logger.error("OPDS feed parse error: ", e) + taskRecorder.currentStepFailed( + message = "Failed to parse the OPDS feed entry (${e.message}).", + errorCode = "Parse error", + exception = e + ) + throw SelectTaskException.SelectTaskFailed() + } + } } diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt index ab917f6fc..efbf20db6 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt @@ -1,5 +1,6 @@ package org.nypl.simplified.books.controller +import com.io7m.jfunctional.None import com.io7m.jfunctional.Some import org.librarysimplified.http.api.LSHTTPClientType import org.librarysimplified.http.api.LSHTTPResponseStatus @@ -24,6 +25,7 @@ import org.nypl.simplified.books.book_registry.BookWithStatus import org.nypl.simplified.books.controller.api.BooksControllerType import org.nypl.simplified.feeds.api.FeedLoaderType import org.nypl.simplified.feeds.api.FeedLoading +import org.nypl.simplified.opds.core.OPDSAcquisitionFeedEntry import org.nypl.simplified.opds.core.OPDSAvailabilityRevoked import org.nypl.simplified.opds.core.OPDSFeedParserType import org.nypl.simplified.opds.core.OPDSParseException @@ -65,10 +67,12 @@ class BookSyncTask( this.logger.debug("syncing account {}", account.id) this.taskRecorder.beginNewStep("Syncing...") + //Handle keys MDC.put(MDCKeys.ACCOUNT_INTERNAL_ID, account.id.uuid.toString()) MDC.put(MDCKeys.ACCOUNT_PROVIDER_NAME, account.provider.displayName) MDC.put(MDCKeys.ACCOUNT_PROVIDER_ID, account.provider.id.toString()) + //Update provider val provider = this.updateAccountProvider(account) val providerAuth = provider.authentication if (providerAuth == AccountProviderAuthenticationDescription.Anonymous) { @@ -76,6 +80,7 @@ class BookSyncTask( return this.taskRecorder.finishSuccess(Unit) } + //Get credentials to use for request authentication val credentials = account.loginState.credentials if (credentials == null) { this.logger.debug("no credentials, aborting!") @@ -87,33 +92,34 @@ class BookSyncTask( credentials = credentials ) + //The URI to use for getting the loans val loansURI = provider.loansURI if (loansURI == null) { this.logger.debug("no loans URI, aborting!") return this.taskRecorder.finishSuccess(Unit) } - + //Loan request val request = this.http.newRequest(loansURI) .setAuthorization(AccountAuthenticatedHTTP.createAuthorization(credentials)) .addCredentialsToProperties(credentials) .build() + //Initialize loans stream + var loansStream : InputStream = ByteArrayInputStream(ByteArray(0)) + + //Execute loan fetch val response = request.execute() when (val status = response.status) { is LSHTTPResponseStatus.Responded.OK -> { - this.onHTTPOK( - stream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)), - provider = provider, - account = account, - accessToken = status.getAccessToken() - ) + // Save stream for later use + loansStream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)) } is LSHTTPResponseStatus.Responded.Error -> { val recovered = this.onHTTPError(status, account) if (recovered) { - //this.taskRecorder.finishSuccess(Unit) + this.taskRecorder.finishSuccess(Unit) } else { val message = String.format("%s: %d: %s", provider.loansURI, status.properties.status, status.properties.message) val exception = IOException(message) @@ -129,29 +135,29 @@ class BookSyncTask( throw IOException(status.exception) } - //Do the same for selected - //FIXFIX - //Check if this is needed + // Do the same for selected feed val selectedURI = provider.selectedURI if (selectedURI == null) { this.logger.debug("no selected URI, aborting!") return this.taskRecorder.finishSuccess(Unit) } - - val selectedRequest = + //Select request + val selectRequest = this.http.newRequest(selectedURI) .setAuthorization(AccountAuthenticatedHTTP.createAuthorization(credentials)) .addCredentialsToProperties(credentials) .build() - val selectedResponse = selectedRequest.execute() - when (val status =selectedResponse.status) { + //Execute selected fetch + val selectedResponse = selectRequest.execute() + return when (val status = selectedResponse.status) { is LSHTTPResponseStatus.Responded.OK -> { - this.onHTTPOK( - stream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)), + //Handle both the loans and selected streams so the books are added into the database + this.onHTTPOKMultipleFeeds( + loansStream = loansStream, + selectedStream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)), provider = provider, - account = account, - accessToken = status.getAccessToken() + account = account ) this.taskRecorder.finishSuccess(Unit) } @@ -174,7 +180,6 @@ class BookSyncTask( is LSHTTPResponseStatus.Failed -> throw IOException(status.exception) } - return this.taskRecorder.finishSuccess(Unit) } private fun fetchPatronUserProfile( @@ -255,6 +260,10 @@ class BookSyncTask( } } + /** + * On successful HTTP request, update basicToken credentials + * and parse the given feed, saving the books into the book registry. + */ @Throws(IOException::class) private fun onHTTPOK( stream: InputStream, @@ -343,6 +352,158 @@ class BookSyncTask( } } + /** + * Handle successful HTTP with multiple feeds + */ + @Throws(IOException::class) + private fun onHTTPOKMultipleFeeds( + loansStream: InputStream, + selectedStream: InputStream, + provider: AccountProviderType, + account: AccountType + ) { + //Parse multiple streams into one + loansStream.use { loans -> + this.parseSelectedAndLoansFeeds(loans, selectedStream, provider, account) + } + } + + /** + * Parse together two feeds and add the books into the database. Removes from the + * database entries that are not available in either of the feeds. + */ + @Throws(OPDSParseException::class) + private fun parseSelectedAndLoansFeeds( + loansStream: InputStream, + selectedStream: InputStream, + provider: AccountProviderType, + account: AccountType + ) { + //Parse loans + val loansFeed = this.feedParser.parse(provider.selectedURI, loansStream) + + //Parse selected + val selectedFeed = this.feedParser.parse(provider.selectedURI, selectedStream) + + //Combine the feed entries from both feeds into one list, create new IDs for them + val loansMap = loansFeed.feedEntries.associateBy { BookIDs.newFromOPDSEntry(it) } + val selectedMap = selectedFeed.feedEntries.associateBy { BookIDs.newFromOPDSEntry(it) } + + //Get all non-duplicate IDs + val allBookIDs = (loansMap.keys + selectedMap.keys).toSet() + + //Initiate the list for the combined feed entries + val combinedFeedEntries = mutableListOf() + + //Map values based on their id + allBookIDs.map { id -> + //Get the entries for id for both feeds + val loansEntry = loansMap[id] + val selectedEntry = selectedMap[id] + + //If there is a loans entry, use it as a base + if (loansEntry != null) { + //If there is a selected entry, we want to copy its selected value to the loans entry + if (selectedEntry != null) { + //Create new entry based on the loans entry and replace the values you want to replace + //Which currently only selected info + val newEntry = OPDSAcquisitionFeedEntry.newBuilderFrom(loansEntry) + .setSelectedOption(selectedEntry.selected) + .build() + //Add the new entry + combinedFeedEntries.add(newEntry) + } else { + // If book is not selected, we can just add the loans entry + combinedFeedEntries.add(loansEntry) + } + } else { + //If there is no loans entry, we can just add the selected entry + combinedFeedEntries.add(selectedEntry!!) + } + } + + /* + * Obtain the set of books that are on disk already. If any + * of these books are not selected and not loaned, then they have + * expired and/or are unselected and should be deleted. + */ + + val bookDatabase = account.bookDatabase + val existing = bookDatabase.books() + + /* + * Handle each book in the combined feed by checking if matching book is in database + */ + val receivedBooks = HashSet(64) + for (opdsEntry in combinedFeedEntries) { + // Create new id for the entry (will match the ID of the book in the registry, if any) + val bookId = BookIDs.newFromOPDSEntry(opdsEntry) + // Add to the received loans + receivedBooks.add(bookId) + + this.logger.debug("[{}] updating", bookId.brief()) + + // Try to add the book to the database + // Update old entries or add new ones + try { + //Create a new database entry, or update old one + val databaseEntry = bookDatabase.createOrUpdate(bookId, opdsEntry) + //get the book we just added and refresh the registry + val book = databaseEntry.book + //Update the book state in the registry, create the status based on the book entry + this.bookRegistry.update(BookWithStatus(book, BookStatus.fromBook(book))) + } catch (e: BookDatabaseException) { + this.logger.error("[{}] unable to update database entry: ", bookId.brief(), e) + } + } + + /* + * Now delete/revoke any book that previously existed, but is not in the + * received set of IDs we formed previously. + */ + + // Initiate the list into which we collect the book's IDs that need to be revoked + //Not just deleted + val revoking = HashSet(existing.size) + //Go through all id:s in database + for (existingId in existing) { + try { + this.logger.debug("[{}] checking for deletion", existingId.brief()) + + //If book not in loans or selected, delete it + if (!allBookIDs.contains(existingId)) { + val dbEntry = bookDatabase.entry(existingId) + val a = dbEntry.book.entry.availability + val b = dbEntry.book.entry.selected + // If the book availability is revoked, it should be revoked + if (a is OPDSAvailabilityRevoked) { + revoking.add(existingId) + } else { + //Otherwide just deleting will do + this.logger.debug("[{}] deleting", existingId.brief()) + //Load the single entry and add the "neutral" version to the book + this.updateRegistryForBook(account, dbEntry) + //Delete entry from database + dbEntry.delete() + } + } else { + this.logger.debug("[{}] keeping", existingId.brief()) + } + } catch (x: Throwable) { + this.logger.error("[{}]: unable to delete entry: ", existingId.value(), x) + } + } + + /* + * Finish the revocation of any books that need it. + */ + + for (revoke_id in revoking) { + this.logger.debug("[{}] revoking", revoke_id.brief()) + this.booksController.bookRevoke(account.id, revoke_id) + } + } + private fun updateRegistryForBook( account: AccountType, dbEntry: BookDatabaseEntryType diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt index 9543fa091..52d097c51 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt @@ -1,24 +1,43 @@ package org.nypl.simplified.books.controller +import com.io7m.jfunctional.None +import com.io7m.jfunctional.OptionType +import com.io7m.jfunctional.Some import one.irradia.mime.api.MIMECompatibility import org.librarysimplified.http.api.LSHTTPClientType import org.librarysimplified.http.api.LSHTTPRequestBuilderType import org.librarysimplified.http.api.LSHTTPResponseStatus +import org.librarysimplified.mdc.MDCKeys import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP.addCredentialsToProperties import org.nypl.simplified.accounts.database.api.AccountType +import org.nypl.simplified.books.api.BookID +import org.nypl.simplified.books.api.BookIDs +import org.nypl.simplified.books.book_database.api.BookDatabaseEntryType +import org.nypl.simplified.books.book_registry.BookRegistry +import org.nypl.simplified.books.book_registry.BookRegistryType +import org.nypl.simplified.books.book_registry.BookStatus +import org.nypl.simplified.books.book_registry.BookWithStatus import org.nypl.simplified.opds.core.OPDSAcquisitionFeedEntry +import org.nypl.simplified.opds.core.OPDSAcquisitionFeedEntryParser +import org.nypl.simplified.opds.core.OPDSAvailabilityLoanable +import org.nypl.simplified.opds.core.OPDSAvailabilityType +import org.nypl.simplified.opds.core.OPDSParseException import org.nypl.simplified.opds.core.getOrNull import org.nypl.simplified.taskrecorder.api.TaskRecorder import org.nypl.simplified.taskrecorder.api.TaskRecorderType import org.nypl.simplified.taskrecorder.api.TaskResult import org.slf4j.LoggerFactory +import org.slf4j.MDC +import java.io.ByteArrayInputStream +import java.io.InputStream import java.net.URI class BookUnselectTask ( private val HTTPClient: LSHTTPClientType, private val account: AccountType, - private val feedEntry: OPDSAcquisitionFeedEntry + private val feedEntry: OPDSAcquisitionFeedEntry, + private val bookRegistry: BookRegistryType ) { private lateinit var taskRecorder: TaskRecorderType @@ -31,9 +50,16 @@ class BookUnselectTask ( taskRecorder.beginNewStep("Creating an OPDS Unselect...") try { - //Using the alternate link as it seems to be the same basis as the ref "self" - //FIXFIX - // See if better solution to get the link + logger.debug("setting up book database entry") + val database = account.bookDatabase + val bookID = BookIDs.newFromOPDSEntry(feedEntry) + val databaseEntry = database.entry(bookID) + + MDC.put(MDCKeys.BOOK_INTERNAL_ID, databaseEntry.book.id.value()) + MDC.put(MDCKeys.BOOK_TITLE, databaseEntry.book.entry.title) + MDCKeys.put(MDCKeys.BOOK_PUBLISHER, databaseEntry.book.entry.publisher) + + //Using the alternate link as the base for request val baseURI = feedEntry.alternate.getOrNull() if (baseURI == null) { logger.debug("No link to form") @@ -68,8 +94,8 @@ class BookUnselectTask ( request.execute().use { response -> when (val status = response.status) { is LSHTTPResponseStatus.Responded.OK -> { - //If successful, answer with success, handle feed refresh elsewhere - this.handleOKRequest(currentURI, status) + //If successful, store returned value to bookRegistry + this.handleOKRequest(currentURI, status, databaseEntry, bookID) return taskRecorder.finishSuccess("Success") } is LSHTTPResponseStatus.Responded.Error -> { @@ -105,13 +131,6 @@ class BookUnselectTask ( val report = status.properties.problemReport if (report != null) { taskRecorder.addAttributes(report.toMap()) - - //FIXFIX - //Report type for already unselected book? - if (report.type == "http://librarysimplified.org/terms/problem/selection-already-removed") { - taskRecorder.currentStepSucceeded("It turns out we already didn't have this book selected.") - return - } } taskRecorder.currentStepFailed( @@ -128,10 +147,63 @@ class BookUnselectTask ( throw SelectTaskException.UnselectTaskFailed() } + /** + * Update the database and registry so that the selected info is removed + */ private fun handleOKRequest( uri: URI, - status: LSHTTPResponseStatus.Responded.OK + status: LSHTTPResponseStatus.Responded.OK, + databaseEntry: BookDatabaseEntryType, + bookID: BookID ) { - taskRecorder.currentStepSucceeded("Book selected successfully") + //If the book is on loan, the loan info should be retained, and the selected info removed + + //Get the input stream from the delete + val inputStream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)) + + //Returned value,without the selected info + val entry = this.parseOPDSFeedEntry(inputStream, uri) + + //Make a new value from response with old availability info from database + val newEntry = OPDSAcquisitionFeedEntry.newBuilderFrom(entry) + .setAvailability(databaseEntry.book.entry.availability) + .build() + + //Update the database and take the new answer to be put into the registry + val newValue = account.bookDatabase.createOrUpdate(bookID, newEntry) + + //Old status + val statusOld = this.bookRegistry.bookStatusOrNull(bookID) + + //Shows the correct popup before possibly removing the entry + this.bookRegistry.updateIfStatusIsMoreImportant( + BookWithStatus( + newValue.book, + BookStatus.Unselected(bookID, statusOld)) + ) + + taskRecorder.currentStepSucceeded("Book unselected successfully") + } + + private fun parseOPDSFeedEntry( + inputStream: InputStream, + uri: URI + ): OPDSAcquisitionFeedEntry { + taskRecorder.beginNewStep("Parsing the OPDS feed entry...") + val parser = OPDSAcquisitionFeedEntryParser.newParser() + + return try { + inputStream.use { + parser.parseEntryStream(uri, it) + } + } catch (e: OPDSParseException) { + logger.error("OPDS feed parse error: ", e) + taskRecorder.currentStepFailed( + message = "Failed to parse the OPDS feed entry (${e.message}).", + errorCode = "Parse error", + exception = e + ) + throw SelectTaskException.SelectTaskFailed() + } } } diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt index 1a2009b49..3e1f516de 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt @@ -1,7 +1,9 @@ package org.nypl.simplified.books.controller import com.google.common.collect.ImmutableList +import com.google.common.util.concurrent.AsyncFunction import com.google.common.util.concurrent.FluentFuture +import com.google.common.util.concurrent.Futures import com.google.common.util.concurrent.ListeningExecutorService import com.google.common.util.concurrent.MoreExecutors import com.google.common.util.concurrent.SettableFuture @@ -26,7 +28,9 @@ import org.nypl.simplified.accounts.database.api.AccountsDatabaseNonexistentExce import org.nypl.simplified.accounts.registry.api.AccountProviderRegistryEvent import org.nypl.simplified.accounts.registry.api.AccountProviderRegistryType import org.nypl.simplified.analytics.api.AnalyticsType +import org.nypl.simplified.books.api.Book import org.nypl.simplified.books.api.BookID +import org.nypl.simplified.books.api.BookIDs import org.nypl.simplified.books.book_registry.BookPreviewRegistryType import org.nypl.simplified.books.book_registry.BookRegistryType import org.nypl.simplified.books.book_registry.BookStatus @@ -36,6 +40,7 @@ import org.nypl.simplified.books.borrowing.BorrowRequirements import org.nypl.simplified.books.borrowing.BorrowTask import org.nypl.simplified.books.borrowing.BorrowTaskType import org.nypl.simplified.books.borrowing.SAMLDownloadContext +import org.nypl.simplified.books.borrowing.internal.BorrowErrorCodes import org.nypl.simplified.books.controller.api.BookRevokeStringResourcesType import org.nypl.simplified.books.controller.api.BooksControllerType import org.nypl.simplified.books.controller.api.BooksPreviewControllerType @@ -741,13 +746,13 @@ class Controller private constructor( //FIXFIXFIX //Add 401 handling - //Add successful reload? return this.submitTask( - Callable >{ + Callable> { val bookSelectTask = BookSelectTask( HTTPClient = this.lsHttp, account = account, - feedEntry =feedEntry.feedEntry + feedEntry = feedEntry.feedEntry, + bookRegistry = bookRegistry ) bookSelectTask.execute() } @@ -763,13 +768,13 @@ class Controller private constructor( //FIXFIXFIX //Add 401 handling - //Add successful reload? return this.submitTask( Callable >{ val bookUnselectTask = BookUnselectTask( HTTPClient = this.lsHttp, account = account, - feedEntry =feedEntry.feedEntry + feedEntry =feedEntry.feedEntry, + bookRegistry = bookRegistry ) bookUnselectTask.execute() } diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt index 2f3cef231..ca307a2bb 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt @@ -280,6 +280,8 @@ internal class ProfileFeedTask( is BookStatus.Loaned, is BookStatus.RequestingDownload, is BookStatus.RequestingLoan, + is BookStatus.Selected, + is BookStatus.Unselected, is BookStatus.RequestingRevoke -> true } @@ -303,6 +305,8 @@ internal class ProfileFeedTask( is BookStatus.RequestingDownload, is BookStatus.RequestingLoan, is BookStatus.RequestingRevoke, + is BookStatus.Selected, + is BookStatus.Unselected, is BookStatus.Revoked -> false } diff --git a/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookStatus.kt b/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookStatus.kt index 17902ccee..4f3081f82 100644 --- a/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookStatus.kt +++ b/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookStatus.kt @@ -199,6 +199,32 @@ sealed class BookStatus { get() = BookStatusPriorityOrdering.BOOK_STATUS_DOWNLOAD_FAILED } + /** + * The book has just been added to the selected list. This status should be reset. + */ + data class Selected( + override val id: BookID, + + val previousStatus: BookStatus? + ) : BookStatus() { + + override val priority: BookStatusPriorityOrdering + get() = BookStatusPriorityOrdering.BOOK_STATUS_SELECTED + } + + /** + * The book has just been removed from the selected list. This status should be reset. + */ + data class Unselected( + override val id: BookID, + + val previousStatus: BookStatus? + ) : BookStatus() { + + override val priority: BookStatusPriorityOrdering + get() = BookStatusPriorityOrdering.BOOK_STATUS_UNSELECTED + } + /** * The given book not available for loan, but may be placed on hold. */ diff --git a/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookStatusPriorityOrdering.java b/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookStatusPriorityOrdering.java index ac722b76d..e15b3374d 100644 --- a/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookStatusPriorityOrdering.java +++ b/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookStatusPriorityOrdering.java @@ -27,6 +27,16 @@ public enum BookStatusPriorityOrdering BOOK_STATUS_DOWNLOAD_FAILED(90), + /** + * {@link BookStatus.Selected} + */ + BOOK_STATUS_SELECTED(90), + + + /** + * {@link BookStatus.Unselected} + */ + BOOK_STATUS_UNSELECTED(90), /** * {@link BookStatus.Loaned.Downloading.Downloading.DownloadExternalAuthenticationInProgress} */ diff --git a/simplified-main/src/main/java/org/librarysimplified/main/MainFragment.kt b/simplified-main/src/main/java/org/librarysimplified/main/MainFragment.kt index 06716c520..720bb25bc 100644 --- a/simplified-main/src/main/java/org/librarysimplified/main/MainFragment.kt +++ b/simplified-main/src/main/java/org/librarysimplified/main/MainFragment.kt @@ -266,6 +266,8 @@ class MainFragment : Fragment(R.layout.main_tabbed_host) { is BookStatus.RequestingDownload, is BookStatus.RequestingLoan, is BookStatus.RequestingRevoke, + is BookStatus.Selected, + is BookStatus.Unselected, is BookStatus.Revoked, null -> { // do nothing diff --git a/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt b/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt index 5e356c9a1..0bf8b2437 100644 --- a/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt +++ b/simplified-opds-auth-document-api/src/main/java/org/nypl/simplified/opds/auth_document/api/AuthenticationDocument.kt @@ -72,8 +72,8 @@ data class AuthenticationDocument( val loansURI: URI? = this.links.find { link -> link.relation == "http://opds-spec.org/shelf" }?.hrefURI - val selectedURI: URI? = //FIXFIX change to correct : http://opds-spec.org/shelf/selected_books - this.links.find { link -> link.relation == "http://opds-spec.org/shelf" }?.hrefURI + val selectedURI: URI? = + this.links.find { link -> link.relation == "http://opds-spec.org/shelf/selected_books" }?.hrefURI val cardCreatorURI: URI? = this.links.find { link -> link.relation == "register" }?.hrefURI diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookAvailabilityStrings.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookAvailabilityStrings.kt index 502b9eb07..d0cf58f5a 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookAvailabilityStrings.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookAvailabilityStrings.kt @@ -67,6 +67,10 @@ object CatalogBookAvailabilityStrings { "" is BookStatus.ReachedLoanLimit -> "" + is BookStatus.Selected -> + "" + is BookStatus.Unselected -> + "" } } diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt index f6b21aff5..067e237da 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt @@ -11,6 +11,7 @@ import android.widget.ImageView import android.widget.LinearLayout import android.widget.ProgressBar import android.widget.TextView +import android.widget.Toast import androidx.appcompat.app.AlertDialog import androidx.core.content.ContextCompat import com.google.android.material.dialog.MaterialAlertDialogBuilder @@ -30,6 +31,7 @@ import org.librarysimplified.services.api.Services import org.nypl.simplified.android.ktx.supportActionBar import org.nypl.simplified.books.api.Book import org.nypl.simplified.books.api.BookFormat +import org.nypl.simplified.books.api.BookID import org.nypl.simplified.books.book_database.api.BookFormats import org.nypl.simplified.books.book_registry.BookPreviewStatus import org.nypl.simplified.books.book_registry.BookStatus @@ -612,6 +614,8 @@ class CatalogBookDetailFragment : Fragment(R.layout.book_detail) { is BookStatus.DownloadExternalAuthenticationInProgress -> { this.onBookStatusDownloadExternalAuthenticationInProgress() } + is BookStatus.Selected -> this.onBookStatusBookSelected(book.book.id, status) + is BookStatus.Unselected -> this.onBookStatusBookUnselected(book.book.id, status) } } @@ -791,6 +795,23 @@ class CatalogBookDetailFragment : Fragment(R.layout.book_detail) { viewModel.resetInitialBookStatus(this.parameters.feedEntry) } + /** + * Show a toast for successful select and trigger status reset + */ + private fun onBookStatusBookSelected(bookID: BookID, status: BookStatus) { + Toast.makeText(this.requireContext(), getString(R.string.catalogBookSelect), Toast.LENGTH_SHORT).show() + viewModel.resetPreviousBookStatus(bookID, status, true) + logger.debug("BookStatusReset") + } + + /** + * Show a toast for successful unselect and trigger status reset + */ + private fun onBookStatusBookUnselected(bookID: BookID, status: BookStatus) { + Toast.makeText(this.requireContext(), getString(R.string.catalogBookUnselect), Toast.LENGTH_SHORT).show() + viewModel.resetPreviousBookStatus(bookID, status, false) + logger.debug("BookStatusResetToPrevious") + } private fun onBookStatusHoldable( bookStatus: BookStatus.Holdable, bookPreviewStatus: BookPreviewStatus diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt index 4b5f78f87..e35c056d4 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt @@ -107,6 +107,7 @@ class CatalogBookDetailViewModel( mutableMapOf() private fun onBookStatusEvent(event: BookStatusEvent) { + logger.debug("ALALALALALLALALALALALALALALLA") val bookWithStatus = this.createBookWithStatus() this.bookWithStatusMutable.value = Pair(bookWithStatus, bookWithStatusMutable.value!!.second) } @@ -261,6 +262,7 @@ class CatalogBookDetailViewModel( feedEntry: FeedEntry.FeedEntryOPDS, callback: (BookWithStatus) -> Unit ) { + logger.debug("Observer registered") this.bookModels.getOrPut(feedEntry.bookID, { BookModel(feedEntry) }).onBookChanged.add(callback) this.notifyBookStatus(feedEntry, callback) } @@ -306,7 +308,7 @@ class CatalogBookDetailViewModel( } override fun unselectBook(feedEntry: FeedEntry.FeedEntryOPDS) { - booksController.bookAddToSelected( + booksController.bookRemoveFromSelected( accountID = profilesController.profileCurrent().mostRecentAccount().id, feedEntry = feedEntry ) @@ -318,6 +320,30 @@ class CatalogBookDetailViewModel( this.bookWithStatusMutable.value = Pair(initialBookStatus, BookPreviewStatus.None) } + /** + * Reset to the previous status provided in the current status, if not provided, generate it from the book. + */ + override fun resetPreviousBookStatus(bookID: BookID, status: BookStatus, selected: Boolean) { + //Cast tho the correct status depending if select is true or not + val previousStatus: BookStatus? = if (selected) { + val currentStatus = status as BookStatus.Selected + currentStatus.previousStatus + } else { + val currentStatus = status as BookStatus.Unselected + currentStatus.previousStatus + } + // If there is a previous status, set that as the new status in book registry + if (previousStatus != null) { + val bookWithStatus = this.bookRegistry.bookOrNull(bookID) + this.bookRegistry.update(BookWithStatus(bookWithStatus!!.book, previousStatus)) + } else { + // The book for certain has been added to the bookRegistry so if no status provided, create + //one from the bookRegistry entry + val bookWithStatus = this.bookRegistry.bookOrNull(bookID) + this.bookRegistry.update(BookWithStatus(bookWithStatus!!.book, BookStatus.fromBook(bookWithStatus.book))) + } + } + override fun borrowMaybeAuthenticated(book: Book) { // do nothing } diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt index 2ba8eaa43..1c0b6959d 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt @@ -270,6 +270,7 @@ class CatalogFeedViewModel( is BookStatus.Loaned, is BookStatus.Revoked -> { if (this.state.arguments.isLocallyGenerated) { + //Selecting and unselecting can trigger this for a book that is not on loan this.reloadFeed() } } @@ -285,6 +286,8 @@ class CatalogFeedViewModel( is BookStatus.RequestingDownload, is BookStatus.RequestingLoan, is BookStatus.RequestingRevoke, + is BookStatus.Selected, + is BookStatus.Unselected, null -> { // do nothing } @@ -1027,6 +1030,32 @@ class CatalogFeedViewModel( this.bookRegistry.update(initialBookStatus) } + /** + * Reset the book in registry to its previous status, or generate a new one from the available information + * if there isn't one + */ + override fun resetPreviousBookStatus(bookID: BookID, status: BookStatus, selected: Boolean) { + logger.debug("Resetting status: {}", status) + //Cast tho the correct status depending if select is true or not + val previousStatus: BookStatus? = if (selected) { + val currentStatus = status as BookStatus.Selected + currentStatus.previousStatus + } else { + val currentStatus = status as BookStatus.Unselected + currentStatus.previousStatus + } + // If there is a previous status, set that as the new status in book registry + if (previousStatus != null) { + val bookWithStatus = this.bookRegistry.bookOrNull(bookID) + this.bookRegistry.update(BookWithStatus(bookWithStatus!!.book, previousStatus)) + } else { + // The book for certain has been added to the bookRegistry so if no status provided, create + //one from the bookRegistry entry + val bookWithStatus = this.bookRegistry.bookOrNull(bookID) + this.bookRegistry.update(BookWithStatus(bookWithStatus!!.book, BookStatus.fromBook(bookWithStatus.book))) + } + } + override fun registerObserver( feedEntry: FeedEntry.FeedEntryOPDS, callback: (BookWithStatus) -> Unit diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt index e2629c5b4..c5587e198 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewHolder.kt @@ -9,6 +9,7 @@ import android.widget.Button import android.widget.ImageView import android.widget.ProgressBar import android.widget.TextView +import android.widget.Toast import androidx.appcompat.app.AlertDialog import androidx.core.content.ContextCompat import com.google.android.material.dialog.MaterialAlertDialogBuilder @@ -130,7 +131,7 @@ class CatalogPagedViewHolder( } } - private fun onFeedEntryOPDS(item: FeedEntryOPDS) { + private fun onFeedEntryOPDS(item: FeedEntryOPDS, book: Book) { this.setVisibilityIfNecessary(this.corrupt, View.GONE) this.setVisibilityIfNecessary(this.error, View.GONE) this.setVisibilityIfNecessary(this.idle, View.VISIBLE) @@ -156,7 +157,7 @@ class CatalogPagedViewHolder( null -> "" } //If there is a selected date, the book is selected - if (item.feedEntry.selected is Some) { + if (book.entry.selected is Some) { //Set the drawable as the "checked" version this.idleSelectedButton.setImageDrawable( ContextCompat.getDrawable(context,R.drawable.baseline_check_circle_24) @@ -173,7 +174,7 @@ class CatalogPagedViewHolder( ContextCompat.getDrawable(context,R.drawable.round_add_circle_outline_24) ) //Add audio description - this.idleSelectedButton.contentDescription = context.getString(R.string.catalogAccessibilityBookSelect) + this.idleSelectedButton.contentDescription = context.getString(R.string.catalogAccessibilityBookUnselect) this.idleSelectedButton.setOnClickListener { //Add book to selected this.listener.selectBook(item) @@ -198,7 +199,7 @@ class CatalogPagedViewHolder( } private fun onBookChanged(bookWithStatus: BookWithStatus) { - this.onFeedEntryOPDS(this.feedEntry as FeedEntryOPDS) + this.onFeedEntryOPDS(this.feedEntry as FeedEntryOPDS, bookWithStatus.book) this.onBookWithStatus(bookWithStatus) this.checkSomethingIsVisible() } @@ -256,6 +257,8 @@ class CatalogPagedViewHolder( this.onBookStatusDownloadWaitingForExternalAuthentication(book.book) is BookStatus.DownloadExternalAuthenticationInProgress -> this.onBookStatusDownloadExternalAuthenticationInProgress(book.book) + is BookStatus.Selected -> this.onBookStatusSelected(book) + is BookStatus.Unselected -> this.onBookStatusUnselected(book) } } @@ -733,6 +736,24 @@ class CatalogPagedViewHolder( this.progressProgress.isIndeterminate = true } + /** + * Show toast informing user that the book has been added to selected books + * and reset bookStatus to what it was before selection + */ + private fun onBookStatusSelected(book: BookWithStatus) { + Toast.makeText(this.context, context.getString(R.string.catalogBookSelect, book.book.entry.title), Toast.LENGTH_SHORT).show() + this.listener.resetPreviousBookStatus(book.book.id, book.status, true) + } + + /** + * Show toast informing user that the book has been removed from selected books + * and reset bookStatus to what it was before + */ + private fun onBookStatusUnselected(book: BookWithStatus) { + Toast.makeText(this.context, context.getString(R.string.catalogBookUnselect, book.book.entry.title), Toast.LENGTH_SHORT).show() + this.listener.resetPreviousBookStatus(book.book.id, book.status, false) + } + fun unbind() { val currentFeedEntry = this.feedEntry if (currentFeedEntry is FeedEntryOPDS) { diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewListener.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewListener.kt index 85330fc99..12fb4909a 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewListener.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogPagedViewListener.kt @@ -2,6 +2,8 @@ package org.librarysimplified.ui.catalog import org.nypl.simplified.books.api.Book import org.nypl.simplified.books.api.BookFormat +import org.nypl.simplified.books.api.BookID +import org.nypl.simplified.books.book_registry.BookStatus import org.nypl.simplified.books.book_registry.BookWithStatus import org.nypl.simplified.feeds.api.FeedEntry import org.nypl.simplified.taskrecorder.api.TaskResult @@ -36,6 +38,8 @@ interface CatalogPagedViewListener { fun resetInitialBookStatus(feedEntry: FeedEntry.FeedEntryOPDS) + fun resetPreviousBookStatus(bookID: BookID, status: BookStatus, selected: Boolean) + fun reserveMaybeAuthenticated(book: Book) fun revokeMaybeAuthenticated(book: Book) diff --git a/simplified-ui-catalog/src/main/res/values/stringsCatalog.xml b/simplified-ui-catalog/src/main/res/values/stringsCatalog.xml index 38f3dca62..c7f4adee9 100644 --- a/simplified-ui-catalog/src/main/res/values/stringsCatalog.xml +++ b/simplified-ui-catalog/src/main/res/values/stringsCatalog.xml @@ -4,8 +4,8 @@ Get Delete Download - Add book to your selected books - Remove book from your selected books + Add this book to your selected books + Remove this book from your selected books Cancel download Details Dismiss @@ -93,7 +93,8 @@ Search %1$s hours, %2$s minutes Read more - + %1$s added to favorites + %1$s removed from favorites You have reached your loan limit. You cannot borrow anything further until you return something. Loan limit reached. OK From 0dc9e0d62f7c3c41145661578c791f15f7cc9b54 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Thu, 23 Jan 2025 17:31:17 +0200 Subject: [PATCH 05/23] Minor UI fixes --- .../ui/catalog/CatalogBookDetailFragment.kt | 16 ++++++++++------ .../src/main/res/layout/book_cell_idle.xml | 1 - 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt index 067e237da..3ca079c69 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailFragment.kt @@ -614,8 +614,12 @@ class CatalogBookDetailFragment : Fragment(R.layout.book_detail) { is BookStatus.DownloadExternalAuthenticationInProgress -> { this.onBookStatusDownloadExternalAuthenticationInProgress() } - is BookStatus.Selected -> this.onBookStatusBookSelected(book.book.id, status) - is BookStatus.Unselected -> this.onBookStatusBookUnselected(book.book.id, status) + is BookStatus.Selected -> { + this.onBookStatusBookSelected(book.book.id,book.book.entry.title, status) + } + is BookStatus.Unselected -> { + this.onBookStatusBookUnselected(book.book.id, book.book.entry.title,status) + } } } @@ -798,8 +802,8 @@ class CatalogBookDetailFragment : Fragment(R.layout.book_detail) { /** * Show a toast for successful select and trigger status reset */ - private fun onBookStatusBookSelected(bookID: BookID, status: BookStatus) { - Toast.makeText(this.requireContext(), getString(R.string.catalogBookSelect), Toast.LENGTH_SHORT).show() + private fun onBookStatusBookSelected(bookID: BookID, title: String, status: BookStatus) { + Toast.makeText(this.requireContext(), getString(R.string.catalogBookSelect, title), Toast.LENGTH_SHORT).show() viewModel.resetPreviousBookStatus(bookID, status, true) logger.debug("BookStatusReset") } @@ -807,8 +811,8 @@ class CatalogBookDetailFragment : Fragment(R.layout.book_detail) { /** * Show a toast for successful unselect and trigger status reset */ - private fun onBookStatusBookUnselected(bookID: BookID, status: BookStatus) { - Toast.makeText(this.requireContext(), getString(R.string.catalogBookUnselect), Toast.LENGTH_SHORT).show() + private fun onBookStatusBookUnselected(bookID: BookID, title: String, status: BookStatus) { + Toast.makeText(this.requireContext(), getString(R.string.catalogBookUnselect, title), Toast.LENGTH_SHORT).show() viewModel.resetPreviousBookStatus(bookID, status, false) logger.debug("BookStatusResetToPrevious") } diff --git a/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml b/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml index 248914c08..7aefffca5 100644 --- a/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml +++ b/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml @@ -61,7 +61,6 @@ android:layout_height="wrap_content" android:layout_marginTop="8dp" android:layout_marginEnd="16dp" - android:layout_marginStart="16dp" android:gravity="right" android:src="@drawable/round_add_circle_outline_24" app:layout_constraintStart_toEndOf="@id/bookCellIdleTitle" From 8b77244d31ee369d30da674c5d325092e8f2f730 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Wed, 5 Feb 2025 14:50:23 +0200 Subject: [PATCH 06/23] Move selected location and ensure correct funtions Moved the selected feed to now be shown in the same bottom tab as loans. Removing a loan removed the entry from database and register, so added a selected check, so the book is only removed if it's also not selected. Added translatable strings for the different tabs and ensured correct functioning even when feed is empty (used to not show the nvigation tabs if feed was empty). Added descriptions to functions. Removed the seected button from the settings page. Ensured that foltering and sorting in the new views works correctly. NOTE: All favorites are now stored in the local database which might cause slowness and storage spage filling if there are alot of them. --- .../books/controller/BookDeleteTask.kt | 14 +- .../books/controller/BookRevokeTask.kt | 27 ++- .../books/controller/BookSelectTask.kt | 2 +- .../books/controller/BookUnselectTask.kt | 2 +- .../books/controller/ProfileFeedTask.kt | 101 +++++++- .../books/book_registry/BookRegistry.kt | 4 +- .../feeds/api/FeedBooksSelection.java | 7 +- .../nypl/simplified/feeds/api/FeedFacet.kt | 30 +++ .../api/FeedFacetPseudoTitleProviderType.kt | 3 + .../nypl/simplified/feeds/api/FeedFacets.kt | 11 +- .../main/MainFragmentListenerDelegate.kt | 14 -- .../main/MainFragmentViewModel.kt | 8 + .../controller/api/ProfileFeedRequest.kt | 7 + .../ui/accounts/AccountDetailEvent.kt | 5 - .../ekirjasto/EKirjastoAccountFragment.kt | 16 -- .../src/main/res/layout/account_ekirjasto.xml | 9 - .../ui/catalog/CatalogBookDetailViewModel.kt | 9 + .../ui/catalog/CatalogFeedArguments.kt | 18 ++ .../ui/catalog/CatalogFeedFragment.kt | 33 +++ .../ui/catalog/CatalogFeedState.kt | 1 + .../ui/catalog/CatalogFeedViewModel.kt | 217 +++++++++++++++++- .../res/drawable/baseline_check_circle_24.xml | 11 +- .../src/main/res/values/stringsFeed.xml | 5 + .../ui/navigation/tabs/BottomNavigators.kt | 88 ++++++- 24 files changed, 568 insertions(+), 74 deletions(-) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookDeleteTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookDeleteTask.kt index 754067103..eb5e8b282 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookDeleteTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookDeleteTask.kt @@ -1,11 +1,15 @@ package org.nypl.simplified.books.controller +import com.io7m.jfunctional.Some +import org.joda.time.DateTime import org.librarysimplified.mdc.MDCKeys import org.nypl.simplified.accounts.api.AccountID import org.nypl.simplified.accounts.database.api.AccountType import org.nypl.simplified.books.api.BookID import org.nypl.simplified.books.book_database.api.BookDatabaseException import org.nypl.simplified.books.book_registry.BookRegistryType +import org.nypl.simplified.books.book_registry.BookStatus +import org.nypl.simplified.books.book_registry.BookWithStatus import org.nypl.simplified.profiles.api.ProfileID import org.nypl.simplified.profiles.api.ProfilesDatabaseType import org.nypl.simplified.taskrecorder.api.TaskRecorder @@ -44,8 +48,14 @@ class BookDeleteTask( MDC.put(MDCKeys.BOOK_TITLE, entry.book.entry.title) MDCKeys.put(MDCKeys.BOOK_PUBLISHER, entry.book.entry.publisher) - entry.delete() - this.bookRegistry.clearFor(entry.book.id) + //If the book is still selected, don't delete the book, just update + if (entry.book.entry.selected is Some) { + this.bookRegistry.update(BookWithStatus(entry.book, BookStatus.fromBook(entry.book))) + } else { + //Otherwise delete the db and registry entries + entry.delete() + this.bookRegistry.clearFor(entry.book.id) + } this.taskRecorder.finishSuccess(Unit) } catch (e: Exception) { this.taskRecorder.currentStepFailed( diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt index a736ab84a..ea0bf4879 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt @@ -1,5 +1,7 @@ package org.nypl.simplified.books.controller +import com.io7m.jfunctional.None +import com.io7m.jfunctional.OptionType import com.io7m.jfunctional.Some import com.io7m.junreachable.UnreachableCodeException import org.joda.time.DateTime @@ -86,12 +88,33 @@ class BookRevokeTask( this.debug("revoke") this.taskRecorder.beginNewStep(this.revokeStrings.revokeStarted) + //Set the status in bookRegistry to Requesting revoke this.publishRequestingRevokeStatus() + //Setup the database, by looking up the current database entry for the book + //And publishing the revoke status again this.setupBookDatabaseEntry(account) + //Revoke the book based on its format this.revokeFormatHandle(account) + //Revoke request is sent to server, we set the revoke status again + //and from the answer we form a new entry, + //That we store to the db and register this.revokeNotifyServer(account) - this.revokeNotifyServerDeleteBook() - this.bookRegistry.clearFor(this.bookID) + + //Get the registry entry + val revokeBook = bookRegistry.books()[this.bookID] + + //If there is a book and it is selected, just publish revoked status and then + //Update registry with the status created form the book + if (revokeBook != null && revokeBook.book.entry.selected is Some) { + this.publishRevokedStatus() + this.publishStatusFromDatabase() + this.bookRegistry.update(BookWithStatus(revokeBook.book, BookStatus.fromBook(revokeBook.book))) + } else { + //If not selected, we delete the book from database + //And we clear the registry too + this.revokeNotifyServerDeleteBook() + this.bookRegistry.clearFor(this.bookID) + } return this.taskRecorder.finishSuccess(Unit) } diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt index d4734fed6..b1f09893b 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt @@ -219,7 +219,7 @@ class BookSelectTask ( val oldBookStatus = this.bookRegistry.bookStatusOrNull(book.id) logger.debug("OLD BOOK STATUS :{}",oldBookStatus) //If successful, update the state of the book in database to selected, so other things happen - this.bookRegistry.updateIfStatusIsMoreImportant( + this.bookRegistry.update( BookWithStatus( book = updatedEntry.book, status = BookStatus.Selected(updatedEntry.book.id, oldBookStatus) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt index 52d097c51..82f8704a4 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt @@ -176,7 +176,7 @@ class BookUnselectTask ( val statusOld = this.bookRegistry.bookStatusOrNull(bookID) //Shows the correct popup before possibly removing the entry - this.bookRegistry.updateIfStatusIsMoreImportant( + this.bookRegistry.update( BookWithStatus( newValue.book, BookStatus.Unselected(bookID, statusOld)) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt index ca307a2bb..ad4588aa8 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt @@ -1,5 +1,6 @@ package org.nypl.simplified.books.controller +import com.io7m.jfunctional.None import org.nypl.simplified.accounts.api.AccountID import org.nypl.simplified.accounts.api.AccountLoginState import org.nypl.simplified.books.book_registry.BookRegistryReadableType @@ -13,6 +14,8 @@ import org.nypl.simplified.feeds.api.FeedFacet import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.FilteringForAccount import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.Sorting import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.Sorting.SortBy +import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.FilteringForFeed +import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.FilteringForFeed.FilterBy import org.nypl.simplified.feeds.api.FeedSearch import org.nypl.simplified.profiles.controller.api.ProfileFeedRequest import org.nypl.simplified.profiles.controller.api.ProfilesControllerType @@ -39,7 +42,9 @@ internal class ProfileFeedTask( * Generate facets. */ - val facetGroups = this.makeFacets() + //Chose the tabs we want to show on the my books page + val doSelectFacets = request.feedSelection != FeedBooksSelection.BOOKS_FEED_HOLDS + val facetGroups = this.makeFacets(doSelectFacets) val facets = facetGroups.values.flatten() val feed = @@ -57,14 +62,25 @@ internal class ProfileFeedTask( val books = this.collectAllBooks(this.bookRegistry) this.logger.debug("collected {} candidate books", books.size) + //Get the correct filter based on the bookStatus of the books in the registry val filter = this.selectFeedFilter(this.request) + //Filter the books based on their bookStatus this.filterBooks(filter, books) this.logger.debug("after filtering, {} candidate books remain", books.size) + //If we look at selected feed, filter out the non selected books + //This is not done by the filtering, since the selection can not be seen from bookStatus + if(this.request.feedSelection == FeedBooksSelection.BOOKS_FEED_SELECTED) { + this.searchSelectedBooks(books) + this.logger.debug("after selecting, {} candidate books remain", books.size) + } + //If there is a search term, filter the unfitting books out this.searchBooks(this.request.search, books) this.logger.debug("after searching, {} candidate books remain", books.size) + //Sort the books in the way we want this.sortBooks(this.request.sortBy, books) this.logger.debug("after sorting, {} candidate books remain", books.size) + //Create feed entries for all the books for (book in books) { feed.entriesInOrder.add( FeedEntry.FeedEntryOPDS( @@ -80,13 +96,25 @@ internal class ProfileFeedTask( } } - private fun makeFacets(): Map> { + /** + * Create the facets shown on the top of the feed. If fragment should handle + * multiple different feeds (or selections), create the feed selection facets. + * @param showSelectionFacets true, if fragment should have navigation facets for different feeds + */ + private fun makeFacets(showSelectionFacets: Boolean): Map> { + //The facets for switching between multiple feeds + val selecting = this.makeSelectionFacets() + //The facets for sorting val sorting = this.makeSortingFacets() + //The facets for filtering val filtering = this.makeFilteringFacets() val results = mutableMapOf>() + //If we want to show the feed facet, add it + if (showSelectionFacets) { + results[selecting.first] = selecting.second + } results[sorting.first] = sorting.second results[filtering.first] = filtering.second - check(results.size == 2) return results.toMap() } @@ -109,9 +137,42 @@ internal class ProfileFeedTask( return Pair(this.request.facetTitleProvider.collection, facets) } + private fun makeSelectionFacets(): Pair> { + val facets = mutableListOf() + //Create entries for all FilterBy options + val values = FilterBy.entries.toTypedArray() + //Create a facet for each of the bookFilters + for (filterFacet in values) { + //Set the activity of the facet based on the FilterBy of the request + val active = filterFacet == this.request.filterBy + //Get the translatable title of the tab + val title = + when (filterFacet) { + FilterBy.FILTER_BY_LOANS -> this.request.facetTitleProvider.showTabLoans + FilterBy.FILTER_BY_HOLDS -> this.request.facetTitleProvider.showTabHolds + FilterBy.FILTER_BY_SELECTED -> this.request.facetTitleProvider.showTabSelected + } + //Determine which feed should be shown, based on what we're filtering by + val selectedFeed = + when(filterFacet) { + FilterBy.FILTER_BY_LOANS -> FeedBooksSelection.BOOKS_FEED_LOANED + FilterBy.FILTER_BY_HOLDS -> FeedBooksSelection.BOOKS_FEED_HOLDS + FilterBy.FILTER_BY_SELECTED -> FeedBooksSelection.BOOKS_FEED_SELECTED + } + //Currently we don't want to show the button for holds feed + //So we don't add it to the facets + + if (selectedFeed != FeedBooksSelection.BOOKS_FEED_HOLDS) { + facets.add(FilteringForFeed(title, active, selectedFeed, filterFacet)) + } + } + //The name is not shown anywhere, so have it just be something relevant + return Pair("FeedTabs", facets) + } + private fun makeSortingFacets(): Pair> { val facets = mutableListOf() - val values = SortBy.values() + val values = SortBy.entries.toTypedArray() for (sortingFacet in values) { val active = sortingFacet == this.request.sortBy val title = @@ -146,6 +207,22 @@ internal class ProfileFeedTask( } } + /** + * Removes books that should not be in the selected book list. + */ + private fun searchSelectedBooks( + books: ArrayList + ) { + val iterator = books.iterator() + while (iterator.hasNext()) { + val book = iterator.next() + //If there is no selected info, remove from books + if (book.book.entry.selected is None) { + iterator.remove() + } + } + } + /** * Split the given search string into a list of uppercase search terms. */ @@ -174,7 +251,7 @@ internal class ProfileFeedTask( } private fun sortBooksByTitle(books: ArrayList) { - Collections.sort(books) { book0, book1 -> + books.sortWith { book0, book1 -> val entry0 = book0.book.entry val entry1 = book1.book.entry entry0.title.compareTo(entry1.title) @@ -182,7 +259,7 @@ internal class ProfileFeedTask( } private fun sortBooksByAuthor(books: ArrayList) { - Collections.sort(books) { book0, book1 -> + books.sortWith { book0, book1 -> val entry0 = book0.book.entry val entry1 = book1.book.entry val authors1 = entry0.authors @@ -312,6 +389,17 @@ internal class ProfileFeedTask( } } + /** + * Return true if the book is usable for selected feed + */ + private fun usableForSelectedFeed(status: BookStatus): Boolean { + //Allow the usage for any Bookstatus, expect a book when the book is just unselected + return when (status) { + is BookStatus.Unselected -> false + else -> true + } + } + /** * @return `true` if any of the given search terms match the given book, or the list of * search terms is empty @@ -350,6 +438,7 @@ internal class ProfileFeedTask( return when (request.feedSelection) { FeedBooksSelection.BOOKS_FEED_LOANED -> ::usableForBooksFeed FeedBooksSelection.BOOKS_FEED_HOLDS -> ::usableForHoldsFeed + FeedBooksSelection.BOOKS_FEED_SELECTED -> ::usableForSelectedFeed } } } diff --git a/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookRegistry.kt b/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookRegistry.kt index e5fab0ca7..b54e08a6d 100644 --- a/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookRegistry.kt +++ b/simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookRegistry.kt @@ -79,12 +79,12 @@ class BookRegistry private constructor( val updatePri = status.status.priority if (currentPri.priority <= updatePri.priority) { - this.logger.debug("current {} <= {}, updating", current, status) + this.logger.debug("current {} <= {}, updating", current.status, status.status) this.update(status) return } - this.logger.debug("current {} > {}, not updating", current, status) + this.logger.debug("current {} > {}, not updating", current.status, status.status) } else { this.update(status) } diff --git a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedBooksSelection.java b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedBooksSelection.java index 669f79631..460c526ac 100644 --- a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedBooksSelection.java +++ b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedBooksSelection.java @@ -18,5 +18,10 @@ public enum FeedBooksSelection implements Serializable * Generate a feed of books that are currently on hold. */ - BOOKS_FEED_HOLDS + BOOKS_FEED_HOLDS, + + /** + * Generate a feed of books that are currently selected. + */ + BOOKS_FEED_SELECTED } diff --git a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacet.kt b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacet.kt index a71e24a5a..01f3e7f8d 100644 --- a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacet.kt +++ b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacet.kt @@ -43,6 +43,36 @@ sealed class FeedFacet : Serializable { val account: AccountID? ) : FeedFacetPseudo() + /** + * A feed select facet. + */ + data class FilteringForFeed( + override val title: String, + override val isActive: Boolean, + val selectedFeed: FeedBooksSelection, + val filterBy: FilterBy + ) : FeedFacetPseudo() { + enum class FilterBy { + + /** + * Filter loans. + */ + + FILTER_BY_LOANS, + + /** + * Filter holds. + */ + + FILTER_BY_HOLDS, + + /** + * Filter the selected. + */ + FILTER_BY_SELECTED + } + } + /** * A sorting facet. */ diff --git a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacetPseudoTitleProviderType.kt b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacetPseudoTitleProviderType.kt index 81938a4e0..f6a590197 100644 --- a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacetPseudoTitleProviderType.kt +++ b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacetPseudoTitleProviderType.kt @@ -17,4 +17,7 @@ interface FeedFacetPseudoTitleProviderType { val show: String val showAll: String val showOnLoan: String + val showTabSelected: String + val showTabLoans: String + val showTabHolds: String } diff --git a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacets.kt b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacets.kt index 4264d7e7a..ec55fdd81 100644 --- a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacets.kt +++ b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedFacets.kt @@ -69,6 +69,8 @@ object FeedFacets { } /** + * Checks if a facet is an entry point. Returns true for opdsFacets with the + * correct group type, and for pseudofacets that are the kind of FilteringForFeed * @return `true` if the given facet is "entry point" typed */ @@ -78,7 +80,14 @@ object FeedFacets { is FeedFacet.FeedFacetOPDS -> facet.opdsFacet.groupType == Option.some(ENTRYPOINT_FACET_GROUP_TYPE) is FeedFacet.FeedFacetPseudo -> - false + when (facet) { + is FeedFacet.FeedFacetPseudo.FilteringForFeed -> { + true + } + else -> { + false + } + } } } } diff --git a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt index 6a3a3ef35..f9d09991b 100644 --- a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt +++ b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt @@ -430,20 +430,6 @@ internal class MainFragmentListenerDelegate( this.openDependentPage() state } - - //FIXFIX - is AccountDetailEvent.OpenSelectedFragment -> { - val accountID = profilesController.profileCurrent().mostRecentAccount().id - val selectedURI = profilesController.profileCurrent().mostRecentAccount().provider.selectedURI!! - val arguments = CatalogFeedArguments.CatalogFeedArgumentsRemote( - title = "Selected", - ownership = CatalogFeedOwnership.OwnedByAccount(accountID), - feedURI = selectedURI, - isSearchResults = false - ) - this.openFeed(arguments) - state - } } } diff --git a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentViewModel.kt b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentViewModel.kt index fb6d8693d..99d2e30e1 100644 --- a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentViewModel.kt +++ b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentViewModel.kt @@ -224,5 +224,13 @@ class MainFragmentViewModel( get() = this.resources.getString(R.string.feedShowAll) override val showOnLoan: String get() = this.resources.getString(R.string.feedShowOnLoan) + override val showTabSelected: String + get() = this.resources.getString(R.string.feedFacetSelected) + + override val showTabLoans: String + get() = this.resources.getString(R.string.feedFacetLoans) + + override val showTabHolds: String + get() = this.resources.getString(R.string.feedFacetHolds) } } diff --git a/simplified-profiles-controller-api/src/main/java/org/nypl/simplified/profiles/controller/api/ProfileFeedRequest.kt b/simplified-profiles-controller-api/src/main/java/org/nypl/simplified/profiles/controller/api/ProfileFeedRequest.kt index 6f8533443..0e12604f1 100644 --- a/simplified-profiles-controller-api/src/main/java/org/nypl/simplified/profiles/controller/api/ProfileFeedRequest.kt +++ b/simplified-profiles-controller-api/src/main/java/org/nypl/simplified/profiles/controller/api/ProfileFeedRequest.kt @@ -5,6 +5,7 @@ import org.nypl.simplified.accounts.api.AccountID import org.nypl.simplified.feeds.api.FeedBooksSelection import org.nypl.simplified.feeds.api.FeedBooksSelection.BOOKS_FEED_LOANED import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.Sorting.SortBy +import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.FilteringForFeed.FilterBy import org.nypl.simplified.feeds.api.FeedFacetPseudoTitleProviderType import java.net.URI @@ -38,6 +39,12 @@ data class ProfileFeedRequest( val title: String, + /** + * The current local feed facet. + */ + + val filterBy: FilterBy = FilterBy.FILTER_BY_LOANS, + /** * The active sorting facet. */ diff --git a/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/AccountDetailEvent.kt b/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/AccountDetailEvent.kt index 2071c5616..af564c021 100644 --- a/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/AccountDetailEvent.kt +++ b/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/AccountDetailEvent.kt @@ -75,9 +75,4 @@ sealed class AccountDetailEvent { * Open the dependents view. */ object OpenDependentInvite : AccountDetailEvent() - - /** - * FIXFIX Open selectedfragment - */ - object OpenSelectedFragment : AccountDetailEvent() } diff --git a/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/ekirjasto/EKirjastoAccountFragment.kt b/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/ekirjasto/EKirjastoAccountFragment.kt index 09bc93220..6b12427e7 100644 --- a/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/ekirjasto/EKirjastoAccountFragment.kt +++ b/simplified-ui-accounts/src/main/java/org/nypl/simplified/ui/accounts/ekirjasto/EKirjastoAccountFragment.kt @@ -80,9 +80,6 @@ class EKirjastoAccountFragment : Fragment(R.layout.account_ekirjasto){ private lateinit var bookmarkStatement: TextView private lateinit var buttonPreferences: Button - //FIXFIX - private lateinit var buttonSelected: Button - //inherited elements private lateinit var toolbar: PalaceToolbar @@ -122,7 +119,6 @@ class EKirjastoAccountFragment : Fragment(R.layout.account_ekirjasto){ this.versionText = view.findViewById(R.id.appVersion) this.bookmarkSyncCheck = view.findViewById(R.id.accountSyncBookmarksCheck) this.buttonPreferences = view.findViewById(R.id.buttonPreferences) - this.buttonSelected = view.findViewById(R.id.buttonSelected) this.toolbar = @@ -204,16 +200,6 @@ class EKirjastoAccountFragment : Fragment(R.layout.account_ekirjasto){ } } - private fun configureSelectedButton(button: Button) { - //Clicking the button opens the dependents fragment - button.setOnClickListener { - logger.debug("Selected clicked") - this.listener.post( - AccountDetailEvent.OpenSelectedFragment - ) - } - } - private fun configureDocViewButton(button: Button, document: DocumentType?){ button.isEnabled = document != null if (document != null) { @@ -281,8 +267,6 @@ class EKirjastoAccountFragment : Fragment(R.layout.account_ekirjasto){ */ configureDependentsButton(buttonDependents) - configureSelectedButton(buttonSelected) - //Set version text that's shown on the bottom of the page val versionString = this.requireContext().getString(R.string.app_version_string, this.viewModel.appVersion) this.versionText.text = versionString diff --git a/simplified-ui-accounts/src/main/res/layout/account_ekirjasto.xml b/simplified-ui-accounts/src/main/res/layout/account_ekirjasto.xml index be4c601c3..d407e5211 100644 --- a/simplified-ui-accounts/src/main/res/layout/account_ekirjasto.xml +++ b/simplified-ui-accounts/src/main/res/layout/account_ekirjasto.xml @@ -70,15 +70,6 @@ android:layout_height="wrap_content" android:text="@string/invite_a_dependent" /> - - { + CatalogFeedArguments.CatalogFeedArgumentsRemote( + feedURI = uri, + isSearchResults = false, + ownership = CatalogFeedOwnership.OwnedByAccount(accountID), + title = "" + ) + } } } diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedArguments.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedArguments.kt index 8ead95f9c..85d643fd5 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedArguments.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedArguments.kt @@ -3,6 +3,7 @@ package org.librarysimplified.ui.catalog import org.nypl.simplified.accounts.api.AccountID import org.nypl.simplified.feeds.api.FeedBooksSelection import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.Sorting.SortBy +import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.FilteringForFeed.FilterBy import java.io.Serializable import java.net.URI @@ -67,4 +68,21 @@ sealed class CatalogFeedArguments : Serializable { override val isSearchResults: Boolean = false override val isLocallyGenerated: Boolean = true } + + /** + * Arguments that define a multiple feed view. + */ + data class CatalogFeedArgumentsAllLocalBooks( + override val title: String, + override val ownership: CatalogFeedOwnership, + val sortBy: SortBy = SortBy.SORT_BY_TITLE, + val searchTerms: String?, + val filterBy: FilterBy = FilterBy.FILTER_BY_LOANS, + val selection: FeedBooksSelection, + val filterAccount: AccountID?, + val updateHolds: Boolean + ) : CatalogFeedArguments() { + override val isSearchResults: Boolean = false + override val isLocallyGenerated: Boolean = true + } } diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt index ce8191077..684d59bee 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt @@ -291,6 +291,13 @@ class CatalogFeedFragment : Fragment(R.layout.feed), AgeGateDialog.BirthYearSele } ) } + //If user is viewing the multiple feed view, set the message to always be the same + //No matter the view + if (parameters is CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks) { + this.feedEmptyMessage.setText( + R.string.emptyBooksNotLoggedIn + ) + } } //User is logged in, so we show text that informs about important things concerning loans and @@ -310,6 +317,20 @@ class CatalogFeedFragment : Fragment(R.layout.feed), AgeGateDialog.BirthYearSele } ) } + //Set the feed empty message for a multifeed view, show special message for selected + if (parameters is CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks) { + this.feedEmptyMessage.setText( + //Add a special text to selected, otherwise show loans text + if ( + (parameters as CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks).selection == + FeedBooksSelection.BOOKS_FEED_SELECTED + ) { + R.string.feedWithGroupsEmptySelected + } else { + R.string.feedWithGroupsEmptyLoaned + } + ) + } } private fun openLogoLink() { @@ -357,6 +378,9 @@ class CatalogFeedFragment : Fragment(R.layout.feed), AgeGateDialog.BirthYearSele is CatalogFeedArguments.CatalogFeedArgumentsLocalBooks -> { (parameters as CatalogFeedArguments.CatalogFeedArgumentsLocalBooks).searchTerms.orEmpty() } + is CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks -> { + (parameters as CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks).searchTerms.orEmpty() + } is CatalogFeedArguments.CatalogFeedArgumentsRemote -> { val uri = Uri.parse( @@ -462,6 +486,15 @@ class CatalogFeedFragment : Fragment(R.layout.feed), AgeGateDialog.BirthYearSele this.feedLoading.visibility = View.INVISIBLE this.feedNavigation.visibility = View.INVISIBLE + //If we are viewing multiple feed view, we want to show the top facets even if + //The feed is empty + if (feedState.arguments is CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks) { + if (feedState.facetsByGroup != null) { + // If there are some top level facets,configure them so they are shown on an empty view + this.configureFacetTabs(FeedFacets.findEntryPointFacetGroup(feedState.facetsByGroup), feedContentTabs) + feedContentHeader.visibility = View.VISIBLE + } + } this.configureLogoHeader(feedState) this.configureToolbar() } diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedState.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedState.kt index 791549eee..3aef18ad7 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedState.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedState.kt @@ -113,6 +113,7 @@ sealed class CatalogFeedState { data class CatalogFeedEmpty( override val arguments: CatalogFeedArguments, + val facetsByGroup: Map>? = null, override val search: FeedSearch?, override val title: String ) : CatalogFeedLoaded() diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt index 1c0b6959d..c21edb12c 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt @@ -15,6 +15,7 @@ import org.joda.time.DateTime import org.joda.time.LocalDateTime import org.librarysimplified.mdc.MDCKeys import org.librarysimplified.ui.catalog.CatalogFeedArguments.CatalogFeedArgumentsLocalBooks +import org.librarysimplified.ui.catalog.CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks import org.librarysimplified.ui.catalog.CatalogFeedArguments.CatalogFeedArgumentsRemote import org.librarysimplified.ui.catalog.CatalogFeedState.CatalogFeedLoaded import org.librarysimplified.ui.catalog.CatalogFeedState.CatalogFeedLoaded.CatalogFeedEmpty @@ -46,6 +47,7 @@ import org.nypl.simplified.feeds.api.FeedEntry import org.nypl.simplified.feeds.api.FeedFacet import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.FilteringForAccount +import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.FilteringForFeed import org.nypl.simplified.feeds.api.FeedFacet.FeedFacetPseudo.Sorting import org.nypl.simplified.feeds.api.FeedFacetPseudoTitleProviderType import org.nypl.simplified.feeds.api.FeedLoaderResult @@ -265,12 +267,11 @@ class CatalogFeedViewModel( if (event is BookStatusEvent.BookStatusEventRemoved && this.state.arguments.isLocallyGenerated) { this.reloadFeed() } else { - when (event.statusNow) { + when (val status = event.statusNow) { is BookStatus.Held, is BookStatus.Loaned, is BookStatus.Revoked -> { if (this.state.arguments.isLocallyGenerated) { - //Selecting and unselecting can trigger this for a book that is not on loan this.reloadFeed() } } @@ -286,8 +287,22 @@ class CatalogFeedViewModel( is BookStatus.RequestingDownload, is BookStatus.RequestingLoan, is BookStatus.RequestingRevoke, - is BookStatus.Selected, - is BookStatus.Unselected, + is BookStatus.Selected -> { + //No clue why this needs the status check, but it does, as otherwise it keeps + //triggering this reload every chance it gets + if (this.state.arguments.isLocallyGenerated && status is BookStatus.Selected) { + //Reload the feeds when book selected or unselected so + //What the user sees in the favorites feed is up to date + this.reloadFeed() + } + } + is BookStatus.Unselected -> { + if (this.state.arguments.isLocallyGenerated) { + //Reload the feeds when book selected or unselected so + //What the user sees in the favorites feed is up to date + this.reloadFeed() + } + } null -> { // do nothing } @@ -335,25 +350,30 @@ class CatalogFeedViewModel( fun syncAccounts() { when (val arguments = state.arguments) { is CatalogFeedArgumentsLocalBooks -> { - this.syncAccounts(arguments) + this.syncAccounts(arguments.filterAccount) } is CatalogFeedArgumentsRemote -> { } + is CatalogFeedArgumentsAllLocalBooks -> + this.syncAccounts(arguments.filterAccount) } } - private fun syncAccounts(arguments: CatalogFeedArgumentsLocalBooks) { + /** + * Sync the books in the book register, based on possible accountID. + */ + private fun syncAccounts(filterAccountID: AccountID?) { val profile = this.profilesController.profileCurrent() val accountsToSync = - if (arguments.filterAccount == null) { + if (filterAccountID == null) { // Sync all accounts this.logger.debug("[{}]: syncing all accounts", this.instanceId) profile.accounts() } else { // Sync the account we're filtering on - this.logger.debug("[{}]: syncing account {}", this.instanceId, arguments.filterAccount) - profile.accounts().filterKeys { it == arguments.filterAccount } + this.logger.debug("[{}]: syncing account {}", this.instanceId, filterAccountID) + profile.accounts().filterKeys { it == filterAccountID } } for (account in accountsToSync.keys) { @@ -375,8 +395,67 @@ class CatalogFeedViewModel( this.doLoadRemoteFeed(arguments) is CatalogFeedArgumentsLocalBooks -> this.doLoadLocalFeed(arguments) + is CatalogFeedArgumentsAllLocalBooks -> + this.doLoadLocalCombinationFeed(arguments) } } + /** + * Load a locally-generated feed that has multiple feeds combined. + */ + private fun doLoadLocalCombinationFeed( + arguments: CatalogFeedArgumentsAllLocalBooks + ) { + this.logger.debug("[{}]: loading local feed {}", this.instanceId, arguments.filterBy.name) + + MDC.remove(MDCKeys.FEED_URI) + MDC.remove(MDCKeys.ACCOUNT_INTERNAL_ID) + MDC.remove(MDCKeys.ACCOUNT_PROVIDER_ID) + MDC.remove(MDCKeys.ACCOUNT_PROVIDER_NAME) + + val booksUri = URI.create("Books") + val request = + ProfileFeedRequest( + facetTitleProvider = CatalogFacetPseudoTitleProvider(this.resources), + feedSelection = arguments.selection, + filterByAccountID = arguments.filterAccount, + filterBy = arguments.filterBy, + search = arguments.searchTerms, + sortBy = arguments.sortBy, + title = arguments.title, + uri = booksUri + ) + + val future = this.profilesController.profileFeed(request) + .map { feed -> + if (arguments.filterBy == FilteringForFeed.FilterBy.FILTER_BY_LOANS) { + feed.entriesInOrder.removeAll { feedEntry -> + feedEntry is FeedEntry.FeedEntryOPDS && + feedEntry.feedEntry.availability is OPDSAvailabilityLoaned && + feedEntry.feedEntry.availability.endDate.getOrNull()?.isBeforeNow == true + } + } + if (arguments.updateHolds) { + bookRegistry.updateHolds( + numberOfHolds = feed.entriesInOrder.filter { feedEntry -> + feedEntry is FeedEntry.FeedEntryOPDS && + feedEntry.feedEntry.availability is OPDSAvailabilityHeldReady + }.size + ) + } + FeedLoaderResult.FeedLoaderSuccess( + feed = feed, + accessToken = null + ) as FeedLoaderResult + } + .onAnyError { ex -> FeedLoaderResult.wrapException(booksUri, ex) } + + this.createNewStatus( + account = null, + arguments = arguments, + future = future + ) + } + /** * Load a locally-generated feed. @@ -632,7 +711,8 @@ class CatalogFeedViewModel( return CatalogFeedEmpty( arguments = arguments, search = feed.feedSearch, - title = feed.feedTitle + title = feed.feedTitle, + facetsByGroup = feed.facetsByGroup ) } @@ -689,6 +769,14 @@ class CatalogFeedViewModel( get() = this.resources.getString(R.string.feedShowAll) override val showOnLoan: String get() = this.resources.getString(R.string.feedShowOnLoan) + override val showTabSelected: String + get() = this.resources.getString(R.string.feedFacetSelected) + + override val showTabLoans: String + get() = this.resources.getString(R.string.feedFacetLoans) + + override val showTabHolds: String + get() = this.resources.getString(R.string.feedFacetHolds) } val accountProvider: AccountProviderType? @@ -874,6 +962,44 @@ class CatalogFeedViewModel( } } } + + is CatalogFeedArgumentsAllLocalBooks -> { + when (search) { + is FeedSearch.FeedSearchLocal -> { + //Get the tab we are in from the stored values + //Otherwise search and filter always reset to loans + val oldValues = state.arguments as CatalogFeedArgumentsAllLocalBooks + + return CatalogFeedArgumentsAllLocalBooks( + filterAccount = currentArguments.filterAccount, + ownership = currentArguments.ownership, + searchTerms = query, + selection = oldValues.selection, + sortBy = currentArguments.sortBy, + filterBy = oldValues.filterBy, + title = currentArguments.title, + updateHolds = currentArguments.updateHolds + ) + } + is FeedSearch.FeedSearchOpen1_1 -> { + //Get the tab we are in from the stored values + //Otherwise search and filter always reset to loans + val oldValues = state.arguments as CatalogFeedArgumentsAllLocalBooks + + return CatalogFeedArgumentsAllLocalBooks( + filterAccount = currentArguments.filterAccount, + ownership = currentArguments.ownership, + searchTerms = query, + selection = oldValues.selection, + sortBy = currentArguments.sortBy, + filterBy = oldValues.filterBy, + title = currentArguments.title, + updateHolds = currentArguments.updateHolds + ) + } + } + + } } } @@ -912,6 +1038,11 @@ class CatalogFeedViewModel( "Can't transition local to remote feed: ${this.feedArguments.title} -> $title" ) } + + is CatalogFeedArgumentsAllLocalBooks -> + throw IllegalStateException( + "Can't transition local to remote feed: ${this.feedArguments.title} -> $title" + ) } } @@ -972,6 +1103,72 @@ class CatalogFeedViewModel( title = facet.title, updateHolds = currentArguments.updateHolds ) + + is FilteringForFeed -> + CatalogFeedArgumentsAllLocalBooks( + title = facet.title, + ownership = currentArguments.ownership, + sortBy = currentArguments.sortBy, + searchTerms = currentArguments.searchTerms, + selection = facet.selectedFeed, + filterBy = facet.filterBy, + filterAccount = currentArguments.filterAccount, + updateHolds = currentArguments.updateHolds + ) + } + } + + is CatalogFeedArgumentsAllLocalBooks -> { + when (facet) { + is FeedFacet.FeedFacetOPDS -> + throw IllegalStateException("Cannot transition from a local feed to a remote feed.") + is FilteringForAccount -> { + //Get the tab we are in from the stored values + //Otherwise search and filter always reset to loans + val oldValues = state.arguments as CatalogFeedArgumentsAllLocalBooks + + return CatalogFeedArgumentsAllLocalBooks( + title = currentArguments.title, + ownership = currentArguments.ownership, + sortBy = currentArguments.sortBy, + searchTerms = currentArguments.searchTerms, + selection = oldValues.selection, + filterBy = oldValues.filterBy, + filterAccount = facet.account, + updateHolds = currentArguments.updateHolds + ) + } + is FilteringForFeed -> { + // From the old state, use the information of which selection and filter we are currently in + //Otherwise they will reset to the assumed value of loans + val oldValues = state.arguments as CatalogFeedArgumentsAllLocalBooks + + return CatalogFeedArgumentsAllLocalBooks( + title = facet.title, + ownership = currentArguments.ownership, + sortBy = oldValues.sortBy, + searchTerms = currentArguments.searchTerms, + selection = facet.selectedFeed, + filterBy = facet.filterBy, + filterAccount = currentArguments.filterAccount, + updateHolds = currentArguments.updateHolds + ) + } + is Sorting -> { + // From the old state, use the information of which selection and filter we are currently in + //Otherwise they will reset to the assumed value of loans + val oldValues = state.arguments as CatalogFeedArgumentsAllLocalBooks + return CatalogFeedArgumentsAllLocalBooks( + title = facet.title, + ownership = currentArguments.ownership, + sortBy = facet.sortBy, + searchTerms = currentArguments.searchTerms, + selection = oldValues.selection, + filterBy = oldValues.filterBy, + filterAccount = currentArguments.filterAccount, + updateHolds = currentArguments.updateHolds + ) + } } } } diff --git a/simplified-ui-catalog/src/main/res/drawable/baseline_check_circle_24.xml b/simplified-ui-catalog/src/main/res/drawable/baseline_check_circle_24.xml index 9a125bcdb..6a53520c4 100644 --- a/simplified-ui-catalog/src/main/res/drawable/baseline_check_circle_24.xml +++ b/simplified-ui-catalog/src/main/res/drawable/baseline_check_circle_24.xml @@ -1,5 +1,10 @@ - - - + + + diff --git a/simplified-ui-catalog/src/main/res/values/stringsFeed.xml b/simplified-ui-catalog/src/main/res/values/stringsFeed.xml index 453e50a60..493777449 100644 --- a/simplified-ui-catalog/src/main/res/values/stringsFeed.xml +++ b/simplified-ui-catalog/src/main/res/values/stringsFeed.xml @@ -18,10 +18,15 @@ Books Browse Books Holds + Favorites + Loans + Holds When you reserve a book from the catalog, it will show up here. Look here from time to time to see if your book is available to download.\n Number of reservations is limited to 5. Visit the Catalog to add books to My Books.\n Number of loans is limited to 5.\n Ensure good internet connection when downloading a book and make sure there is enough space on your device. + Visit the Catalog to add books to Favorites. Sign in to see your reservations. Sign in to see your books. + Sign in to see your books. Please enter your birth year Select Year Age Verification diff --git a/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt b/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt index 7863559f6..b6c463f76 100644 --- a/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt +++ b/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt @@ -66,7 +66,7 @@ object BottomNavigators { ) }, R.id.tabBooks to { - createBooksFragment( + createCombinationFragment( context = context, id = R.id.tabBooks, profilesController = profilesController, @@ -229,6 +229,9 @@ object BottomNavigators { ) } + /** + * Create a fragment for showing the loaned books. + */ private fun createBooksFragment( context: Context, id: Int, @@ -269,6 +272,89 @@ object BottomNavigators { ) } + /** + * Create a fragment for showing the selected books. + */ + private fun createSelectedFragment( + context: Context, + id: Int, + profilesController: ProfilesControllerType, + settingsConfiguration: BuildConfigurationServiceType, + defaultProvider: AccountProviderType + ): Fragment { + logger.debug("[{}]: creating selected fragment", id) + + //Choose the account we want to use, in our case, currently always null + val filterAccountId = + if (settingsConfiguration.showBooksFromAllAccounts) { + null + } else { + pickDefaultAccount(profilesController, defaultProvider).id + } + + //Choose the feed owner, currently we always collect from all accounts + val ownership = + if (filterAccountId == null) { + CatalogFeedOwnership.CollectedFromAccounts + } else { + CatalogFeedOwnership.OwnedByAccount(filterAccountId) + } + + //Create the fragment hacing the selection be the selected + return CatalogFeedFragment.create( + CatalogFeedArguments.CatalogFeedArgumentsLocalBooks( + filterAccount = filterAccountId, + ownership = ownership, + searchTerms = null, + selection = FeedBooksSelection.BOOKS_FEED_SELECTED, + sortBy = FeedFacet.FeedFacetPseudo.Sorting.SortBy.SORT_BY_TITLE, + title = "Favorites", + updateHolds = false + ) + ) + } + + /** + * Create a new fragment from the bookRegister with the option to switch between + * multiple different views, like loans and selected books. + */ + private fun createCombinationFragment( + context: Context, + id: Int, + profilesController: ProfilesControllerType, + settingsConfiguration: BuildConfigurationServiceType, + defaultProvider: AccountProviderType + ): Fragment { + logger.debug("[{}]: creating combination fragment", id) + + //Check if should show all books in the registry or the books of one account + val filterAccountId = + if (settingsConfiguration.showBooksFromAllAccounts) { + null + } else { + pickDefaultAccount(profilesController, defaultProvider).id + } + // Choose the owner, should be the ID of the account currently logged in + val ownership = + if (filterAccountId == null) { + CatalogFeedOwnership.CollectedFromAccounts + } else { + CatalogFeedOwnership.OwnedByAccount(filterAccountId) + } + //Create the fragment, enter from the loans + return CatalogFeedFragment.create( + CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks( + filterAccount = filterAccountId, + ownership = ownership, + searchTerms = null, + sortBy = FeedFacet.FeedFacetPseudo.Sorting.SortBy.SORT_BY_TITLE, + filterBy = FeedFacet.FeedFacetPseudo.FilteringForFeed.FilterBy.FILTER_BY_LOANS, + selection = FeedBooksSelection.BOOKS_FEED_LOANED, + title = context.getString(R.string.tabBooks), + updateHolds = false + ) + ) + } private fun createCatalogFragment( id: Int, feedArguments: CatalogFeedArguments.CatalogFeedArgumentsRemote From e16fb2965f73f788b8b82e7b15cb26904541060e Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Wed, 5 Feb 2025 15:35:20 +0200 Subject: [PATCH 07/23] Remove useless log --- .../librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt index c7cc61632..22d157a48 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt @@ -107,7 +107,6 @@ class CatalogBookDetailViewModel( mutableMapOf() private fun onBookStatusEvent(event: BookStatusEvent) { - logger.debug("ALALALALALLALALALALALALALALLA") val bookWithStatus = this.createBookWithStatus() this.bookWithStatusMutable.value = Pair(bookWithStatus, bookWithStatusMutable.value!!.second) } From 84eae2c4b9ddb821b35a1c239fb8dcafd807b386 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Wed, 5 Feb 2025 16:08:00 +0200 Subject: [PATCH 08/23] Refractor BookSync because of repetetive code --- .../books/controller/BookSyncTask.kt | 107 ++++++++---------- 1 file changed, 46 insertions(+), 61 deletions(-) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt index efbf20db6..10be58814 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt @@ -92,74 +92,58 @@ class BookSyncTask( credentials = credentials ) - //The URI to use for getting the loans - val loansURI = provider.loansURI - if (loansURI == null) { - this.logger.debug("no loans URI, aborting!") - return this.taskRecorder.finishSuccess(Unit) - } - //Loan request - val request = - this.http.newRequest(loansURI) - .setAuthorization(AccountAuthenticatedHTTP.createAuthorization(credentials)) - .addCredentialsToProperties(credentials) - .build() - - //Initialize loans stream - var loansStream : InputStream = ByteArrayInputStream(ByteArray(0)) - - //Execute loan fetch - val response = request.execute() - when (val status = response.status) { - is LSHTTPResponseStatus.Responded.OK -> { - // Save stream for later use - loansStream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)) - } - is LSHTTPResponseStatus.Responded.Error -> { - val recovered = this.onHTTPError(status, account) - - if (recovered) { - this.taskRecorder.finishSuccess(Unit) - } else { - val message = String.format("%s: %d: %s", provider.loansURI, status.properties.status, status.properties.message) - val exception = IOException(message) - this.taskRecorder.currentStepFailed( - message = message, - errorCode = "syncFailed", - exception = exception - ) - throw TaskFailedHandled(exception) - } - } - is LSHTTPResponseStatus.Failed -> - throw IOException(status.exception) + //Get the loans stream + val loansStream: InputStream? = fetchFeed( + provider.loansURI, + credentials, + account + ) + //Get the selected stream + val selectedStream: InputStream? = fetchFeed( + provider.selectedURI, + credentials, + account + ) + //If both fetches went fine, we combine the streams + //And update database and registry + if (loansStream != null && selectedStream != null) { + this.onHTTPOKMultipleFeeds( + loansStream = loansStream, + selectedStream = selectedStream, + provider = provider, + account = account + ) } + return this.taskRecorder.finishSuccess(Unit) + } - // Do the same for selected feed - val selectedURI = provider.selectedURI - if (selectedURI == null) { - this.logger.debug("no selected URI, aborting!") - return this.taskRecorder.finishSuccess(Unit) + /** + * Fetch a feed from the URI provided. + */ + private fun fetchFeed( + uri: URI?, + credentials: AccountAuthenticationCredentials, + account: AccountType + ) : InputStream? { + if (uri == null) { + this.logger.debug("no fetch URI, aborting!") + this.taskRecorder.finishSuccess(Unit) + return null } - //Select request - val selectRequest = - this.http.newRequest(selectedURI) + //Create the request + val feedRequest = + this.http.newRequest(uri) .setAuthorization(AccountAuthenticatedHTTP.createAuthorization(credentials)) .addCredentialsToProperties(credentials) .build() - //Execute selected fetch - val selectedResponse = selectRequest.execute() + //Execute the fetch + val selectedResponse = feedRequest.execute() return when (val status = selectedResponse.status) { is LSHTTPResponseStatus.Responded.OK -> { - //Handle both the loans and selected streams so the books are added into the database - this.onHTTPOKMultipleFeeds( - loansStream = loansStream, - selectedStream = status.bodyStream ?: ByteArrayInputStream(ByteArray(0)), - provider = provider, - account = account - ) - this.taskRecorder.finishSuccess(Unit) + //If answer is okay + //Return the response stream + status.bodyStream ?: ByteArrayInputStream(ByteArray(0)) } is LSHTTPResponseStatus.Responded.Error -> { val recovered = this.onHTTPError(status, account) @@ -167,7 +151,7 @@ class BookSyncTask( if (recovered) { this.taskRecorder.finishSuccess(Unit) } else { - val message = String.format("%s: %d: %s", provider.selectedURI, status.properties.status, status.properties.message) + val message = String.format("%s: %d: %s", uri, status.properties.status, status.properties.message) val exception = IOException(message) this.taskRecorder.currentStepFailed( message = message, @@ -176,6 +160,7 @@ class BookSyncTask( ) throw TaskFailedHandled(exception) } + null } is LSHTTPResponseStatus.Failed -> throw IOException(status.exception) @@ -470,7 +455,7 @@ class BookSyncTask( try { this.logger.debug("[{}] checking for deletion", existingId.brief()) - //If book not in loans or selected, delete it + //If book not in loans or selected, handle it accordingly if (!allBookIDs.contains(existingId)) { val dbEntry = bookDatabase.entry(existingId) val a = dbEntry.book.entry.availability From e82637d80d67afe61985867cc3cf9794cc0195f8 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Wed, 5 Feb 2025 16:10:57 +0200 Subject: [PATCH 09/23] Fix value naming --- .../java/org/nypl/simplified/books/controller/BookSyncTask.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt index 10be58814..f1ae44b68 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt @@ -138,8 +138,8 @@ class BookSyncTask( .build() //Execute the fetch - val selectedResponse = feedRequest.execute() - return when (val status = selectedResponse.status) { + val feedResponse = feedRequest.execute() + return when (val status = feedResponse.status) { is LSHTTPResponseStatus.Responded.OK -> { //If answer is okay //Return the response stream From b3104b574a21a89250aa8bc6d85e981eb8b74920 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Wed, 5 Feb 2025 16:30:36 +0200 Subject: [PATCH 10/23] Remove leftover logs and markers --- .../org/nypl/simplified/accounts/json/AccountProvidersJSON.kt | 2 +- .../src/main/java/org/nypl/simplified/feeds/api/FeedLoader.kt | 2 +- .../librarysimplified/main/MainFragmentListenerDelegate.kt | 4 ---- .../ui/catalog/CatalogBookDetailViewModel.kt | 1 - 4 files changed, 2 insertions(+), 7 deletions(-) diff --git a/simplified-accounts-json/src/main/java/org/nypl/simplified/accounts/json/AccountProvidersJSON.kt b/simplified-accounts-json/src/main/java/org/nypl/simplified/accounts/json/AccountProvidersJSON.kt index 63377048a..a347df18d 100644 --- a/simplified-accounts-json/src/main/java/org/nypl/simplified/accounts/json/AccountProvidersJSON.kt +++ b/simplified-accounts-json/src/main/java/org/nypl/simplified/accounts/json/AccountProvidersJSON.kt @@ -277,7 +277,7 @@ object AccountProvidersJSON { val loansURI = JSONParserUtilities.getURIOrNull(obj, "loansURI") val selectedURI = - JSONParserUtilities.getURIOrNull(obj, "loansURI")//Wrong URI + JSONParserUtilities.getURIOrNull(obj, "selectedURI") val patronSettingsURI = JSONParserUtilities.getURIOrNull(obj, "patronSettingsURI") val privacyPolicy = diff --git a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedLoader.kt b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedLoader.kt index 31c152e71..25bc60c20 100644 --- a/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedLoader.kt +++ b/simplified-feeds-api/src/main/java/org/nypl/simplified/feeds/api/FeedLoader.kt @@ -186,7 +186,7 @@ class FeedLoader private constructor( accountId: AccountID, uri: URI ): FeedLoaderResult { - val streamMaybe = this.contentResolver.openInputStream(uri)// + val streamMaybe = this.contentResolver.openInputStream(uri) return if (streamMaybe != null) { streamMaybe.use { stream -> FeedLoaderSuccess( diff --git a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt index f9d09991b..d0cb6a01a 100644 --- a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt +++ b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt @@ -1,9 +1,7 @@ package org.librarysimplified.main -import android.net.Uri import androidx.activity.addCallback import androidx.appcompat.app.AppCompatActivity -import androidx.core.net.toUri import androidx.fragment.app.Fragment import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleObserver @@ -16,7 +14,6 @@ import org.librarysimplified.ui.catalog.CatalogBookDetailFragmentParameters import org.librarysimplified.ui.catalog.CatalogFeedArguments import org.librarysimplified.ui.catalog.CatalogFeedEvent import org.librarysimplified.ui.catalog.CatalogFeedFragment -import org.librarysimplified.ui.catalog.CatalogFeedOwnership import org.librarysimplified.ui.catalog.saml20.CatalogSAML20Event import org.librarysimplified.ui.login.LoginMainFragment import org.librarysimplified.ui.navigation.tabs.TabbedNavigator @@ -63,7 +60,6 @@ import org.nypl.simplified.ui.settings.SettingsMainEvent import org.nypl.simplified.viewer.api.Viewers import org.nypl.simplified.viewer.spi.ViewerPreferences import org.slf4j.LoggerFactory -import java.net.URI import java.net.URL internal class MainFragmentListenerDelegate( diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt index 22d157a48..0f1ad3ead 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt @@ -261,7 +261,6 @@ class CatalogBookDetailViewModel( feedEntry: FeedEntry.FeedEntryOPDS, callback: (BookWithStatus) -> Unit ) { - logger.debug("Observer registered") this.bookModels.getOrPut(feedEntry.bookID, { BookModel(feedEntry) }).onBookChanged.add(callback) this.notifyBookStatus(feedEntry, callback) } From 8ae48a5bf08de7d2e37134ea1270926112d03c23 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Wed, 5 Feb 2025 17:31:18 +0200 Subject: [PATCH 11/23] Minor fix --- .../java/org/nypl/simplified/books/controller/BookSyncTask.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt index f1ae44b68..b0937d234 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt @@ -375,7 +375,7 @@ class BookSyncTask( val selectedMap = selectedFeed.feedEntries.associateBy { BookIDs.newFromOPDSEntry(it) } //Get all non-duplicate IDs - val allBookIDs = (loansMap.keys + selectedMap.keys).toSet() + val allBookIDs = (loansMap.keys + selectedMap.keys) //Initiate the list for the combined feed entries val combinedFeedEntries = mutableListOf() From 9af56e5f1184a924d6a38b2a5b82103618ba5ca3 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Thu, 6 Feb 2025 12:45:07 +0200 Subject: [PATCH 12/23] Add selectedURI to tests and mockProfiles --- .../source/nyplregistry/AccountProviderResolution.kt | 3 ++- .../java/org/nypl/simplified/tests/TransformProviders.kt | 1 + .../AccountProviderSourceNYPLRegistryDescriptionTest.kt | 9 ++++++--- .../tests/books/controller/ProfilesControllerContract.kt | 6 ++++++ .../simplified/tests/mocking/MockAccessibilityStrings.kt | 2 ++ .../org/nypl/simplified/tests/mocking/MockAccount.kt | 3 ++- .../simplified/tests/mocking/MockAccountProviders.kt | 3 ++- 7 files changed, 21 insertions(+), 6 deletions(-) diff --git a/simplified-accounts-source-nyplregistry/src/main/java/org/nypl/simplified/accounts/source/nyplregistry/AccountProviderResolution.kt b/simplified-accounts-source-nyplregistry/src/main/java/org/nypl/simplified/accounts/source/nyplregistry/AccountProviderResolution.kt index b529f0236..2f68bf7c1 100644 --- a/simplified-accounts-source-nyplregistry/src/main/java/org/nypl/simplified/accounts/source/nyplregistry/AccountProviderResolution.kt +++ b/simplified-accounts-source-nyplregistry/src/main/java/org/nypl/simplified/accounts/source/nyplregistry/AccountProviderResolution.kt @@ -135,7 +135,8 @@ class AccountProviderResolution( supportsReservations = supportsReservations, updated = updated, location = this.description.location, - alternateURI = alternateURI + alternateURI = alternateURI, + selectedURI = authDocument?.selectedURI ) taskRecorder.finishSuccess(accountProvider) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/TransformProviders.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/TransformProviders.kt index c2b29b297..bbd2447d2 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/TransformProviders.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/TransformProviders.kt @@ -82,6 +82,7 @@ class TransformProviders { updated = DateTime.parse(entry.updated), location = null, alternateURI = null, + selectedURI = null ) providers.add(provider) } diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt index dc3f5f231..2a1abd6f1 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt @@ -433,7 +433,8 @@ class AccountProviderSourceNYPLRegistryDescriptionTest { supportsReservations = true, updated = DateTime.parse("1970-01-01T00:00:00.000Z"), location = null, - alternateURI = URI.create("https://www.example.com/alternate") + alternateURI = URI.create("https://www.example.com/alternate"), + selectedURI = null ) Assertions.assertEquals(provider, result.result) @@ -596,7 +597,8 @@ class AccountProviderSourceNYPLRegistryDescriptionTest { supportsReservations = true, updated = DateTime.parse("1970-01-01T00:00:00.000Z"), location = null, - alternateURI = URI.create("https://www.example.com/alternate") + alternateURI = URI.create("https://www.example.com/alternate"), + selectedURI = null ) Assertions.assertEquals(provider, result.result) @@ -739,7 +741,8 @@ class AccountProviderSourceNYPLRegistryDescriptionTest { supportsReservations = true, updated = DateTime.parse("1970-01-01T00:00:00.000Z"), location = null, - alternateURI = URI.create("https://www.example.com/alternate") + alternateURI = URI.create("https://www.example.com/alternate"), + selectedURI = null ) Assertions.assertEquals(provider, result.result) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/ProfilesControllerContract.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/ProfilesControllerContract.kt index c5fb76b09..6a31be1b1 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/ProfilesControllerContract.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/ProfilesControllerContract.kt @@ -471,6 +471,12 @@ abstract class ProfilesControllerContract { get() = "All" override val showOnLoan: String get() = "On Loan" + override val showTabSelected: String + get() = "Selected" + override val showTabLoans: String + get() = "Loans" + override val showTabHolds: String + get() = "Holds" } ) ).get() diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccessibilityStrings.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccessibilityStrings.kt index f73961561..b3016c404 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccessibilityStrings.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccessibilityStrings.kt @@ -11,4 +11,6 @@ class MockAccessibilityStrings : AccessibilityStringsType { override fun bookFailedLoan(title: String): String = "bookFailedLoan $title" override fun bookFailedDownload(title: String): String = "bookFailedDownload $title" override fun bookLoanLimitReached(): String = "bookLoanLimitReached" + override fun bookSelected(title: String): String = "bookSelected $title" + override fun bookUnselected(title: String): String = "bookUnselected $title" } diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccount.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccount.kt index 522be02e3..f2282cc23 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccount.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccount.kt @@ -81,7 +81,8 @@ class MockAccount(override val id: AccountID) : AccountType { supportsReservations = false, updated = DateTime(), location = null, - alternateURI = URI.create("https://www.example.com/alternate") + alternateURI = URI.create("https://www.example.com/alternate"), + selectedURI = null ) } diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt index 1cc93b4d2..e321a8f25 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt @@ -47,7 +47,8 @@ object MockAccountProviders { supportsReservations = false, updated = DateTime.parse("2000-01-01T00:00:00Z"), location = null, - alternateURI = URI.create("https://www.example.com/alternate") + alternateURI = URI.create("https://www.example.com/alternate"), + selectedURI = null ) } From 66254a49c985d4d08a6192137beeda9f3b95eb50 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Fri, 7 Feb 2025 10:03:44 +0200 Subject: [PATCH 13/23] Attempt to fix tests to simulate syncBooks correct --- ...oviderSourceNYPLRegistryDescriptionTest.kt | 6 +-- .../controller/BooksControllerContract.kt | 48 +++++++++++++++++++ .../tests/mocking/MockAccountProviders.kt | 2 +- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt index 2a1abd6f1..6a5b07421 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt @@ -434,7 +434,7 @@ class AccountProviderSourceNYPLRegistryDescriptionTest { updated = DateTime.parse("1970-01-01T00:00:00.000Z"), location = null, alternateURI = URI.create("https://www.example.com/alternate"), - selectedURI = null + selectedURI = URI("http://www.example.com/shelf.xml") ) Assertions.assertEquals(provider, result.result) @@ -598,7 +598,7 @@ class AccountProviderSourceNYPLRegistryDescriptionTest { updated = DateTime.parse("1970-01-01T00:00:00.000Z"), location = null, alternateURI = URI.create("https://www.example.com/alternate"), - selectedURI = null + selectedURI = URI("http://www.example.com/shelf.xml") ) Assertions.assertEquals(provider, result.result) @@ -742,7 +742,7 @@ class AccountProviderSourceNYPLRegistryDescriptionTest { updated = DateTime.parse("1970-01-01T00:00:00.000Z"), location = null, alternateURI = URI.create("https://www.example.com/alternate"), - selectedURI = null + selectedURI = URI("http://www.example.com/shelf.xml") ) Assertions.assertEquals(provider, result.result) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/BooksControllerContract.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/BooksControllerContract.kt index 7aa9372b8..b442f54ad 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/BooksControllerContract.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/BooksControllerContract.kt @@ -560,12 +560,20 @@ abstract class BooksControllerContract { .setResponseCode(200) .setBody(this.simpleUserProfile()) ) + //For loans this.server.enqueue( MockResponse() .setResponseCode(200) .setBody("Unlikely!") ) + //For selected + this.server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody("Still unlikely!") + ) + val result = controller.booksSync(account.id).get() Assertions.assertTrue(result is TaskResult.Failure) Assertions.assertEquals(OPDSParseException::class.java, (result as TaskResult.Failure).exception!!.javaClass) @@ -612,6 +620,14 @@ abstract class BooksControllerContract { .setBody(this.simpleUserProfile()) ) + //Setup for loans + this.server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody(Buffer().readFrom(resource("testBooksSyncNewEntries.xml"))) + ) + + //Setup for selected this.server.enqueue( MockResponse() .setResponseCode(200) @@ -700,6 +716,14 @@ abstract class BooksControllerContract { * Populate the database by syncing against a feed that contains books. */ + //Loans + this.server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody(Buffer().readFrom(resource("testBooksSyncNewEntries.xml"))) + ) + + //Selected this.server.enqueue( MockResponse() .setResponseCode(200) @@ -729,11 +753,20 @@ abstract class BooksControllerContract { .setResponseCode(200) .setBody(this.simpleUserProfile()) ) + //Loans this.server.enqueue( MockResponse() .setResponseCode(200) .setBody(Buffer().readFrom(resource("testBooksSyncRemoveEntries.xml"))) ) + + //Selected + this.server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody(Buffer().readFrom(resource("testBooksSyncRemoveEntries.xml"))) + ) + this.server.enqueue( MockResponse() .setResponseCode(200) @@ -819,6 +852,14 @@ abstract class BooksControllerContract { .setBody(this.simpleUserProfile()) ) + //First setup for loans + this.server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody(Buffer().readFrom(resource("testBooksDelete.xml"))) + ) + + //First setup for selected this.server.enqueue( MockResponse() .setResponseCode(200) @@ -953,7 +994,14 @@ abstract class BooksControllerContract { .setResponseCode(200) .setBody(this.simpleUserProfile()) ) + //Setup for loans + this.server.enqueue( + MockResponse() + .setResponseCode(200) + .setBody(Buffer().readFrom(resource("testBooksSyncNewEntries.xml"))) + ) + //Setup for selected this.server.enqueue( MockResponse() .setResponseCode(200) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt index e321a8f25..04d257186 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt @@ -48,7 +48,7 @@ object MockAccountProviders { updated = DateTime.parse("2000-01-01T00:00:00Z"), location = null, alternateURI = URI.create("https://www.example.com/alternate"), - selectedURI = null + selectedURI = URI.create("http://$host:$port/accounts0/loans.xml") ) } From c32845c622374c9409b41eb1e8832902a036bcf7 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Fri, 7 Feb 2025 11:36:35 +0200 Subject: [PATCH 14/23] Replace selectedURI links with null The link being not null in some cases when it was expected to be null in some tests caused failures. --- .../AccountProviderSourceNYPLRegistryDescriptionTest.kt | 6 +++--- .../nypl/simplified/tests/mocking/MockAccountProviders.kt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt index 6a5b07421..2a1abd6f1 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/accounts/AccountProviderSourceNYPLRegistryDescriptionTest.kt @@ -434,7 +434,7 @@ class AccountProviderSourceNYPLRegistryDescriptionTest { updated = DateTime.parse("1970-01-01T00:00:00.000Z"), location = null, alternateURI = URI.create("https://www.example.com/alternate"), - selectedURI = URI("http://www.example.com/shelf.xml") + selectedURI = null ) Assertions.assertEquals(provider, result.result) @@ -598,7 +598,7 @@ class AccountProviderSourceNYPLRegistryDescriptionTest { updated = DateTime.parse("1970-01-01T00:00:00.000Z"), location = null, alternateURI = URI.create("https://www.example.com/alternate"), - selectedURI = URI("http://www.example.com/shelf.xml") + selectedURI = null ) Assertions.assertEquals(provider, result.result) @@ -742,7 +742,7 @@ class AccountProviderSourceNYPLRegistryDescriptionTest { updated = DateTime.parse("1970-01-01T00:00:00.000Z"), location = null, alternateURI = URI.create("https://www.example.com/alternate"), - selectedURI = URI("http://www.example.com/shelf.xml") + selectedURI = null ) Assertions.assertEquals(provider, result.result) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt index 04d257186..e321a8f25 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt @@ -48,7 +48,7 @@ object MockAccountProviders { updated = DateTime.parse("2000-01-01T00:00:00Z"), location = null, alternateURI = URI.create("https://www.example.com/alternate"), - selectedURI = URI.create("http://$host:$port/accounts0/loans.xml") + selectedURI = null ) } From 59cba8c43c2849570a0fef7f53e0e9c7d897f12a Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Fri, 7 Feb 2025 12:39:13 +0200 Subject: [PATCH 15/23] Add selectedURI to mock account Added a link that s a copy of loans link to the mock account, as otherwise tests that use syncBook and the mock account would get the noURI error. --- .../tests/http/refresh_token/sync/SyncBookRefreshToken.kt | 7 +++++++ .../nypl/simplified/tests/mocking/MockAccountProviders.kt | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/http/refresh_token/sync/SyncBookRefreshToken.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/http/refresh_token/sync/SyncBookRefreshToken.kt index a129b4c2b..53ab8a6a4 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/http/refresh_token/sync/SyncBookRefreshToken.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/http/refresh_token/sync/SyncBookRefreshToken.kt @@ -232,7 +232,14 @@ class SyncBookRefreshToken { .setHeader(LSHTTPRequestConstants.PROPERTY_KEY_ACCESS_TOKEN, "ghij") .setBody(this.simpleUserProfile()) ) + //Loans feed + this.webServer.enqueue( + MockResponse() + .setResponseCode(200) + .setBody(Buffer().readFrom(resource("testBooksSyncNewEntries.xml"))) + ) + //Selected feed this.webServer.enqueue( MockResponse() .setResponseCode(200) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt index e321a8f25..c94fe43d2 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/mocking/MockAccountProviders.kt @@ -48,7 +48,7 @@ object MockAccountProviders { updated = DateTime.parse("2000-01-01T00:00:00Z"), location = null, alternateURI = URI.create("https://www.example.com/alternate"), - selectedURI = null + selectedURI = URI.create("http://$host:$port/accounts0/loans.xml"), ) } From 950b53842fa3ba7f7260fb4dafbb3263487efa2a Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Fri, 7 Feb 2025 13:17:03 +0200 Subject: [PATCH 16/23] Add selected fetch to 401 test --- .../books/controller/BooksControllerContract.kt | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/BooksControllerContract.kt b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/BooksControllerContract.kt index b442f54ad..fdc77e064 100644 --- a/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/BooksControllerContract.kt +++ b/simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/BooksControllerContract.kt @@ -384,6 +384,14 @@ abstract class BooksControllerContract { .setBody(this.simpleUserProfile()) ) + //Loans response, should throw the error instantly without even reaching selected + this.server.enqueue( + MockResponse() + .setResponseCode(400) + .setBody("") + ) + + //Selected response, should not reach here, but also should throw error this.server.enqueue( MockResponse() .setResponseCode(400) @@ -433,7 +441,13 @@ abstract class BooksControllerContract { .setResponseCode(200) .setBody(this.simpleUserProfile()) ) - + //401 in loans + this.server.enqueue( + MockResponse() + .setResponseCode(401) + .setBody("") + ) + //401 in selected this.server.enqueue( MockResponse() .setResponseCode(401) From fa7c95feee951f307dd9bcc8fabac563e6778826 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Fri, 7 Feb 2025 16:04:46 +0200 Subject: [PATCH 17/23] Add the handling od 401 to selection functions Made select and unselect tasks extend the AbstractBookTask class, so that the 401 error can be caught and handled in the controller. If select or unselect trigger token refresh and it's successful, the view is reloaded, but the popup does still show, though it is delayed due to the login behaviour. SyncBook behaviour was also adjusted, as it was triggering the refresh twice in a row, causing the first one cto succeed and the next one fail, causing the user to be logged out unnecessarily. --- .../books/controller/BookDeleteTask.kt | 2 + .../books/controller/BookSelectTask.kt | 88 ++++++---- .../books/controller/BookSyncTask.kt | 28 +++- .../books/controller/BookUnselectTask.kt | 84 ++++++---- .../simplified/books/controller/Controller.kt | 152 ++++++++++++++---- 5 files changed, 256 insertions(+), 98 deletions(-) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookDeleteTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookDeleteTask.kt index eb5e8b282..acb318f9e 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookDeleteTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookDeleteTask.kt @@ -50,9 +50,11 @@ class BookDeleteTask( //If the book is still selected, don't delete the book, just update if (entry.book.entry.selected is Some) { + logger.debug("Book is selected, don't delete, just update") this.bookRegistry.update(BookWithStatus(entry.book, BookStatus.fromBook(entry.book))) } else { //Otherwise delete the db and registry entries + logger.debug("Book not selected delete from database and register") entry.delete() this.bookRegistry.clearFor(entry.book.id) } diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt index b1f09893b..727ea87dd 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSelectTask.kt @@ -6,6 +6,7 @@ import org.librarysimplified.http.api.LSHTTPRequestBuilderType import org.librarysimplified.http.api.LSHTTPResponseStatus import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP.addCredentialsToProperties +import org.nypl.simplified.accounts.api.AccountID import org.nypl.simplified.accounts.database.api.AccountType import org.nypl.simplified.books.api.Book import org.nypl.simplified.books.api.BookIDs @@ -16,31 +17,42 @@ import org.nypl.simplified.opds.core.OPDSAcquisitionFeedEntry import org.nypl.simplified.opds.core.OPDSAcquisitionFeedEntryParser import org.nypl.simplified.opds.core.OPDSParseException import org.nypl.simplified.opds.core.getOrNull +import org.nypl.simplified.profiles.api.ProfileID +import org.nypl.simplified.profiles.api.ProfilesDatabaseType import org.nypl.simplified.taskrecorder.api.TaskRecorder import org.nypl.simplified.taskrecorder.api.TaskRecorderType import org.nypl.simplified.taskrecorder.api.TaskResult import org.slf4j.LoggerFactory import java.io.ByteArrayInputStream +import java.io.IOException import java.io.InputStream import java.net.URI +/** + * Task for selecting a particular book, and adding it to the favorites list. + * Extends the AbstractBookTask class. + */ class BookSelectTask ( + private val accountID: AccountID, + profileID: ProfileID, + profiles: ProfilesDatabaseType, private val HTTPClient: LSHTTPClientType, - private val account: AccountType, private val feedEntry: OPDSAcquisitionFeedEntry, private val bookRegistry: BookRegistryType -) { +) : AbstractBookTask(accountID, profileID, profiles) { - private lateinit var taskRecorder: TaskRecorderType + override val taskRecorder: TaskRecorderType = + TaskRecorder.create() - private val logger = - LoggerFactory.getLogger(BookSelectTask::class.java) + override fun onFailure(result: TaskResult.Failure) { + //Do nothing + } - fun execute() : TaskResult<*> { - this.taskRecorder = TaskRecorder.create() + override val logger = + LoggerFactory.getLogger(BookSelectTask::class.java) + override fun execute(account: AccountType) : TaskResult.Success { //First create database entry, so we have something we can post status to - this.taskRecorder.beginNewStep("Create database entry") //ID of the book, can be new val bookID = BookIDs.newFromOPDSEntry(this.feedEntry) @@ -48,7 +60,7 @@ class BookSelectTask ( val bookInitial = Book( id = bookID, - account = account.id, + account = this.accountID, cover = null, thumbnail = null, entry = feedEntry, @@ -56,7 +68,7 @@ class BookSelectTask ( ) // Get either the book that already was in database, or initialize a new database entry - val book = this.getOrCreateBookDatabaseEntry(bookInitial, feedEntry) + val book = this.getOrCreateBookDatabaseEntry(account, bookInitial, feedEntry) this.taskRecorder.currentStepSucceeded("Initial database entry successful") @@ -67,7 +79,7 @@ class BookSelectTask ( val baseURI = feedEntry.alternate.getOrNull() if (baseURI == null) { logger.debug("No alternate link to use as basis for request") - return taskRecorder.finishFailure() + return taskRecorder.finishSuccess(Unit) } //Form the select URI by adding the desired path @@ -100,16 +112,16 @@ class BookSelectTask ( when (val status = response.status) { is LSHTTPResponseStatus.Responded.OK -> { // Parse the answer and store the new value to the database and book register - this.handleOKRequest(currentURI, status, book) - return taskRecorder.finishSuccess("Success") + this.handleOKRequest(account, currentURI, status, book) + return taskRecorder.finishSuccess(Unit) } is LSHTTPResponseStatus.Responded.Error -> { this.handleHTTPError(status) - return taskRecorder.finishFailure() + return taskRecorder.finishSuccess(Unit) } is LSHTTPResponseStatus.Failed -> { this.handleHTTPFailure(status) - return taskRecorder.finishFailure() + return taskRecorder.finishSuccess(Unit) } } } @@ -120,13 +132,14 @@ class BookSelectTask ( } private fun getOrCreateBookDatabaseEntry( + account: AccountType, book: Book, entry: OPDSAcquisitionFeedEntry ): Book { this.taskRecorder.beginNewStep("Setting up a book database entry...") try { - val database = this.account.bookDatabase + val database = account.bookDatabase //If the book is already in the database, use that book instead of the generated one //If the book is not, we initialize a db entry with the given book @@ -162,26 +175,39 @@ class BookSelectTask ( private fun handleHTTPError( status: LSHTTPResponseStatus.Responded.Error ) { - val report = status.properties.problemReport - if (report != null) { - taskRecorder.addAttributes(report.toMap()) - } + if (status.properties.status == 401) { + //Create an exception that is handled in AbstractBookTask and forwarded to Controller, + //From where the BookSelectTask was called + val message = String.format("bookSelect failed, bad credentials") + val exception = IOException(message) + //Fail the current step + this.taskRecorder.currentStepFailed( + message = message, + errorCode = "accessTokenExpired", + exception = exception + ) + this.logger.debug("refresh credentials due to 401 server response") + //Failure is checked and handled in Controller, where the tokenRefresh is triggered + //Don't set as logged out, as can possibly be logged in with tokenRefresh + throw TaskFailedHandled(exception) + } else { + val report = status.properties.problemReport + if (report != null) { + taskRecorder.addAttributes(report.toMap()) + } - taskRecorder.currentStepFailed( - message = "HTTP request failed: ${status.properties.originalStatus} ${status.properties.message}", - errorCode = "SelectedError", - exception = null - ) + taskRecorder.currentStepFailed( + message = "HTTP request failed: ${status.properties.originalStatus} ${status.properties.message}", + errorCode = "SelectedError", + exception = null + ) - //Catch 401 with special handling in controller - if (report?.type == "http://librarysimplified.org/terms/problem/credentials-invalid") { - throw SelectTaskException.SelectAccessTokenExpired() + throw SelectTaskException.SelectTaskFailed() } - - throw SelectTaskException.SelectTaskFailed() } private fun handleOKRequest( + account: AccountType, uri: URI, status: LSHTTPResponseStatus.Responded.OK, book: Book @@ -193,7 +219,7 @@ class BookSelectTask ( val entry = this.parseOPDSFeedEntry(inputStream, uri) //get database - val database = this.account.bookDatabase + val database = account.bookDatabase //The version in database might be carrying loan information //And just overwriting the existing entry with the new one would not carry that information over diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt index 8f305b6dd..518ab438c 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt @@ -93,11 +93,15 @@ class BookSyncTask( ) //Get the loans stream - val loansStream: InputStream? = fetchFeed( + //Continue execution only if successful + //Otherwise it's useless and can trigger multiple refresh + //requests in row, which makes the logout happen unnecessarily + val loansStream: InputStream = fetchFeed( provider.loansURI, credentials, account - ) + ) ?: return this.taskRecorder.finishSuccess(Unit) + //Get the selected stream val selectedStream: InputStream? = fetchFeed( provider.selectedURI, @@ -106,7 +110,7 @@ class BookSyncTask( ) //If both fetches went fine, we combine the streams //And update database and registry - if (loansStream != null && selectedStream != null) { + if (selectedStream != null) { this.onHTTPOKMultipleFeeds( loansStream = loansStream, selectedStream = selectedStream, @@ -535,14 +539,22 @@ class BookSyncTask( ): Boolean { when(account.loginState.credentials) { is AccountAuthenticationCredentials.Ekirjasto -> { + //If the answer is 401, refresh needs to be triggered if (result.properties.status == 401) { + //Create an exception that is handled in AbstractBookTask and forwarded to Controller, + //From where the BookSyncTask was called + val message = String.format("bookSync failed, bad credentials") + val exception = IOException(message) + //Fail the current step + this.taskRecorder.currentStepFailed( + message = message, + errorCode = "accessTokenExpired", + exception = exception + ) this.logger.debug("refresh credentials due to 401 server response") - //Launch accessToken refresh - booksController.executeProfileAccountAccessTokenRefresh(accountID) - //Returns true, as it's not an actual error, so can continue normally + //Failure is checked and handled in Controller, where the tokenRefresh is triggered //Don't set as logged out, as can possibly be logged in with tokenRefresh - //If logout is needed, it is handled in another part of the code - return true + throw TaskFailedHandled(exception) } } else -> { diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt index 82f8704a4..8a00b2fbe 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookUnselectTask.kt @@ -10,6 +10,7 @@ import org.librarysimplified.http.api.LSHTTPResponseStatus import org.librarysimplified.mdc.MDCKeys import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP import org.nypl.simplified.accounts.api.AccountAuthenticatedHTTP.addCredentialsToProperties +import org.nypl.simplified.accounts.api.AccountID import org.nypl.simplified.accounts.database.api.AccountType import org.nypl.simplified.books.api.BookID import org.nypl.simplified.books.api.BookIDs @@ -24,29 +25,38 @@ import org.nypl.simplified.opds.core.OPDSAvailabilityLoanable import org.nypl.simplified.opds.core.OPDSAvailabilityType import org.nypl.simplified.opds.core.OPDSParseException import org.nypl.simplified.opds.core.getOrNull +import org.nypl.simplified.profiles.api.ProfileID +import org.nypl.simplified.profiles.api.ProfilesDatabaseType import org.nypl.simplified.taskrecorder.api.TaskRecorder import org.nypl.simplified.taskrecorder.api.TaskRecorderType import org.nypl.simplified.taskrecorder.api.TaskResult import org.slf4j.LoggerFactory import org.slf4j.MDC import java.io.ByteArrayInputStream +import java.io.IOException import java.io.InputStream import java.net.URI +/** + * Task for unselecting a particular book, removing it from the favorites list. + * Extends the AbstractBookTask class. + */ + class BookUnselectTask ( + private val accountID: AccountID, + profileID: ProfileID, + profiles: ProfilesDatabaseType, private val HTTPClient: LSHTTPClientType, - private val account: AccountType, private val feedEntry: OPDSAcquisitionFeedEntry, private val bookRegistry: BookRegistryType -) { - - private lateinit var taskRecorder: TaskRecorderType +) : AbstractBookTask(accountID, profileID, profiles) { - private val logger = + override val taskRecorder: TaskRecorderType = + TaskRecorder.create() + override val logger = LoggerFactory.getLogger(BookUnselectTask::class.java) - fun execute() : TaskResult<*> { - this.taskRecorder = TaskRecorder.create() + override fun execute(account: AccountType): TaskResult.Success { taskRecorder.beginNewStep("Creating an OPDS Unselect...") try { @@ -63,7 +73,7 @@ class BookUnselectTask ( val baseURI = feedEntry.alternate.getOrNull() if (baseURI == null) { logger.debug("No link to form") - return taskRecorder.finishFailure() + return taskRecorder.finishSuccess(Unit) } //Form the select URI by adding the desired path val currentURI = URI.create(baseURI.toString().plus("/unselect_book")) @@ -95,16 +105,16 @@ class BookUnselectTask ( when (val status = response.status) { is LSHTTPResponseStatus.Responded.OK -> { //If successful, store returned value to bookRegistry - this.handleOKRequest(currentURI, status, databaseEntry, bookID) - return taskRecorder.finishSuccess("Success") + this.handleOKRequest(account, currentURI, status, databaseEntry, bookID) + return taskRecorder.finishSuccess(Unit) } is LSHTTPResponseStatus.Responded.Error -> { this.handleHTTPError(status) - return taskRecorder.finishFailure() + return taskRecorder.finishSuccess(Unit) } is LSHTTPResponseStatus.Failed -> { this.handleHTTPFailure(status) - return taskRecorder.finishFailure() + return taskRecorder.finishSuccess(Unit) } } } @@ -114,6 +124,10 @@ class BookUnselectTask ( } } + override fun onFailure(result: TaskResult.Failure) { + //Do nothing + } + private fun handleHTTPFailure( status: LSHTTPResponseStatus.Failed ) { @@ -128,29 +142,45 @@ class BookUnselectTask ( private fun handleHTTPError( status: LSHTTPResponseStatus.Responded.Error ) { - val report = status.properties.problemReport - if (report != null) { - taskRecorder.addAttributes(report.toMap()) - } - taskRecorder.currentStepFailed( - message = "HTTP request failed: ${status.properties.originalStatus} ${status.properties.message}", - errorCode = "UnselectError", - exception = null - ) + if (status.properties.status == 401) { + //Create an exception that is handled in AbstractBookTask and forwarded to Controller, + //From where the BookUnselectTask was called + val message = String.format("bookSelect failed, bad credentials") + val exception = IOException(message) + //Fail the current step + this.taskRecorder.currentStepFailed( + message = message, + errorCode = "accessTokenExpired", + exception = exception + ) + this.logger.debug("refresh credentials due to 401 server response") + //Failure is checked and handled in Controller, where the tokenRefresh is triggered + //Don't set as logged out, as can possibly be logged in with tokenRefresh + throw TaskFailedHandled(exception) - //Catch 401 with special handling in controller - if (report?.type == "http://librarysimplified.org/terms/problem/credentials-invalid") { - throw SelectTaskException.SelectAccessTokenExpired() - } + } else { - throw SelectTaskException.UnselectTaskFailed() + val report = status.properties.problemReport + if (report != null) { + taskRecorder.addAttributes(report.toMap()) + } + + taskRecorder.currentStepFailed( + message = "HTTP request failed: ${status.properties.originalStatus} ${status.properties.message}", + errorCode = "UnselectError", + exception = null + ) + + throw SelectTaskException.UnselectTaskFailed() + } } /** * Update the database and registry so that the selected info is removed */ private fun handleOKRequest( + account: AccountType, uri: URI, status: LSHTTPResponseStatus.Responded.OK, databaseEntry: BookDatabaseEntryType, @@ -203,7 +233,7 @@ class BookUnselectTask ( errorCode = "Parse error", exception = e ) - throw SelectTaskException.SelectTaskFailed() + throw SelectTaskException.UnselectTaskFailed() } } } diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt index 1aece99f5..5618e3444 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/Controller.kt @@ -764,19 +764,57 @@ class Controller private constructor( accountID: AccountID ): FluentFuture> { return this.submitTask( - BookSyncTask( - accountID = accountID, - profileID = this.profileCurrent().id, - profiles = this.profiles, - accountRegistry = this.accountProviders, - bookRegistry = this.bookRegistry, - booksController = this, - feedParser = this.feedParser, - feedLoader = this.feedLoader, - patronParsers = this.patronUserProfileParsers, - http = this.lsHttp - ) - ) + Callable> { + val syncTask = BookSyncTask( + accountID = accountID, + profileID = this.profileCurrent().id, + profiles = this.profiles, + accountRegistry = this.accountProviders, + bookRegistry = this.bookRegistry, + booksController = this, + feedParser = this.feedParser, + feedLoader = this.feedLoader, + patronParsers = this.patronUserProfileParsers, + http = this.lsHttp + ) + syncTask.call() + } + ).transformAsync(AsyncFunction { taskResult -> + //Check if the result was a need to refresh the accessToken + //BookSyncTask does special error handling, and if the result is 401, it throws + //A special error, which can be caught with the error code of "accessTokenExpired" + if (taskResult is TaskResult.Failure) { + logger.debug("SYNC ERROR : {}", taskResult.lastErrorCode) + } + if (taskResult is TaskResult.Failure && taskResult.lastErrorCode == "accessTokenExpired" ) { + //In order to make the user not have to do anything + //Try to sync again if the accessToken refresh is successful + executeProfileAccountAccessTokenRefresh(accountID).transformAsync(AsyncFunction { tokenResult -> + if (tokenResult is TaskResult.Success) { + logger.debug("Attempt to execute bookSync again") + val syncTask = BookSyncTask( + accountID = accountID, + profileID = this.profileCurrent().id, + profiles = this.profiles, + accountRegistry = this.accountProviders, + bookRegistry = this.bookRegistry, + booksController = this, + feedParser = this.feedParser, + feedLoader = this.feedLoader, + patronParsers = this.patronUserProfileParsers, + http = this.lsHttp + ) + submitTask(Callable { syncTask.call() }) + } else { + //If accessToken refresh fails, return the result that should popup the login + Futures.immediateFuture(tokenResult) + } + }, MoreExecutors.directExecutor()) + } else { + //In other errors return the normal bookSync answer + Futures.immediateFuture(taskResult) + } + }, MoreExecutors.directExecutor()) } override fun bookRevoke( @@ -802,11 +840,9 @@ class Controller private constructor( } ).transformAsync(AsyncFunction { taskResult -> //Check if the result was a need to refresh the accessToken - //BookRevokeTask does special error handling, and if the result is 401, it throws - //A special error, which can be caught with the error code of "accessTokenExpired" + //This can be caught with the error code of "accessTokenExpired" if (taskResult is TaskResult.Failure && taskResult.lastErrorCode == "accessTokenExpired" ) { - //In order to make the user not have to do anything - //Try to return the book again if the accessToken refresh is successful + //Try to run revoke again if the accessToken refresh is successful executeProfileAccountAccessTokenRefresh(accountID).transformAsync(AsyncFunction { tokenResult -> if (tokenResult is TaskResult.Success) { logger.debug("Attempt to execute return again") @@ -904,21 +940,47 @@ class Controller private constructor( ) : FluentFuture> { val profile = this.profileCurrent() - val account = profile.account(accountID) - //FIXFIXFIX - //Add 401 handling return this.submitTask( - Callable> { + Callable> { val bookSelectTask = BookSelectTask( + accountID= accountID, + profileID = profile.id, + profiles = profiles, HTTPClient = this.lsHttp, - account = account, feedEntry = feedEntry.feedEntry, bookRegistry = bookRegistry ) - bookSelectTask.execute() + bookSelectTask.call() } - ) + ).transformAsync(AsyncFunction { taskResult -> + //Check if the result was a need to refresh the accessToken + //Can be caught with the error code of "accessTokenExpired" + if (taskResult is TaskResult.Failure && taskResult.lastErrorCode == "accessTokenExpired" ) { + //In order to make the user not have to do anything + //Try to return select the book again if the accessToken refresh is successful + executeProfileAccountAccessTokenRefresh(accountID).transformAsync(AsyncFunction { tokenResult -> + if (tokenResult is TaskResult.Success) { + logger.debug("Attempt to execute bookSelect again") + val bookSelectTask = BookSelectTask( + accountID= accountID, + profileID = profile.id, + profiles = profiles, + HTTPClient = this.lsHttp, + feedEntry = feedEntry.feedEntry, + bookRegistry = bookRegistry + ) + submitTask(Callable { bookSelectTask.call() }) + } else { + //If accessToken refresh fails, return the result that should popup the login + Futures.immediateFuture(tokenResult) + } + }, MoreExecutors.directExecutor()) + } else { + //In other errors return the normal bookSync answer + Futures.immediateFuture(taskResult) + } + }, MoreExecutors.directExecutor()) } override fun bookRemoveFromSelected( @@ -926,21 +988,47 @@ class Controller private constructor( feedEntry: FeedEntry.FeedEntryOPDS ): FluentFuture> { val profile = this.profileCurrent() - val account = profile.account(accountID) - //FIXFIXFIX - //Add 401 handling return this.submitTask( - Callable >{ + Callable> { val bookUnselectTask = BookUnselectTask( + accountID= accountID, + profileID = profile.id, + profiles = profiles, HTTPClient = this.lsHttp, - account = account, - feedEntry =feedEntry.feedEntry, + feedEntry = feedEntry.feedEntry, bookRegistry = bookRegistry ) - bookUnselectTask.execute() + bookUnselectTask.call() } - ) + ).transformAsync(AsyncFunction { taskResult -> + //Check if the result was a need to refresh the accessToken + //Can be caught with the error code of "accessTokenExpired" + if (taskResult is TaskResult.Failure && taskResult.lastErrorCode == "accessTokenExpired" ) { + //In order to make the user not have to do anything + //Try to unselect the book again if the accessToken refresh is successful + executeProfileAccountAccessTokenRefresh(accountID).transformAsync(AsyncFunction { tokenResult -> + if (tokenResult is TaskResult.Success) { + logger.debug("Attempt to execute bookUnselect again") + val bookUnselectTask = BookUnselectTask( + accountID= accountID, + profileID = profile.id, + profiles = profiles, + HTTPClient = this.lsHttp, + feedEntry = feedEntry.feedEntry, + bookRegistry = bookRegistry + ) + submitTask(Callable { bookUnselectTask.call() }) + } else { + //If accessToken refresh fails, return the result that should popup the login + Futures.immediateFuture(tokenResult) + } + }, MoreExecutors.directExecutor()) + } else { + //In other errors return the normal bookSync answer + Futures.immediateFuture(taskResult) + } + }, MoreExecutors.directExecutor()) } override fun profileAnyIsCurrent(): Boolean = From 62cd8847458f92182f5516a17318e0b9d26042dd Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Mon, 24 Feb 2025 15:19:37 +0200 Subject: [PATCH 18/23] Adjust the bottom bar and fix book returning Moved the tabs around so that loans and reservation are under one tab, and that selected books are on another, TO this second tab, new tabs can be added for other lists. Revoking a book worked incorrectly when a book was selected, causing an old state of the book to be stored in registry. Fixed this. There also was a stalement issue with the views on logging out and back in. Fixed this by popping the backstack of the botton fragments, so that they are recreated the next time the views are looked at. Hid the collection buttons for all languages. --- .../src/main/res/values/drawables.xml | 2 +- .../books/controller/BookRevokeTask.kt | 14 +++++-- .../books/controller/BookSyncTask.kt | 2 +- .../books/controller/ProfileFeedTask.kt | 24 +++++------ .../librarysimplified/main/MainFragment.kt | 15 +++++-- .../main/MainFragmentListenerDelegate.kt | 6 ++- .../ui/catalog/CatalogFeedEvent.kt | 6 +++ .../ui/catalog/CatalogFeedFragment.kt | 40 ++++++++++--------- .../ui/catalog/CatalogFeedViewModel.kt | 2 + .../src/main/res/values/stringsFeed.xml | 5 +-- .../ui/navigation/tabs/BottomNavigators.kt | 12 +++--- .../{tab_holds.xml => tab_selected.xml} | 0 .../src/main/res/menu/navigation_items.xml | 6 +-- .../src/main/res/values/strings.xml | 2 +- 14 files changed, 83 insertions(+), 53 deletions(-) rename simplified-ui-navigation-tabs/src/main/res/drawable/{tab_holds.xml => tab_selected.xml} (100%) diff --git a/simplified-app-ekirjasto/src/main/res/values/drawables.xml b/simplified-app-ekirjasto/src/main/res/values/drawables.xml index 95bdf3b82..59d459b5e 100644 --- a/simplified-app-ekirjasto/src/main/res/values/drawables.xml +++ b/simplified-app-ekirjasto/src/main/res/values/drawables.xml @@ -3,7 +3,7 @@ @drawable/ic_elibrary_catalog_active @drawable/ic_elibrary_mybooks_active - @drawable/ic_elibrary_reservation_active + @drawable/ic_elibrary_reservation_active @drawable/ic_elibrary_settings_active @drawable/ic_caret_left @drawable/ekirjasto_logo_smaller diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt index ff81dca75..31b95b878 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt @@ -98,17 +98,25 @@ class BookRevokeTask( //Revoke request is sent to server, we set the revoke status again //and from the answer we form a new entry, //That we store to the db and register + //HERE this.revokeNotifyServer(account) //Get the registry entry val revokeBook = bookRegistry.books()[this.bookID] - //If there is a book and it is selected, just publish revoked status and then - //Update registry with the status created form the book + //If there is a book in the registry that is selected, just publish revoked status and then + //Update the database entry with the selected info + // And then update the registry from the database if (revokeBook != null && revokeBook.book.entry.selected is Some) { this.publishRevokedStatus() + //Update the database entry with the selected info + val updatedEntry = OPDSAcquisitionFeedEntry.newBuilderFrom(this.databaseEntry.book.entry) + .setSelectedOption(revokeBook.book.entry.selected) + .build() + //Write the entry to the datsabase + this.databaseEntry.writeOPDSEntry(updatedEntry) + //Update the book registry, based on the book that we just updated to database this.publishStatusFromDatabase() - this.bookRegistry.update(BookWithStatus(revokeBook.book, BookStatus.fromBook(revokeBook.book))) } else { //If not selected, we delete the book from database //And we clear the registry too diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt index 518ab438c..92117a417 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookSyncTask.kt @@ -468,7 +468,7 @@ class BookSyncTask( if (a is OPDSAvailabilityRevoked) { revoking.add(existingId) } else { - //Otherwide just deleting will do + //Otherwise just deleting will do this.logger.debug("[{}] deleting", existingId.brief()) //Load the single entry and add the "neutral" version to the book this.updateRegistryForBook(account, dbEntry) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt index ad4588aa8..15c358265 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/ProfileFeedTask.kt @@ -43,7 +43,7 @@ internal class ProfileFeedTask( */ //Chose the tabs we want to show on the my books page - val doSelectFacets = request.feedSelection != FeedBooksSelection.BOOKS_FEED_HOLDS + val doSelectFacets = request.feedSelection != FeedBooksSelection.BOOKS_FEED_SELECTED val facetGroups = this.makeFacets(doSelectFacets) val facets = facetGroups.values.flatten() @@ -159,10 +159,10 @@ internal class ProfileFeedTask( FilterBy.FILTER_BY_HOLDS -> FeedBooksSelection.BOOKS_FEED_HOLDS FilterBy.FILTER_BY_SELECTED -> FeedBooksSelection.BOOKS_FEED_SELECTED } - //Currently we don't want to show the button for holds feed + //Currently we don't want to show the selected feed in the books tab //So we don't add it to the facets - if (selectedFeed != FeedBooksSelection.BOOKS_FEED_HOLDS) { + if (selectedFeed != FeedBooksSelection.BOOKS_FEED_SELECTED) { facets.add(FilteringForFeed(title, active, selectedFeed, filterFacet)) } } @@ -339,7 +339,7 @@ internal class ProfileFeedTask( } } - private fun usableForBooksFeed(status: BookStatus): Boolean { + private fun usableForLoansFeed(status: BookStatus): Boolean { return when (status) { is BookStatus.Held, is BookStatus.Holdable, @@ -347,18 +347,17 @@ internal class ProfileFeedTask( is BookStatus.ReachedLoanLimit, is BookStatus.Revoked -> false - is BookStatus.Downloading, is BookStatus.DownloadWaitingForExternalAuthentication, is BookStatus.DownloadExternalAuthenticationInProgress, is BookStatus.FailedDownload, is BookStatus.FailedLoan, is BookStatus.FailedRevoke, - is BookStatus.Loaned, + is BookStatus.Loaned -> true is BookStatus.RequestingDownload, is BookStatus.RequestingLoan, - is BookStatus.Selected, - is BookStatus.Unselected, + is BookStatus.Selected -> false + is BookStatus.Unselected -> false is BookStatus.RequestingRevoke -> true } @@ -368,7 +367,6 @@ internal class ProfileFeedTask( return when (status) { is BookStatus.Held -> true - is BookStatus.Downloading, is BookStatus.DownloadWaitingForExternalAuthentication, is BookStatus.DownloadExternalAuthenticationInProgress, @@ -382,8 +380,8 @@ internal class ProfileFeedTask( is BookStatus.RequestingDownload, is BookStatus.RequestingLoan, is BookStatus.RequestingRevoke, - is BookStatus.Selected, - is BookStatus.Unselected, + is BookStatus.Selected -> false + is BookStatus.Unselected -> false is BookStatus.Revoked -> false } @@ -393,7 +391,7 @@ internal class ProfileFeedTask( * Return true if the book is usable for selected feed */ private fun usableForSelectedFeed(status: BookStatus): Boolean { - //Allow the usage for any Bookstatus, expect a book when the book is just unselected + //Allow the usage for any BookStatus, except a book when the book is just unselected return when (status) { is BookStatus.Unselected -> false else -> true @@ -436,7 +434,7 @@ internal class ProfileFeedTask( request: ProfileFeedRequest ): (BookStatus) -> Boolean { return when (request.feedSelection) { - FeedBooksSelection.BOOKS_FEED_LOANED -> ::usableForBooksFeed + FeedBooksSelection.BOOKS_FEED_LOANED -> ::usableForLoansFeed FeedBooksSelection.BOOKS_FEED_HOLDS -> ::usableForHoldsFeed FeedBooksSelection.BOOKS_FEED_SELECTED -> ::usableForSelectedFeed } diff --git a/simplified-main/src/main/java/org/librarysimplified/main/MainFragment.kt b/simplified-main/src/main/java/org/librarysimplified/main/MainFragment.kt index 720bb25bc..e4b3caa84 100644 --- a/simplified-main/src/main/java/org/librarysimplified/main/MainFragment.kt +++ b/simplified-main/src/main/java/org/librarysimplified/main/MainFragment.kt @@ -105,7 +105,7 @@ class MainFragment : Fragment(R.layout.main_tabbed_host) { when (it.itemId) { org.librarysimplified.ui.tabs.R.id.tabCatalog -> it.title = getString(org.librarysimplified.ui.tabs.R.string.tabCatalog) org.librarysimplified.ui.tabs.R.id.tabBooks -> it.title = getString(org.librarysimplified.ui.tabs.R.string.tabBooks) - org.librarysimplified.ui.tabs.R.id.tabHolds -> it.title = getString(org.librarysimplified.ui.tabs.R.string.tabHolds) + org.librarysimplified.ui.tabs.R.id.tabSelected -> it.title = getString(org.librarysimplified.ui.tabs.R.string.tabSelected) org.librarysimplified.ui.tabs.R.id.tabMagazines -> it.title = getString(org.librarysimplified.ui.tabs.R.string.tabMagazines) org.librarysimplified.ui.tabs.R.id.tabSettings -> it.title = getString(org.librarysimplified.ui.tabs.R.string.tabSettings) } @@ -225,7 +225,7 @@ class MainFragment : Fragment(R.layout.main_tabbed_host) { this.logger.debug("DBGHOLDS viewModel.showHoldsTab=$showHolds") val provider = viewModel.profilesController.profileCurrent().mostRecentAccount().provider this.logger.debug("DBGHOLDS accountProvider id=${provider.id}, displayName=${provider.displayName}, supportReservation=${provider.supportsReservations}") - val holdsItem = this.bottomView.menu.findItem(org.librarysimplified.ui.tabs.R.id.tabHolds) + val holdsItem = this.bottomView.menu.findItem(org.librarysimplified.ui.tabs.R.id.tabBooks) holdsItem.isVisible = showHolds holdsItem.isEnabled = showHolds } @@ -292,26 +292,35 @@ class MainFragment : Fragment(R.layout.main_tabbed_host) { } private fun onBookHoldsUpdateEvent(event: BookHoldsUpdateEvent) { + //Claimable holds val numberOfHolds = event.numberOfHolds + //If we have holds that are claimable, we show a red dot on the bottom tab to indicate it if (viewModel.showHoldsTab) { + //Get the item in the bottom nav we want to show the dot in val bottomNavigationItem = - this.bottomView.findViewById(org.librarysimplified.ui.tabs.R.id.tabHolds) + this.bottomView.findViewById(org.librarysimplified.ui.tabs.R.id.tabBooks) + //Get the red dot badge var badgeView = bottomNavigationItem.findViewById(org.librarysimplified.ui.tabs.R.id.badgeView) + //If there are showable holds and a badge to show if (numberOfHolds > 0) { if (badgeView == null) { + //Combine the dot and the item in the bottom tab into one view badgeView = LayoutInflater.from(requireContext()).inflate( org.librarysimplified.ui.tabs.R.layout.layout_menu_item_badge, bottomNavigationItem, false ) + //Add the new view to the spot of the original bottomNavigationItem.addView(badgeView) } + //Add the number in the dot that says how many are claimable val badgeNumber = (badgeView as? ViewGroup)?.findViewById( org.librarysimplified.ui.tabs.R.id.badgeNumber) badgeNumber?.text = numberOfHolds.toString() } + //Show the badge only if there are claimable holds badgeView?.isVisible = numberOfHolds > 0 } } diff --git a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt index fa6c1175e..434689735 100644 --- a/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt +++ b/simplified-main/src/main/java/org/librarysimplified/main/MainFragmentListenerDelegate.kt @@ -247,10 +247,14 @@ internal class MainFragmentListenerDelegate( state } - CatalogFeedEvent.GoUpwards -> { + is CatalogFeedEvent.GoUpwards -> { this.goUpwards() state } + is CatalogFeedEvent.RefreshViews -> { + this.navigator.popToRoot() + state + } } } diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedEvent.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedEvent.kt index 8f4526e2e..d44afb771 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedEvent.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedEvent.kt @@ -31,4 +31,10 @@ sealed class CatalogFeedEvent { val book: Book, val format: BookFormat ) : CatalogFeedEvent() + + /** + * Removes views not currently visible, so they are recreated with + * up to date information after login. + */ + data object RefreshViews :CatalogFeedEvent() } diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt index 684d59bee..1157ee373 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt @@ -261,7 +261,7 @@ class CatalogFeedFragment : Fragment(R.layout.feed), AgeGateDialog.BirthYearSele /* * Reconfigures the UI based on the log in status of the account. - * This effects the texts shown on my books and loans pages. + * This effects the texts shown on my books and favorites pages. * Triggered in onViewCreated and Start */ private fun reconfigureCatalogUI() { @@ -279,15 +279,15 @@ class CatalogFeedFragment : Fragment(R.layout.feed), AgeGateDialog.BirthYearSele private fun showLoginText() { if (parameters is CatalogFeedArguments.CatalogFeedArgumentsLocalBooks) { this.feedEmptyMessage.setText( - //Check if shown books are holds or not, set the message that is shown when - //viewing loans and holds. When not logged in, this text is always shown + //If selection is the selected books, show a suitable + //Text, otherwise show default (should never happen in current shape) if ( (parameters as CatalogFeedArguments.CatalogFeedArgumentsLocalBooks).selection == - FeedBooksSelection.BOOKS_FEED_HOLDS + FeedBooksSelection.BOOKS_FEED_SELECTED ) { - R.string.emptyHoldsNotLoggedIn + R.string.emptySelectedNotLoggedIn } else { - R.string.emptyLoansNotLoggedIn + R.string.emptyBooksNotLoggedIn } ) } @@ -305,29 +305,31 @@ class CatalogFeedFragment : Fragment(R.layout.feed), AgeGateDialog.BirthYearSele private fun showInfoWhenLoggedIn() { if (parameters is CatalogFeedArguments.CatalogFeedArgumentsLocalBooks) { this.feedEmptyMessage.setText( - //Check if shown books are holds or not, set the message that is shown when there are no - //books accordingly to loans and reserves when logged in + //Check if books shown are selected, and if the list is empty + //show an info of how to add books to selected + //Current layout should never show the default if ( (parameters as CatalogFeedArguments.CatalogFeedArgumentsLocalBooks).selection == - FeedBooksSelection.BOOKS_FEED_HOLDS + FeedBooksSelection.BOOKS_FEED_SELECTED ) { - R.string.feedWithGroupsEmptyHolds + R.string.feedWithGroupsEmptySelected } else { R.string.feedWithGroupsEmptyLoaned } ) } - //Set the feed empty message for a multifeed view, show special message for selected + //Set the feed empty messages for a multifeed view if (parameters is CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks) { this.feedEmptyMessage.setText( - //Add a special text to selected, otherwise show loans text + //Add a special text to loans and holds + //Check the current selection from stateLive to show the correct message per view if ( - (parameters as CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks).selection == - FeedBooksSelection.BOOKS_FEED_SELECTED + (viewModel.stateLive.value?.arguments as CatalogFeedArguments.CatalogFeedArgumentsAllLocalBooks).selection == + FeedBooksSelection.BOOKS_FEED_LOANED ) { - R.string.feedWithGroupsEmptySelected - } else { R.string.feedWithGroupsEmptyLoaned + } else { + R.string.feedWithGroupsEmptyHolds } ) } @@ -754,9 +756,11 @@ class CatalogFeedFragment : Fragment(R.layout.feed), AgeGateDialog.BirthYearSele val remainingGroups = facetsByGroup .filter { entry -> /* - * SIMPLY-2923: Hide the 'Collection' Facet until approved by UX. + * Hide the 'Collection' Facet since we only have one library */ - entry.key != "Collection" + entry.key != "Collection" && + entry.key != "Kokoelma" && + entry.key != "Samling" } .filter { entry -> !FeedFacets.facetGroupIsEntryPointTyped(entry.value) diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt index dc89968fd..cd58aceb8 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt @@ -197,7 +197,9 @@ class CatalogFeedViewModel( //We reload feed on login since in some login cases, //(in cases where there are books stored on the device) //the feed shows up empty despite there being loans due to not being updated on login + //Adding different types of feeds meant that there needs to be a backlog clear logger.debug("reloading feed due to successful login") + this.listener.post(CatalogFeedEvent.RefreshViews) this.reloadFeed() } } diff --git a/simplified-ui-catalog/src/main/res/values/stringsFeed.xml b/simplified-ui-catalog/src/main/res/values/stringsFeed.xml index 493777449..ed988be79 100644 --- a/simplified-ui-catalog/src/main/res/values/stringsFeed.xml +++ b/simplified-ui-catalog/src/main/res/values/stringsFeed.xml @@ -23,9 +23,8 @@ Holds When you reserve a book from the catalog, it will show up here. Look here from time to time to see if your book is available to download.\n Number of reservations is limited to 5. Visit the Catalog to add books to My Books.\n Number of loans is limited to 5.\n Ensure good internet connection when downloading a book and make sure there is enough space on your device. - Visit the Catalog to add books to Favorites. - Sign in to see your reservations. - Sign in to see your books. + Visit the Catalog to add books to Favorites. Add a book by clicking the plus icon next to the book\'s name. + Sign in to see your favorites. Sign in to see your books. Please enter your birth year Select Year diff --git a/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt b/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt index b6c463f76..44259733a 100644 --- a/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt +++ b/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt @@ -74,10 +74,10 @@ object BottomNavigators { defaultProvider = accountProviders.defaultProvider ) }, - R.id.tabHolds to { - createHoldsFragment( + R.id.tabSelected to { + createSelectedFragment( context = context, - id = R.id.tabHolds, + id = R.id.tabSelected, profilesController = profilesController, settingsConfiguration = settingsConfiguration, defaultProvider = accountProviders.defaultProvider @@ -223,7 +223,7 @@ object BottomNavigators { searchTerms = null, selection = FeedBooksSelection.BOOKS_FEED_HOLDS, sortBy = FeedFacet.FeedFacetPseudo.Sorting.SortBy.SORT_BY_TITLE, - title = context.getString(R.string.tabHolds), + title = "Holds", updateHolds = true ) ) @@ -232,7 +232,7 @@ object BottomNavigators { /** * Create a fragment for showing the loaned books. */ - private fun createBooksFragment( + private fun createLoansFragment( context: Context, id: Int, profilesController: ProfilesControllerType, @@ -300,7 +300,7 @@ object BottomNavigators { CatalogFeedOwnership.OwnedByAccount(filterAccountId) } - //Create the fragment hacing the selection be the selected + //Create the fragment having the selection be the selected return CatalogFeedFragment.create( CatalogFeedArguments.CatalogFeedArgumentsLocalBooks( filterAccount = filterAccountId, diff --git a/simplified-ui-navigation-tabs/src/main/res/drawable/tab_holds.xml b/simplified-ui-navigation-tabs/src/main/res/drawable/tab_selected.xml similarity index 100% rename from simplified-ui-navigation-tabs/src/main/res/drawable/tab_holds.xml rename to simplified-ui-navigation-tabs/src/main/res/drawable/tab_selected.xml diff --git a/simplified-ui-navigation-tabs/src/main/res/menu/navigation_items.xml b/simplified-ui-navigation-tabs/src/main/res/menu/navigation_items.xml index c693119b3..1b5fe3e54 100644 --- a/simplified-ui-navigation-tabs/src/main/res/menu/navigation_items.xml +++ b/simplified-ui-navigation-tabs/src/main/res/menu/navigation_items.xml @@ -13,9 +13,9 @@ android:title="@string/tabBooks" /> + android:id="@+id/tabSelected" + android:icon="@drawable/tab_selected" + android:title="@string/tabSelected" /> My Books Browse Books - Reservations + Favorites Magazines Profile Settings From 088c3bca68309ab285fb70b8ca0c7cf4e9695843 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Mon, 24 Feb 2025 17:27:16 +0200 Subject: [PATCH 19/23] Show login if select while logged out Added a check for select funtion, that shows the login page when trying to choose a book while logged out. Fized a string. --- .../books/controller/BookRevokeTask.kt | 2 +- .../ui/catalog/CatalogBookDetailViewModel.kt | 17 +++++++++++++---- .../ui/catalog/CatalogFeedViewModel.kt | 16 ++++++++++++---- .../src/main/res/values/stringsCatalog.xml | 4 ++-- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt index 31b95b878..939751660 100644 --- a/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt +++ b/simplified-books-controller/src/main/java/org/nypl/simplified/books/controller/BookRevokeTask.kt @@ -113,7 +113,7 @@ class BookRevokeTask( val updatedEntry = OPDSAcquisitionFeedEntry.newBuilderFrom(this.databaseEntry.book.entry) .setSelectedOption(revokeBook.book.entry.selected) .build() - //Write the entry to the datsabase + //Write the entry to the database this.databaseEntry.writeOPDSEntry(updatedEntry) //Update the book registry, based on the book that we just updated to database this.publishStatusFromDatabase() diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt index 5b5ea5649..a9a201e73 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt @@ -11,6 +11,7 @@ import io.reactivex.disposables.CompositeDisposable import io.reactivex.subjects.PublishSubject import net.jcip.annotations.GuardedBy import org.nypl.simplified.accounts.api.AccountID +import org.nypl.simplified.accounts.api.AccountLoginState import org.nypl.simplified.accounts.api.AccountProviderAuthenticationDescription import org.nypl.simplified.accounts.database.api.AccountType import org.nypl.simplified.books.api.Book @@ -38,6 +39,7 @@ import org.nypl.simplified.opds.core.OPDSAvailabilityMatcherType import org.nypl.simplified.opds.core.OPDSAvailabilityOpenAccess import org.nypl.simplified.opds.core.OPDSAvailabilityRevoked import org.nypl.simplified.profiles.api.ProfilePreferences +import org.nypl.simplified.profiles.api.ProfileReadableType import org.nypl.simplified.profiles.controller.api.ProfilesControllerType import org.nypl.simplified.taskrecorder.api.TaskResult import org.nypl.simplified.ui.errorpage.ErrorPageParameters @@ -299,10 +301,17 @@ class CatalogBookDetailViewModel( } override fun selectBook(feedEntry: FeedEntry.FeedEntryOPDS) { - booksController.bookAddToSelected( - accountID = profilesController.profileCurrent().mostRecentAccount().id, - feedEntry = feedEntry - ) + //Check if we are logged in, if not, show login + val account = this.profilesController.profileCurrent().mostRecentAccount() + if (account.loginState is AccountLoginState.AccountNotLoggedIn) { + openLoginDialog(account.id) + } else { + //If logged in, try adding book to selected + booksController.bookAddToSelected( + accountID = profilesController.profileCurrent().mostRecentAccount().id, + feedEntry = feedEntry + ) + } } override fun unselectBook(feedEntry: FeedEntry.FeedEntryOPDS) { diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt index cd58aceb8..b3c2d8ac9 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt @@ -1184,10 +1184,18 @@ class CatalogFeedViewModel( } override fun selectBook(feedEntry: FeedEntry.FeedEntryOPDS) { - booksController.bookAddToSelected( - accountID = profilesController.profileCurrent().mostRecentAccount().id, - feedEntry = feedEntry - ) + //Check if logged in + val account = this.profilesController.profileCurrent().mostRecentAccount() + if (account.loginState is AccountLoginState.AccountNotLoggedIn) { + //Show login page if not + openLoginDialog(account.id) + } else { + //Otherwise try selecting the book + booksController.bookAddToSelected( + accountID = profilesController.profileCurrent().mostRecentAccount().id, + feedEntry = feedEntry + ) + } } override fun unselectBook(feedEntry: FeedEntry.FeedEntryOPDS) { diff --git a/simplified-ui-catalog/src/main/res/values/stringsCatalog.xml b/simplified-ui-catalog/src/main/res/values/stringsCatalog.xml index 977a74616..64fb6c444 100644 --- a/simplified-ui-catalog/src/main/res/values/stringsCatalog.xml +++ b/simplified-ui-catalog/src/main/res/values/stringsCatalog.xml @@ -4,8 +4,8 @@ Get Delete Download - Add this book to your selected books - Remove this book from your selected books + Add this book to your favorites + Remove this book from your favorites Cancel download Details Dismiss From afd343b02f64d872a200a7acd3b0ba46ec2743fc Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Mon, 24 Feb 2025 17:38:25 +0200 Subject: [PATCH 20/23] Add same handling to unselect Added the same account check to unselect, though ending up in that case shouldn't ever happen. --- .../ui/catalog/CatalogBookDetailViewModel.kt | 15 +++++++++++---- .../ui/catalog/CatalogFeedViewModel.kt | 15 +++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt index a9a201e73..ff9786d66 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookDetailViewModel.kt @@ -315,10 +315,17 @@ class CatalogBookDetailViewModel( } override fun unselectBook(feedEntry: FeedEntry.FeedEntryOPDS) { - booksController.bookRemoveFromSelected( - accountID = profilesController.profileCurrent().mostRecentAccount().id, - feedEntry = feedEntry - ) + //Check if we are logged in, if not, show login + val account = this.profilesController.profileCurrent().mostRecentAccount() + if (account.loginState is AccountLoginState.AccountNotLoggedIn) { + openLoginDialog(account.id) + } else { + //Attempt to unselect + booksController.bookRemoveFromSelected( + accountID = profilesController.profileCurrent().mostRecentAccount().id, + feedEntry = feedEntry + ) + } } override fun resetInitialBookStatus(feedEntry: FeedEntry.FeedEntryOPDS) { diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt index b3c2d8ac9..c1267b69f 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt @@ -1199,10 +1199,17 @@ class CatalogFeedViewModel( } override fun unselectBook(feedEntry: FeedEntry.FeedEntryOPDS) { - booksController.bookRemoveFromSelected( - accountID = profilesController.profileCurrent().mostRecentAccount().id, - feedEntry = feedEntry - ) + //Check if we are logged in, if not, show login + val account = this.profilesController.profileCurrent().mostRecentAccount() + if (account.loginState is AccountLoginState.AccountNotLoggedIn) { + openLoginDialog(account.id) + } else { + //Attempt to unselect + booksController.bookRemoveFromSelected( + accountID = profilesController.profileCurrent().mostRecentAccount().id, + feedEntry = feedEntry + ) + } } override fun openBookPreview(feedEntry: FeedEntry.FeedEntryOPDS) { From 58f52bd29c3c57469618ce4fbb64c617e8dd5b70 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Tue, 25 Feb 2025 15:12:14 +0200 Subject: [PATCH 21/23] Add translatable title to selected fragment Added a translatable title to selected, and added back an old title, in case it needs usage later, and to not delete the translation. --- .../librarysimplified/ui/navigation/tabs/BottomNavigators.kt | 4 ++-- simplified-ui-navigation-tabs/src/main/res/values/strings.xml | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt b/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt index 44259733a..730b1d8ba 100644 --- a/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt +++ b/simplified-ui-navigation-tabs/src/main/java/org/librarysimplified/ui/navigation/tabs/BottomNavigators.kt @@ -223,7 +223,7 @@ object BottomNavigators { searchTerms = null, selection = FeedBooksSelection.BOOKS_FEED_HOLDS, sortBy = FeedFacet.FeedFacetPseudo.Sorting.SortBy.SORT_BY_TITLE, - title = "Holds", + title = context.getString(R.string.tabHolds), updateHolds = true ) ) @@ -308,7 +308,7 @@ object BottomNavigators { searchTerms = null, selection = FeedBooksSelection.BOOKS_FEED_SELECTED, sortBy = FeedFacet.FeedFacetPseudo.Sorting.SortBy.SORT_BY_TITLE, - title = "Favorites", + title = context.getString(R.string.tabSelected), updateHolds = false ) ) diff --git a/simplified-ui-navigation-tabs/src/main/res/values/strings.xml b/simplified-ui-navigation-tabs/src/main/res/values/strings.xml index 0073a2e66..dedd3e3e9 100644 --- a/simplified-ui-navigation-tabs/src/main/res/values/strings.xml +++ b/simplified-ui-navigation-tabs/src/main/res/values/strings.xml @@ -3,6 +3,7 @@ My Books Browse Books + Reservations Favorites Magazines Profile From 1a821df2906f1de8c2438e78727fbacff489c992 Mon Sep 17 00:00:00 2001 From: Sade-Tuuli Siipola Date: Thu, 6 Mar 2025 11:43:42 +0200 Subject: [PATCH 22/23] Minor fixes Increased the touch area of the favorite button on all views. Added a UI refresh to showing empty feed, so the shown text is correct in each view. --- .../org/librarysimplified/ui/catalog/CatalogFeedFragment.kt | 2 ++ simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml | 5 +++-- simplified-ui-catalog/src/main/res/layout/book_detail.xml | 5 +++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt index 1157ee373..9f7ef9043 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedFragment.kt @@ -494,6 +494,8 @@ class CatalogFeedFragment : Fragment(R.layout.feed), AgeGateDialog.BirthYearSele if (feedState.facetsByGroup != null) { // If there are some top level facets,configure them so they are shown on an empty view this.configureFacetTabs(FeedFacets.findEntryPointFacetGroup(feedState.facetsByGroup), feedContentTabs) + //Update catalog UI to have the texts be up to date + this.reconfigureCatalogUI() feedContentHeader.visibility = View.VISIBLE } } diff --git a/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml b/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml index 7aefffca5..e7dcac642 100644 --- a/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml +++ b/simplified-ui-catalog/src/main/res/layout/book_cell_idle.xml @@ -57,10 +57,11 @@ Date: Thu, 6 Mar 2025 14:51:54 +0200 Subject: [PATCH 23/23] Refresh views after book's status is reset Refresh the views after a book's status is reset to either holdable or loanable. This fixes unloaned books showing up in loans when a loan fails and the error is dismissed. --- .../ui/catalog/CatalogFeedViewModel.kt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt index c1267b69f..c0b31ac7e 100644 --- a/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt +++ b/simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogFeedViewModel.kt @@ -284,8 +284,18 @@ class CatalogFeedViewModel( is BookStatus.FailedDownload, is BookStatus.FailedLoan, is BookStatus.FailedRevoke, - is BookStatus.Holdable, - is BookStatus.Loanable, + is BookStatus.Holdable -> { + if (this.state.arguments.isLocallyGenerated) { + //Reload feed so dismissed failed holds are not shown in holds feed + this.reloadFeed() + } + } + is BookStatus.Loanable -> { + if (this.state.arguments.isLocallyGenerated) { + //Reload feed so no failed and dismissed loans show up + this.reloadFeed() + } + } is BookStatus.ReachedLoanLimit, is BookStatus.RequestingDownload, is BookStatus.RequestingLoan,