Fix/lookup error 404

src/nsls2api/api/v1/user_api.py

@@ -16,52 +16,58 @@
 
 @router.get("/person/username/{username}", response_model=Person)
 async def get_person_from_username(username: str):
-    bnl_person = await bnlpeople_service.get_person_by_username(username)
-    print(bnl_person)
-    if bnl_person:
-        person = Person(
-            firstname=bnl_person.FirstName,
-            lastname=bnl_person.LastName,
-            email=bnl_person.BNLEmail,
-            bnl_id=bnl_person.EmployeeNumber,
-            institution=bnl_person.Institution,
-            username=bnl_person.ActiveDirectoryName,
-            cyber_agreement_signed=bnl_person.CyberAgreementSigned,
-        )
-        # If the person is an Employee then set their institution to BNL
-        if (
-            bnl_person.EmployeeStatus == "Active"
-            and bnl_person.EmployeeType == "Employee"
-        ):
-            person.bnl_employee = True
-            person.institution = "Brookhaven National Laboratory"
-        return person
-    else:
-        return fastapi.responses.JSONResponse(
-            {"error": f"No people with username {username} found."},
-            status_code=404,
-        )
+    try:
+        bnl_person = await bnlpeople_service.get_person_by_username(username)
+    except LookupError as e:
+        raise HTTPException(status_code=404, detail=str(e))
+    except ValueError as e:
+        raise HTTPException(status_code=400, detail=str(e))
+
+    person = Person(
+        firstname=bnl_person.FirstName,
+        lastname=bnl_person.LastName,
+        email=bnl_person.BNLEmail,
+        bnl_id=bnl_person.EmployeeNumber,
+        institution=bnl_person.Institution,
+        username=bnl_person.ActiveDirectoryName,
+        cyber_agreement_signed=bnl_person.CyberAgreementSigned,
+    )
+    # If the person is an Employee then set their institution to BNL
+    if (
+        bnl_person.EmployeeStatus == "Active"
+        and bnl_person.EmployeeType == "Employee"
+    ):
+        person.bnl_employee = True
+        person.institution = "Brookhaven National Laboratory"
+    return person
 
 
-@router.get("/person/email/{email}")
+@router.get("/person/email/{email}", response_model=Person)
 async def get_person_from_email(email: str):
-    bnl_person = await bnlpeople_service.get_person_by_email(email)
-    if bnl_person:
-        person = Person(
-            firstname=bnl_person.FirstName,
-            lastname=bnl_person.LastName,
-            email=bnl_person.BNLEmail,
-            bnl_id=bnl_person.EmployeeNumber,
-            institution=bnl_person.Institution,
-            username=bnl_person.ActiveDirectoryName,
-            cyber_agreement_signed=bnl_person.CyberAgreementSigned,
-        )
-        return person
-    else:
-        return fastapi.responses.JSONResponse(
-            {"error": f"No people with username {email} found."},
-            status_code=404,
-        )
+    try:
+        bnl_person = await bnlpeople_service.get_person_by_email(email)
+    except LookupError as e:
+        raise HTTPException(status_code=404, detail=str(e))
+    except ValueError as e:
+        raise HTTPException(status_code=400, detail=str(e))
+
+    person = Person(
+        firstname=bnl_person.FirstName,
+        lastname=bnl_person.LastName,
+        email=bnl_person.BNLEmail,
+        bnl_id=bnl_person.EmployeeNumber,
+        institution=bnl_person.Institution,
+        username=bnl_person.ActiveDirectoryName,
+        cyber_agreement_signed=bnl_person.CyberAgreementSigned,
+    )
+    # If the person is an Employee then set their institution to BNL
+    if (
+        bnl_person.EmployeeStatus == "Active"
+        and bnl_person.EmployeeType == "Employee"
+    ):
+        person.bnl_employee = True
+        person.institution = "Brookhaven National Laboratory"
+    return person
 
 
 # TODO: Add back into schema if we decide to use this endpoint.

src/nsls2api/services/bnlpeople_service.py

@@ -19,13 +19,19 @@ async def get_all_people():
     return people
 
 
