Library details checks of help_email have been fixed to allow help_ur…

…l too (#177)

The urls_and_contacts subdocument will now feature an additional help_url
which may be a web uri or null

controller.py

@@ -391,6 +391,15 @@ def library_details(self, uuid, library=None, patron_count=None):
             copyright_email_validated_at,
         ) = (self._validated_at(hyperlinks.get(rel, None)) for rel in hyperlink_types)
 
+        # If we don't have a help email, we might have a help uri
+        help_url = None
+        if (
+            not help_email
+            and (link := hyperlinks.get(Hyperlink.HELP_REL))
+            and link.href
+        ):
+            help_url = link.href
+
         setting_types = [Library.PLS_ID]
         settings = dict()
         for s in library.settings:
@@ -427,6 +436,7 @@ def library_details(self, uuid, library=None, patron_count=None):
             authentication_url=library.authentication_url,
             opds_url=library.opds_url,
             web_url=library.web_url,
+            help_url=help_url,
         )
 
         # This will be slow unless ServiceArea has been preloaded with a joinedload().
@@ -459,7 +469,10 @@ def _format_place_name(self, place):
 
     def _get_email(self, hyperlink):
         if hyperlink and hyperlink.resource and hyperlink.resource.href:
-            return hyperlink.resource.href.split("mailto:")[1]
+            try:
+                return hyperlink.resource.href.split("mailto:")[1]
+            except IndexError:
+                return None
 
     def _validated_at(self, hyperlink):
         validated_at = "Not validated"

tests/test_controller.py

@@ -288,6 +288,10 @@ def _is_library(self, expected, actual, has_email=True):
                 assert expected.pls_id.value == flattened.get(k)
             elif k == "number_of_patrons":
                 assert str(getattr(expected, k)) == flattened.get(k)
+            elif k in ["help_url"]:
+                # Alternate constraint, is not directly part of the library model
+                # Is tested in the alternate path
+                pass
             else:
                 assert getattr(expected, k) == flattened.get(k)
 
@@ -324,6 +328,7 @@ def _check_keys(self, library):
             "help_validated",
             "copyright_validated",
             "opds_url",
+            "help_url",
         ]
         assert set(library.get("urls_and_contact")) == set(expected_url_contact_keys)
 
@@ -529,16 +534,26 @@ def test_library_details(self):
         # Test that the controller can look up the complete information for one specific library.
         library = self.nypl
 
-        def check(has_email=True):
+        def check(has_email=True, assertion=True):
             uuid = library.internal_urn.split("uuid:")[1]
             with self.app.test_request_context("/"):
                 response = self.controller.library_details(uuid, 0)
             assert response.get("uuid") == uuid
             self._check_keys(response)
-            self._is_library(library, response, has_email)
+            if assertion:
+                self._is_library(library, response, has_email)
+            return response
 
         check()
 
+        # Check if changing the help email to a link removes the email value, and adds a link
+        for l in library.hyperlinks:
+            if l.rel == Hyperlink.HELP_REL:
+                l.href = "http://example.org/help"
+        response = check(assertion=False)
+        assert response["urls_and_contact"]["help_email"] == None
+        assert response["urls_and_contact"]["help_url"] == "http://example.org/help"
+
         # Delete the library's contact email, simulating an old
         # library created before this rule was instituted, and try
         # again.