From 6ead5b8f75de0e65cee88cf5e89c38bf4eea01b0 Mon Sep 17 00:00:00 2001
From: Brian Moses Hall <moseshll@umich.edu>
Date: Wed, 26 Nov 2025 12:16:05 -0500
Subject: [PATCH 1/3] ETT-1148 Resource sharing users should get access to
 items that are deposited but not reported as held - Add `Access::Holdings`
 utility routine to extract "held" value based on both `currently_held_count`
 and `deposited` - Add tests with manipulated versions of the expected
 currently-held response from mocked Holdings API - Add `deposited` fields to
 the mock holdings used in Playwright tests

---
 mdp-lib/Access/Holdings.pm                    | 28 ++++++++++++++--
 mdp-lib/t/access/holdings.t                   | 33 +++++++++++++++++--
 .../web/v1/item_access.currently_held         |  2 +-
 .../web/v1/item_access.not_current            |  2 +-
 mock-holdings-api/web/v1/item_access.not_held |  2 +-
 5 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/mdp-lib/Access/Holdings.pm b/mdp-lib/Access/Holdings.pm
index 2eed14097..e5e834af8 100644
--- a/mdp-lib/Access/Holdings.pm
+++ b/mdp-lib/Access/Holdings.pm
@@ -121,8 +121,7 @@ sub _query_item_access_api {
       $holdings_data->{n_enum},
       @{$holdings_data->{ocns}}
     );
