EKIRJASTO-341 Show position in queue in holds, remove time estimate (#203)

* Show queue in holds, remove time estimate

As the time estimate is unreliable, remove it and prefer showing your position in the queue. To match iOS and to show more information to the user, show how many copies are currently in "circulation" aka how many copies of the book we have.

* Fix tests

* Fix spacing

Author: natlibfi-burger

simplified-books-borrowing/src/main/java/org/nypl/simplified/books/borrowing/internal/BorrowLoanCreate.kt

@@ -224,6 +224,7 @@ class BorrowLoanCreate private constructor() : BorrowSubtaskType {
               queuePosition = a.positionOrNull,
               startDate = a.startDateOrNull,
               isRevocable = a.revoke.isSome,
+              copiesTotal = a.copiesOrNull,
               endDate = a.endDateOrNull
             )
           )

simplified-books-registry-api/src/main/java/org/nypl/simplified/books/book_registry/BookStatus.kt

@@ -72,6 +72,12 @@ sealed class BookStatus {
 
       val startDate: DateTime?,
 
+      /**
+       * @return The number of copies for this book
+       */
+
+      val copiesTotal: Int?,
+
       override val isRevocable: Boolean,
       override val endDate: DateTime?
     ) : Held() {
@@ -480,6 +486,7 @@ sealed class BookStatus {
         queuePosition = this.someOrNull(a.position),
         startDate = this.someOrNull(a.startDate),
         endDate = this.someOrNull(a.endDate),
+        copiesTotal = this.someOrNull(a.copies),
         isRevocable = a.revoke.isSome
       )
     }

simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAcquisitionFeedEntryParser.java

