Fix Admin API Get (#625)

Fix a bug so that if you GET a user on the Admin API that has no associated addresses, you now get a 200 with a payload with an empty address array rather than a 404 - Not Found.

Author: 0xCLARITY

src/data-access/users.ts

@@ -7,6 +7,27 @@ import knex from '../db/knex'
 import { Account, Address, AddressInformation } from '../types/database'
 import logger from '../utils/logger'
 
+/**
+ * Checks if a given PayID exists in the account table in the PayID database.
+ *
+ * @param payId - The PayID to insert in the account table.
+ *
+ * @returns A boolean indicating whether the PayID exists.
+ *
+ * Note: This could actually be done in getAllAddressInfoFromDatabase,
+ * if we changed the return type a bit, which would let us always do a single database call,
+ * instead of having to call this as a follow-up check, but it's probably cleaner to
+ * let them have separate concerns until performance becomes an issue.
+ */
+// TODO:(hbergren): Type payId better
+export async function checkUserExistence(payId: string): Promise<boolean> {
+  const result = await knex.select(
+    knex.raw('exists(select 1 from account where pay_id = ?)', payId),
+  )
+
+  return result[0].exists
+}
+
 /**
  * Inserts a new user/PayID into the account table in the PayID database.
  *
@@ -174,6 +195,10 @@ async function insertAddresses(
   addresses: readonly DatabaseAddress[],
   transaction: Transaction,
 ): Promise<readonly AddressInformation[]> {
+  if (addresses.length === 0) {
+    return []
+  }
+
   // TODO:(hbergren) Verify that the number of inserted addresses matches the input address array length?
   return knex
     .insert(addresses)

src/db/seed/seeded_values_for_testing.sql

@@ -7,7 +7,8 @@ INSERT INTO account(id, pay_id) VALUES
 ('223ece9c-2a15-48e1-9df6-d9ac77c5db90', 'bob$127.0.0.1'),
 ('ec06236e-d134-4a7b-b69e-0606fb54b67b', 'alice$xpring.money'),
 ('69b0d20a-cdef-4bb9-adf9-2109979a12af', 'bob$xpring.money'),
-('b253bed2-79ce-45d0-bbdd-96867aa85fd5', 'zebra$xpring.money');
+('b253bed2-79ce-45d0-bbdd-96867aa85fd5', 'zebra$xpring.money'),
+('8a75f884-ab16-40c4-a82a-aca454dad6b2', 'empty$xpring.money');
 
 INSERT INTO address(account_id, payment_network, environment, details) VALUES
 ('232370e9-045e-4269-96ec-5a79091d65ff', 'XRPL', 'MAINNET', '{"address": "rw2ciyaNshpHe7bCHo4bRWq6pqqynnWKQg", "tag": "67298042"}'),

src/middlewares/users.ts

@@ -7,6 +7,7 @@ import {
   replaceUser,
   removeUser,
   replaceUserPayId,
+  checkUserExistence,
 } from '../data-access/users'
 import {
   LookupError,
@@ -45,16 +46,16 @@ export async function getUser(
   }
 
   // TODO:(hbergren) Does not work for multiple accounts
-  const addresses = await getAllAddressInfoFromDatabase(payId)
-
-  // TODO:(hbergren) Should it be possible to have a PayID with no addresses?
-  if (addresses.length === 0) {
+  const doesUserExist = await checkUserExistence(payId)
+  if (!doesUserExist) {
     throw new LookupError(
       `No information could be found for the PayID ${payId}.`,
       LookupErrorType.Unknown,
     )
   }
 
+  const addresses = await getAllAddressInfoFromDatabase(payId)
+
   res.locals.response = {
     payId,
     addresses,

test/integration/data-access/reports.test.ts

@@ -46,7 +46,7 @@ describe('Data Access - getPayIdCounts()', function (): void {
 
   it('getPayIdCount - Returns a count of PayIDs', async function () {
     const payIdCount = await getPayIdCount()
-    const expectedPayIdCount = 5
+    const expectedPayIdCount = 6
 
     assert.strictEqual(payIdCount, expectedPayIdCount)
   })

test/integration/e2e/admin-api/getUsers.test.ts

@@ -43,6 +43,23 @@ describe('E2E - adminApiRouter - GET /users', function (): void {
       .expect(HttpStatus.OK, expectedResponse, done)
   })
 
+  it('Returns a 200 and correct information for a user known to exist without any addresses', function (done): void {
+    // GIVEN a PayID known to resolve to an account on the PayID service
+    const payId = 'empty$xpring.money'
+    const expectedResponse = {
+      payId: 'empty$xpring.money',
+      addresses: [],
+    }
+
+    // WHEN we make a GET request to /users/ with that PayID as our user
+    request(app.adminApiExpress)
+      .get(`/users/${payId}`)
+      .set('PayID-API-Version', payIdApiVersion)
+      .expect('Content-Type', /json/u)
+      // THEN We expect back a 200 - OK, with the account information
+      .expect(HttpStatus.OK, expectedResponse, done)
+  })
+
   it('Returns a 200 and correct information for a user known to exist, when we use an uppercase PayID', function (done): void {
     // GIVEN a PayID known to resolve to an account on the PayID service
     const payId = 'ALICE$XPRING.MONEY'

test/integration/e2e/admin-api/metrics.test.ts

@@ -100,7 +100,7 @@ describe('E2E - adminApiRouter - GET /metrics', function (): void {
 
     // We create 8 PayIDs in the tests before this one,
     // and start with 5 seeded PayIDs, for a total of 13.
-    await assertMetrics(/actual_payid_count\{org="127.0.0.1"\} 13/u)
+    await assertMetrics(/actual_payid_count\{org="127.0.0.1"\} 14/u)
   })
 
   /**

test/integration/e2e/admin-api/putUsers.test.ts

@@ -71,6 +71,24 @@ describe('E2E - adminApiRouter - PUT /users', function (): void {
       .expect(HttpStatus.OK, updatedInformation, done)
   })
 
+  it('Returns a 200 and updated user payload when removing all addresses', function (done): void {
+    // GIVEN a PayID known to resolve to an account on the PayID service
+    const payId = 'empty$xpring.money'
+    const updatedInformation = {
+      payId: 'empty$xpring.money',
+      addresses: [],
+    }
+
+    // WHEN we make a PUT request to /users/ with the new information to update
+    request(app.adminApiExpress)
+      .put(`/users/${payId}`)
+      .set('PayID-API-Version', payIdApiVersion)
+      .send(updatedInformation)
+      .expect('Content-Type', /json/u)
+      // THEN we expect back a 200-OK, with the updated user information
+      .expect(HttpStatus.OK, updatedInformation, done)
+  })
+
   it('Returns a 201 and inserted user payload for a Admin API PUT creating a new user', function (done): void {
     // GIVEN a PayID known to not exist on the PayID service
     const payId = 'notjohndoe$xpring.money'