-    $held = $holdings_data->{copy_count};
-    $held = $holdings_data->{$field};
+    $held = _extract_held_by_field($holdings_data, $field);
   };
   if (my $err = $@) {
     log_error($err);
@@ -156,6 +155,31 @@ sub _query_item_held_by_api {
 
 # ---------------------------------------------------------------------
 
+=item _extract_held_by_field
+
+Uses the named field to distill Holdings API return structure into a
+`held` value. Plain vanilla `copy_count` and `brlm_count` just extract
+the field of the same name. `currently_held_count` consults the
+`currently_held_count` and the `deposited` value.
+
+=cut
+
+# ---------------------------------------------------------------------
+sub _extract_held_by_field {
+  my $holdings_data = shift;
+  my $field         = shift;
+
+  if ($field eq 'currently_held_count') {
+    if ($holdings_data->{currently_held_count} > 0) {
+      return $holdings_data->{currently_held_count};
+    }
+    return $holdings_data->{deposited} || 0;
+  }
+  return $holdings_data->{$field};
+}
+
+# ---------------------------------------------------------------------
+
 =item _id_is_held_core
 
 Common code for id_is_held, id_is_held_and_BRLM, and id_is_currently_held.
diff --git a/mdp-lib/t/access/holdings.t b/mdp-lib/t/access/holdings.t
index 0d1e9bdb6..90944dede 100644
--- a/mdp-lib/t/access/holdings.t
+++ b/mdp-lib/t/access/holdings.t
@@ -58,6 +58,7 @@ my $held_response = {
   'copy_count' => 555,
   'brlm_count' => 222,
   'currently_held_count' => 111,
+  'deposited' => 1,
   'format' => 'spm',
   'n_enum' => 'v.1-5 (1901-1905)',
   'ocns' => ['001', '002', '003']
@@ -78,7 +79,7 @@ sub get_ua_for_error {
   return $ua;
 }
 
-my $held_response_json = $jsonxs->encode($held_response);
+#my $held_response_json = $jsonxs->encode($held_response);
 my $institutions_response_json = $jsonxs->encode($institutions_response);
 my $item_access_endpoint = qr{$Access::Holdings::ITEM_ACCESS_ENDPOINT};
 my $item_held_by_endpoint = qr{$Access::Holdings::ITEM_HELD_BY_ENDPOINT};
@@ -94,8 +95,13 @@ sub get_ua {
   return $ua;
 }
 
+# Can use predefined $held_response above, or pass a custom structure.
 sub get_ua_for_held {
-  return get_ua($item_access_endpoint, $held_response_json);
+  my $response = shift || $held_response;
+  return get_ua(
+    $item_access_endpoint,
+    $jsonxs->encode($response)
+  );
 }
 
 sub get_ua_for_institutions {
@@ -289,6 +295,16 @@ subtest "id_is_currently_held" => sub {
       expect_params($ua);
       $ENV{DEBUG} = $save_debug;
     };
+
+    subtest "currently_held_count and deposited both 0" => sub {
+      # Copy the default response so we can fiddle with it
+      my $response = { %$held_response };
+      $response->{currently_held_count} = 0;
+      $response->{deposited} = 0;
+      my $ua = get_ua_for_held($response);
+      my ($lock_id, $held) = Access::Holdings::id_is_currently_held($C, $htid, 'umich', $ua);
+      is($held, 0);
+    };
   };
 
   subtest "currently held according to API" => sub {
@@ -307,6 +323,19 @@ subtest "id_is_currently_held" => sub {
       expect_params($ua);
       $ENV{DEBUG} = $save_debug;
     };
+
+    subtest "currently held if currently_held_count or deposited" => sub {
+      my $currently_held_deposited_combinations = [[0, 1], [1, 0], [1, 1]];
+      foreach my $combo (@$currently_held_deposited_combinations) {
+        # Copy the default response so we can fiddle with it
+        my $response = { %$held_response };
+        $response->{currently_held_count} = $combo->[0];
+        $response->{deposited} = $combo->[1];
+        my $ua = get_ua_for_held($response);
+        my ($lock_id, $held) = Access::Holdings::id_is_currently_held($C, $htid, 'umich', $ua);
+        is($held, 1, "currently_held=$combo->[0], deposited=$combo->[1]");
+      };
+    };
   };
 
   subtest "return error message and held = 0 if API fails, and log error message" => sub {
diff --git a/mock-holdings-api/web/v1/item_access.currently_held b/mock-holdings-api/web/v1/item_access.currently_held
index ade90c376..4b4ecf38d 100644
--- a/mock-holdings-api/web/v1/item_access.currently_held
+++ b/mock-holdings-api/web/v1/item_access.currently_held
@@ -1 +1 @@
-{"brlm_count":0,"copy_count":1,"currently_held_count":1,"format":"spm","n_enum":"","ocns":[123456]}
+{"brlm_count":0,"copy_count":1,"currently_held_count":1,"deposited":1,"format":"spm","n_enum":"","ocns":[123456]}
diff --git a/mock-holdings-api/web/v1/item_access.not_current b/mock-holdings-api/web/v1/item_access.not_current
index e6697742c..48f8ed644 100644
--- a/mock-holdings-api/web/v1/item_access.not_current
+++ b/mock-holdings-api/web/v1/item_access.not_current
@@ -1 +1 @@
-{"brlm_count":0,"copy_count":1,"currently_held_count":0,"format":"spm","n_enum":"","ocns":[123456]}
+{"brlm_count":0,"copy_count":1,"currently_held_count":0,"deposited":0,"format":"spm","n_enum":"","ocns":[123456]}
diff --git a/mock-holdings-api/web/v1/item_access.not_held b/mock-holdings-api/web/v1/item_access.not_held
index 07bd4007b..9c2bc64df 100644
--- a/mock-holdings-api/web/v1/item_access.not_held
+++ b/mock-holdings-api/web/v1/item_access.not_held
@@ -1 +1 @@
-{"brlm_count":0,"copy_count":0,"currently_held_count":0,"format":"spm","n_enum":"","ocns":[123456]}
+{"brlm_count":0,"copy_count":0,"currently_held_count":0,"deposited":0,"format":"spm","n_enum":"","ocns":[123456]}

From 6b008c0d8d9028ef1b29371b5d1513e2720f49f2 Mon Sep 17 00:00:00 2001
From: Brian Moses Hall <moseshll@umich.edu>
Date: Wed, 26 Nov 2025 12:22:16 -0500
Subject: [PATCH 2/3] ETT-743 disable brittle auth.t test

---
 mdp-lib/t/auth/{auth.t => auth.t.BRITTLE} | 3 +++
 1 file changed, 3 insertions(+)
 rename mdp-lib/t/auth/{auth.t => auth.t.BRITTLE} (97%)

diff --git a/mdp-lib/t/auth/auth.t b/mdp-lib/t/auth/auth.t.BRITTLE
similarity index 97%
rename from mdp-lib/t/auth/auth.t
rename to mdp-lib/t/auth/auth.t.BRITTLE
index 3f524ce32..64e0c2e76 100644
--- a/mdp-lib/t/auth/auth.t
+++ b/mdp-lib/t/auth/auth.t.BRITTLE
@@ -1,5 +1,8 @@
 #!/usr/bin/perl
 
+# THIS TEST IS DISABLED BECAUSE IT IS SUBJECT TO RANDOM SEGFAULTS
+# SEE ETT-743
+
 use strict;
 use warnings;
 

From 95b9675b5d4d614352a7fdb9cdbc098b2df10548 Mon Sep 17 00:00:00 2001
From: Brian Moses Hall <moseshll@umich.edu>
Date: Wed, 26 Nov 2025 12:31:30 -0500
Subject: [PATCH 3/3] Remove commented out variable no longer used

---
 mdp-lib/t/access/holdings.t | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mdp-lib/t/access/holdings.t b/mdp-lib/t/access/holdings.t
index 90944dede..6cbbb1242 100644
--- a/mdp-lib/t/access/holdings.t
+++ b/mdp-lib/t/access/holdings.t
@@ -79,7 +79,6 @@ sub get_ua_for_error {
   return $ua;
 }
 
-#my $held_response_json = $jsonxs->encode($held_response);
 my $institutions_response_json = $jsonxs->encode($institutions_response);
 my $item_access_endpoint = qr{$Access::Holdings::ITEM_ACCESS_ENDPOINT};
 my $item_held_by_endpoint = qr{$Access::Holdings::ITEM_HELD_BY_ENDPOINT};