@@ -802,11 +802,16 @@ private OPDSAvailabilityType inferAvailability(
         final OptionType<DateTime> start_date =
           OPDSXML.getAttributeRFC3339Optional(available, "since");
         OptionType<Integer> queue = Option.none();
+        OptionType<Integer> copies = Option.none();
         if (holds_opt.isSome()) {
           final Some<Element> holds_some = (Some<Element>) holds_opt;
           queue = OPDSXML.getAttributeIntegerOptional(holds_some.get(), "position");
         }
-        return OPDSAvailabilityHeld.get(start_date, queue, end_date, revoke);
+        if (copies_opt.isSome()) {
+          final Some<Element> copies_some = (Some<Element>) copies_opt;
+          copies = OPDSXML.getAttributeIntegerOptional(copies_some.get(), "total");
+        }
+          return OPDSAvailabilityHeld.get(start_date, queue, end_date, copies, revoke);
       }
 
       if ("available".equals(status)) {

simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSAvailabilityHeld.kt

@@ -13,6 +13,7 @@ import java.net.URI
 data class OPDSAvailabilityHeld private constructor(
   val startDate: OptionType<DateTime>,
   val position: OptionType<Int>,
+  val copies: OptionType<Int>,
   private val endDate: OptionType<DateTime>,
 
   /**
@@ -35,6 +36,9 @@ data class OPDSAvailabilityHeld private constructor(
   val positionOrNull: Int?
     get() = this.position.getOrNull()
 
+  val copiesOrNull: Int?
+    get() = this.copies.getOrNull()
+
   /**
    * @return The date that the hold will become unavailable
    */
@@ -54,6 +58,8 @@ data class OPDSAvailabilityHeld private constructor(
     val b = StringBuilder(256)
     b.append("[OPDSAvailabilityHeld position=")
     b.append(this.position)
+    b.append(" copies=")
+    b.append(this.copies)
     b.append(" start_date=")
     this.startDate.map { e: DateTime? ->
       b.append(fmt.print(e))
@@ -77,6 +83,7 @@ data class OPDSAvailabilityHeld private constructor(
      * @param startDate The start date (if known)
      * @param position The queue position
      * @param endDate The end date (if known)
+     * @param copies The number of copies for the book
      * @param revoke An optional revocation link for the hold
      * @return A value that states that a book is on hold
      */
@@ -86,12 +93,14 @@ data class OPDSAvailabilityHeld private constructor(
       startDate: OptionType<DateTime>,
       position: OptionType<Int>,
       endDate: OptionType<DateTime>,
+      copies: OptionType<Int>,
       revoke: OptionType<URI>
     ): OPDSAvailabilityHeld {
       return OPDSAvailabilityHeld(
         startDate = startDate,
         position = position,
         endDate = endDate,
+        copies = copies,
         revoke = revoke
       )
     }

simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSJSONParser.java

@@ -183,9 +183,11 @@ private static OPDSAvailabilityType parseAvailability(
           JSONParserUtilities.getIntegerOptional(n, "position");
         final OptionType<DateTime> in_end_date =
           JSONParserUtilities.getTimestampOptional(n, "end_date");
+        final OptionType<Integer> in_copies =
+          JSONParserUtilities.getIntegerOptional(n, "total");
         final OptionType<URI> in_revoke =
           JSONParserUtilities.getURIOptional(n, "revoke");
-        return OPDSAvailabilityHeld.get(in_start_date, in_position, in_end_date, in_revoke);
+        return OPDSAvailabilityHeld.get(in_start_date, in_position, in_end_date, in_copies, in_revoke);
       }
 
       if (node.has("held_ready")) {

simplified-opds-core/src/main/java/org/nypl/simplified/opds/core/OPDSJSONSerializer.java

@@ -163,6 +163,10 @@ public ObjectNode onHeld(final OPDSAvailabilityHeld a) {
             oh.put("position", x);
             return Unit.unit();
           });
+          a.getCopies().map(x -> {
+            oh.put("total", x);
+            return Unit.unit();
+          });
           a.getRevoke().map(uri -> {
             oh.put("revoke", uri.toString());
             return Unit.unit();

simplified-tests/src/test/java/org/nypl/simplified/tests/accessibility/AccessibilityServiceTest.kt

@@ -261,7 +261,7 @@ class AccessibilityServiceTest {
     this.bookRegistry.update(
       BookWithStatus(
         book = this.book0,
-        status = BookStatus.Held.HeldInQueue(this.book0.id, null, null, false, null)
+        status = BookStatus.Held.HeldInQueue(this.book0.id, null, null, null, false, null)
       )
     )
 

simplified-tests/src/test/java/org/nypl/simplified/tests/books/controller/BookRevokeTaskTest.kt

@@ -960,6 +960,7 @@ class BookRevokeTaskTest {
           Option.none(),
           Option.none(),
           Option.none(),
+          Option.none(),
           Option.some(this.server.url("revoke").toUri())
         )
       )
@@ -1093,6 +1094,7 @@ class BookRevokeTaskTest {
           Option.none(),
           Option.none(),
           Option.none(),
+          Option.none(),
           Option.none()
         )
       )

simplified-tests/src/test/java/org/nypl/simplified/tests/opds/OPDSFeedEntryParserTest.java

@@ -164,11 +164,12 @@ public void testEntryAvailabilityHeldIndefinite()
     final OptionType<DateTime> expected_start_date = Option.some(
       OPDSDateParsers.dateTimeParser().parseDateTime("2000-01-01T00:00:00Z"));
     final OptionType<Integer> queue_position = Option.none();
+    final OptionType<Integer> copies_total = Option.some(1);
     final OptionType<DateTime> expected_end_date = Option.none();
     final OptionType<URI> expected_revoke =
       Option.some(new URI("http://example.com/revoke"));
     final OPDSAvailabilityHeld expected = OPDSAvailabilityHeld.get(
-      expected_start_date, queue_position, expected_end_date, expected_revoke);
+      expected_start_date, queue_position, expected_end_date, copies_total,expected_revoke);
 
     Assertions.assertEquals(expected, availability);
     Assertions.assertEquals(1, e.getAcquisitions().size());
@@ -196,10 +197,11 @@ public void testEntryAvailabilityHeldTimed()
     final OptionType<DateTime> expected_end_date = Option.some(
       OPDSDateParsers.dateTimeParser().parseDateTime("2010-01-01T00:00:00Z"));
     final OptionType<Integer> queue_position = Option.none();
+    final OptionType<Integer> copies_total = Option.some(1);
     final OptionType<URI> expected_revoke =
       Option.some(new URI("http://example.com/revoke"));
     final OPDSAvailabilityHeld expected = OPDSAvailabilityHeld.get(
-      expected_start_date, queue_position, expected_end_date, expected_revoke);
+      expected_start_date, queue_position, expected_end_date, copies_total, expected_revoke);
 
     Assertions.assertEquals(expected, availability);
     Assertions.assertEquals(1, e.getAcquisitions().size());
@@ -225,11 +227,12 @@ public void testEntryAvailabilityHeldIndefiniteQueued()
     final OptionType<DateTime> expected_start_date = Option.some(
       OPDSDateParsers.dateTimeParser().parseDateTime("2000-01-01T00:00:00Z"));
     final OptionType<Integer> queue_position = Option.some(3);
+    final OptionType<Integer> copies_total = Option.some(1);
     final OptionType<DateTime> expected_end_date = Option.none();
     final OptionType<URI> expected_revoke =
       Option.some(new URI("http://example.com/revoke"));
     final OPDSAvailabilityHeld expected = OPDSAvailabilityHeld.get(
-      expected_start_date, queue_position, expected_end_date, expected_revoke);
+      expected_start_date, queue_position, expected_end_date,copies_total, expected_revoke);
 
     Assertions.assertEquals(expected, availability);
     Assertions.assertEquals(1, e.getAcquisitions().size());
@@ -257,10 +260,11 @@ public void testEntryAvailabilityHeldTimedQueued()
     final OptionType<DateTime> expected_end_date = Option.some(
       OPDSDateParsers.dateTimeParser().parseDateTime("2010-01-01T00:00:00Z"));
     final OptionType<Integer> queue_position = Option.some(3);
+    final OptionType<Integer> copies_total = Option.some(1);
     final OptionType<URI> expected_revoke =
       Option.some(new URI("http://example.com/revoke"));
     final OPDSAvailabilityHeld expected = OPDSAvailabilityHeld.get(
-      expected_start_date, queue_position, expected_end_date, expected_revoke);
+      expected_start_date, queue_position, expected_end_date, copies_total, expected_revoke);
 
     Assertions.assertEquals(expected, availability);
     Assertions.assertEquals(1, e.getAcquisitions().size());

simplified-ui-catalog/src/main/java/org/librarysimplified/ui/catalog/CatalogBookAvailabilityStrings.kt

@@ -7,6 +7,7 @@ import com.io7m.jfunctional.Some
 import org.joda.time.DateTime
 import org.joda.time.Hours
 import org.nypl.simplified.books.book_registry.BookStatus
+import org.nypl.simplified.opds.core.getOrNull
 import java.util.concurrent.TimeUnit
 
 /**
@@ -30,7 +31,7 @@ object CatalogBookAvailabilityStrings {
   ): String {
     return when (status) {
       is BookStatus.Held.HeldInQueue ->
-        onHeld(resources, Option.of(status.endDate), Option.of(status.queuePosition))
+        onHeld(resources, Option.of(status.copiesTotal), Option.of(status.queuePosition))
       is BookStatus.Held.HeldReady ->
         onLoanable(resources)
       is BookStatus.Holdable ->
@@ -84,23 +85,10 @@ object CatalogBookAvailabilityStrings {
     status: BookStatus.Held.HeldInQueue
   ): String {
     val queuePositionOpt : OptionType<Int> = Option.of(status.queuePosition)
-    val endDateOpt : OptionType<DateTime> = Option.of(status.endDate)
-    /*
-     * If there is an availability date, show this in preference to
-     * anything else.
-     */
-    if (endDateOpt is Some<DateTime>) {
-      val endDate = endDateOpt.get()
-      val now = DateTime.now()
-      return resources.getString(
-        R.string.catalogBookAvailabilityHeldTimedShort,
-        this.intervalStringHoldDuration(resources, now, endDate)
-      )
-    }
 
-    /*
-     * If there is a queue position, attempt to show this instead.
-     */
+    /**
+   * If there is a queue position, attempt to show it.
+   */
 
     if (queuePositionOpt is Some<Int>) {
       return resources.getString(R.string.catalogBookAvailabilityHeldQueueShort, queuePositionOpt.get())
@@ -173,28 +161,18 @@ object CatalogBookAvailabilityStrings {
 
   private fun onHeld(
     resources: Resources,
-    endDateOpt: OptionType<DateTime>,
+    copiesTotalOpt: OptionType<Int>,
     queuePositionOpt: OptionType<Int>
   ): String {
-    /*
-     * If there is an availability date, show this in preference to
-     * anything else.
-     */
-
-    if (endDateOpt is Some<DateTime>) {
-      val endDate = endDateOpt.get()
-      val now = DateTime.now()
-      return resources.getString(
-        R.string.catalogBookAvailabilityHeldTimed,
-        this.intervalString(resources, now, endDate)
-      )
-    }
 
-    /*
-     * If there is a queue position, attempt to show this instead.
+    /**
+     * If there is a queue position, attempt to show it.
      */
 
     if (queuePositionOpt is Some<Int>) {
+      if (copiesTotalOpt is Some<Int>) {
+        return resources.getString(R.string.catalogBookAvailabilityHeldQueueWithCopies, queuePositionOpt.get(), copiesTotalOpt.getOrNull())
+      }
       return resources.getString(R.string.catalogBookAvailabilityHeldQueue, queuePositionOpt.get())
     }
 

simplified-ui-catalog/src/main/res/values/stringsCatalog.xml

@@ -23,6 +23,7 @@
   <string name="catalogAccounts">Choose another library catalog…</string>
   <string name="catalogBookAvailabilityHeldIndefinite">You have this book on hold.</string>
   <string name="catalogBookAvailabilityHeldQueue">You are at position %1$d in the queue for this book.</string>
+  <string name="catalogBookAvailabilityHeldQueueWithCopies">You are at position %1$d in the queue for %2$d copies of this book.</string>
   <string name="catalogBookAvailabilityHeldTimed">You will be able to borrow this book in %1$s.</string>
   <string name="catalogBookAvailabilityHeldIndefiniteShort">On hold</string>
   <string name="catalogBookAvailabilityHeldQueueShort">At position: %1$d</string>