-async def get_person_by_username(username: str) -> BNLPerson | None:
+async def get_person_by_username(username: str) -> BNLPerson:
     url = f"{base_url}/api/BNLPeople?accountName={username}"
     person = await _call_bnlpeople_webservice(url)
-    if len(person) == 0 or len(person) > 1:
-        raise LookupError(
-            f"BNL People could not find a person with a username of '{username}'"
+    if len(person) == 0:
+        logger.warning(
+            f"BNL People API could not find a person with a username of '{username}'"
+        )
+        raise LookupError(f"No person with username {username} found.")
+    if len(person) > 1:
+        logger.error(
+            f"BNL People API returned {len(person)} people for username '{username}' - ambiguous result"
         )
+        raise ValueError(f"Multiple people found with username {username}.")
     return BNLPerson(**person[0])
 
 
@@ -44,7 +50,7 @@ async def get_username_by_id(lifenumber: str) -> str | None:
     # logger.debug(person)
     if len(person) == 0 or len(person) > 1:
         logger.warning(
-            f"BNL People could not find a person with an employee/life number of '{lifenumber}'"
+            f"BNL People API could not find a person with an employee/life number of '{lifenumber}'"
         )
         return None
 
@@ -67,18 +73,24 @@ async def get_person_by_id(lifenumber: str) -> BNLPerson | None:
 
     if len(person) == 0 or len(person) > 1:
         raise LookupError(
-            f"BNL People could not find a person with an employee/life number of '{lifenumber}'"
+            f"BNL People API could not find a person with an employee/life number of '{lifenumber}'"
         )
     return BNLPerson(**person[0])
 
 
-async def get_person_by_email(email: str) -> BNLPerson | None:
+async def get_person_by_email(email: str) -> BNLPerson:
     url = f"{base_url}/api/BNLPeople?email={email}"
     person = await _call_bnlpeople_webservice(url)
-    if len(person) == 0 or len(person) > 1:
-        raise LookupError(
-            f"BNL People could not find a person with an email of '{email}'"
+    if len(person) == 0:
+        logger.warning(
+            f"BNL People API could not find a person with an email of '{email}'"
+        )
+        raise LookupError(f"No person with email {email} found.")
+    if len(person) > 1:
+        logger.error(
+            f"BNL People API returned {len(person)} people for email '{email}' - ambiguous result"
         )
+        raise ValueError(f"Multiple people found with email {email}. Query is ambiguous.")
     return BNLPerson(**person[0])
 
 
@@ -89,7 +101,7 @@ async def get_people_by_department(
     people = await _call_bnlpeople_webservice(url)
     if len(people) == 0:
         raise LookupError(
-            f"BNL People could not find a person with the department code of '{department_code}'"
+            f"BNL People API could not find a person with the department code of '{department_code}'"
         )
     people_in_department = [BNLPerson(**p) for p in people]
     return people_in_department

src/nsls2api/services/person_service.py

@@ -37,22 +37,11 @@ async def diagnostic_details_by_username(username: str) -> Person | None:
         )
         ad_groups = await n2sn_service.get_groups_by_username(username)
         proposals = await get_proposals_by_person(bnl_person.EmployeeNumber)
-    except LookupError as error:
+    except (LookupError, ValueError) as error:
         raise LookupError(
             f"Error obtaining diagnostic details for username of {username}"
         ) from error
 
-    print(bnl_person)
-    print("-------")
-
-    print(ad_person)
-    print("-------")
-
-    print(ad_groups)
-    print("-------")
-
-    print(proposals)
-    print("-------")
 
     person = Person(
         firstname=bnl_person.FirstName,

src/nsls2api/services/proposal_service.py

@@ -810,8 +810,8 @@ async def generate_fake_test_proposal(
                     is_pi=True,
                 )
                 user_list.append(user)
-        except LookupError:
-            logger.error(f"Could not find user {add_specific_user} in BNLPeople.")
+        except (LookupError, ValueError) as e:
+            logger.error(f"Could not find user {add_specific_user} in BNLPeople: {e}")
             return None
 
     fake_proposal_id = await generate_fake_proposal_id()