feat(backend): backend tenant graphql resolvers #3234

njlie · 2025-01-21T22:15:52Z

Changes proposed in this pull request

Adds the following resolvers to the backend Admin GraphQL API:
- whoami
- tenant
- tenants
- createTenant
- updateTenant
- deleteTenant

Context

Fixes #3124.

Checklist

Related issues linked using fixes #number
Tests added/updated
Make sure that all checks pass
Bruno collection updated (if necessary)
Documentation issue created with user-docs label (if necessary)
OpenAPI specs updated (if necessary)

koekiebox

The code in the PR looks good.
Just the Bruno collection to be added.

BlairCurrey · 2025-01-24T19:36:39Z

packages/backend/src/graphql/schema.graphql

+  "Fetch a paginated list of tenants on the instance."
+  tenants(


Should probably mention this is for operators only. I also wondered about using a new isOperator directive instead of throwing in the resolver. Not sure it matters much but I wonder if the generated docs would pick it up. Maybe not much point unless we have more resolvers that use it though.

I guess the createTenant is operator only as well. So should probably note it there in the same way.

BlairCurrey · 2025-01-24T19:37:52Z

packages/backend/src/graphql/resolvers/tenant.test.ts

+  beforeAll(async (): Promise<void> => {
+    deps = await initIocContainer({
+      ...Config,
+      databaseUrl: process.env.TENANT_TEST_DATABASE_URL as string


Why do we need this separate db instead of just using TEST_DATABASE_URL?

I added another test database to handle pagination tests correctly for tenants. Pagination tests need to truncate the tables between tests in order to work properly - if we do this on the same database as the other tests, that will break the ones that require the existence of an operator tenant to work, as it will have been deleted.

I realize that the pagination tests will be testing a scenario that shouldn't happen, but I feel like it makes sense to do it this way to make sure that pagination works correctly.

I think you can get away with this by using e.g. dbSchema config instead. This is how I ended up solving this (comment).

mkurapov · 2025-01-22T12:22:06Z

packages/backend/src/graphql/schema.graphql

@@ -1493,6 +1527,75 @@ type CancelIncomingPaymentResponse {
  payment: IncomingPayment
 }

+type Tenant implements Model {


Looking at the create tenants migration, email, idpConsentUrl and idpSecret (in the model and the create input) should be optional

mkurapov · 2025-01-22T12:28:26Z

packages/backend/src/graphql/resolvers/tenant.test.ts

+      test('Cannot get page as non-operator', async (): Promise<void> => {
+        const tenant = await createTenant(deps)
+        const apolloClient = createTenantedApolloClient(appContainer, tenant.id)
+        try {


Suggested change

try {

try {

expect.assertions(2)

(same for a few tests below)

mkurapov · 2025-01-27T16:24:49Z

packages/backend/src/graphql/resolvers/tenant.test.ts

+  beforeAll(async (): Promise<void> => {
+    deps = await initIocContainer({
+      ...Config,
+      databaseUrl: process.env.TENANT_TEST_DATABASE_URL as string


I think you can get away with this by using e.g. dbSchema config instead. This is how I ended up solving this (comment).

koekiebox

LGTM.

koekiebox

https://github.com/interledger/rafiki/actions/runs/12996726301/job/36246174979?pr=3234

Just need to look at the unit and integration tests failing.

mkurapov · 2025-01-28T12:49:22Z

packages/backend/scripts/init.sh

@@ -3,6 +3,8 @@ set -e

 psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL
    DROP DATABASE IF EXISTS TESTING;
+    DROP DATABASE IF EXISTS TENANT_TESTING;


These can be removed as well

(still applies)

mkurapov · 2025-01-28T13:01:05Z

packages/backend/src/graphql/schema.graphql

@@ -148,6 +148,26 @@ type Query {
    "Unique identifier of the receiver (incoming payment URL)."
    id: String!
  ): Receiver
+
+  "Retrieve a tenant of the instance."
+  tenant("Unique identifier of the tenant." id: String!): Tenant


Suggested change

tenant("Unique identifier of the tenant." id: String!): Tenant

tenant("Unique identifier of the tenant." id: String!): Tenant!

Since resolver throws if the tenant isn't found

mkurapov · 2025-01-28T13:03:08Z

packages/backend/src/graphql/resolvers/tenant.test.ts

+        ${true}    | ${'operator'}
+        ${false}   | ${'tenant'}
+      `(
+        'Can delete a tenant as $description',


Does this mean a tenant can delete itself?
If so, do you think should it be reserved only for operators?

Making deleteTenant operator-only as per call today.

koekiebox

LGTM.

mkurapov

A few final things

mkurapov · 2025-01-30T12:42:03Z

packages/backend/jest.env.js

@@ -1,4 +1,4 @@
-process.env.LOG_LEVEL = 'silent'
+process.env.LOG_LEVEL = 'info'


maybe we leave it as silent to keep the test output tidy

mkurapov · 2025-01-30T12:43:05Z

packages/backend/scripts/init.sh

@@ -3,6 +3,8 @@ set -e

 psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL
    DROP DATABASE IF EXISTS TESTING;
+    DROP DATABASE IF EXISTS TENANT_TESTING;


(still applies)

mkurapov · 2025-01-30T12:43:59Z

packages/backend/src/graphql/resolvers/tenant.test.ts

+      ${false}   | ${'tenant'}
+    `('whoami query as $description', async ({ isOperator }): Promise<void> => {
+      const tenant = await createTenant(deps)
+      console.log('created tenant')


Suggested change

console.log('created tenant')

github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. pkg: frontend Changes in the frontend package. type: source Changes business logic pkg: mock-ase pkg: mock-account-service-lib labels Jan 21, 2025

feat: backend tenant graphql resolvers

99d6982

njlie force-pushed the nl/3124/backend-tenant-resolvers branch from 23e34b2 to 99d6982 Compare January 21, 2025 22:16

chore: formatting

344e21a

njlie requested review from koekiebox, BlairCurrey, mkurapov and golobitch January 21, 2025 22:20

njlie marked this pull request as ready for review January 21, 2025 22:20

fix: extra testing db for tenants

657780f

This comment was marked as outdated.

Sign in to view

koekiebox requested changes Jan 23, 2025

View reviewed changes

mkurapov linked an issue Jan 23, 2025 that may be closed by this pull request

[Multi-Tenant] backend Admin Tenant API #3124

Open

6 tasks

feat: bruno collection

3ce9128

BlairCurrey reviewed Jan 24, 2025

View reviewed changes

BlairCurrey approved these changes Jan 24, 2025

View reviewed changes

feat: update graphql schema comments

15cb387

njlie requested a review from koekiebox January 24, 2025 21:26

mkurapov reviewed Jan 27, 2025

View reviewed changes

fix: review comments

4aafebb

koekiebox approved these changes Jan 28, 2025

View reviewed changes

koekiebox requested changes Jan 28, 2025

View reviewed changes

mkurapov reviewed Jan 28, 2025

View reviewed changes

feat: optional idp secret & consent url

9072e02

github-actions bot added the pkg: auth Changes in the GNAP auth package. label Jan 28, 2025

njlie added 2 commits January 28, 2025 11:09

feat: tenant response requirement

e4a6e6c

feat: make delete operator-only

45dcb5b

njlie requested review from koekiebox and mkurapov January 28, 2025 20:51

koekiebox assigned njlie Jan 29, 2025

koekiebox approved these changes Jan 29, 2025

View reviewed changes

mkurapov reviewed Jan 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(backend): backend tenant graphql resolvers #3234

feat(backend): backend tenant graphql resolvers #3234

njlie commented Jan 21, 2025 •

edited by mkurapov

Loading

This comment was marked as outdated.

koekiebox left a comment

BlairCurrey Jan 24, 2025 •

edited

Loading

BlairCurrey Jan 24, 2025

BlairCurrey Jan 24, 2025

njlie Jan 24, 2025

mkurapov Jan 27, 2025

mkurapov Jan 22, 2025

mkurapov Jan 22, 2025

mkurapov Jan 22, 2025

mkurapov Jan 27, 2025

koekiebox left a comment

koekiebox left a comment

mkurapov Jan 28, 2025

mkurapov Jan 30, 2025

mkurapov Jan 28, 2025

mkurapov Jan 28, 2025

njlie Jan 28, 2025

koekiebox left a comment

mkurapov left a comment

mkurapov Jan 30, 2025

mkurapov Jan 30, 2025

mkurapov Jan 30, 2025

		"Fetch a paginated list of tenants on the instance."
		tenants(

	tenant("Unique identifier of the tenant." id: String!): Tenant
	tenant("Unique identifier of the tenant." id: String!): Tenant!

		@@ -1,4 +1,4 @@
		process.env.LOG_LEVEL = 'silent'
		process.env.LOG_LEVEL = 'info'

feat(backend): backend tenant graphql resolvers #3234

Are you sure you want to change the base?

feat(backend): backend tenant graphql resolvers #3234

Conversation

njlie commented Jan 21, 2025 • edited by mkurapov Loading

Changes proposed in this pull request

Context

Checklist

This comment was marked as outdated.

koekiebox left a comment

Choose a reason for hiding this comment

BlairCurrey Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koekiebox left a comment

Choose a reason for hiding this comment

koekiebox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koekiebox left a comment

Choose a reason for hiding this comment

mkurapov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njlie commented Jan 21, 2025 •

edited by mkurapov

Loading

BlairCurrey Jan 24, 2025 •

edited

Loading