fix: Integration delete related bugs (#9189)

* fix: use knex directly to get source list since it requires custom condition

Signed-off-by: Pranav C <pranavxc@gmail.com>

* fix: add missing root scope for MetaService

Signed-off-by: Pranav C <pranavxc@gmail.com>

* fix: invalid integration id handling

Signed-off-by: Pranav C <pranavxc@gmail.com>

* test: bring integration unit tests for OSS

Signed-off-by: Pranav C <pranavxc@gmail.com>

* fix: if NcError throw as it is

Signed-off-by: Pranav C <pranavxc@gmail.com>

* test: use read api instead of list

Signed-off-by: Pranav C <pranavxc@gmail.com>

---------

Signed-off-by: Pranav C <pranavxc@gmail.com>

packages/nocodb-sdk/src/lib/globals.ts

@@ -192,8 +192,8 @@ export enum NcErrorType {
   INVALID_PK_VALUE = 'INVALID_PK_VALUE',
   COLUMN_ASSOCIATED_WITH_LINK = 'COLUMN_ASSOCIATED_WITH_LINK',
   TABLE_ASSOCIATED_WITH_LINK = 'TABLE_ASSOCIATED_WITH_LINK',
-  INTEGRATION_NOT_FOUND= 'INTEGRATION_NOT_FOUND',
+  INTEGRATION_NOT_FOUND = 'INTEGRATION_NOT_FOUND',
-  INTEGRATION_LINKED_WITH_BASES= 'INTEGRATION_LINKED_WITH_BASES',
+  INTEGRATION_LINKED_WITH_BASES = 'INTEGRATION_LINKED_WITH_BASES',
 }
 
 type Roles = OrgUserRoles | ProjectRoles | WorkspaceUserRoles;

packages/nocodb/src/meta/meta.service.ts

@@ -185,7 +185,7 @@ export class MetaService {
         });
       }
     } else {
-      if (!base_id) {
+      if (!base_id && base_id !== RootScopes.WORKSPACE) {
         NcError.metaError({
           message: 'Base ID is required',
           sql: '',
@@ -251,6 +251,7 @@ export class MetaService {
       [MetaTable.COMMENTS_REACTIONS]: 'cre',
       [MetaTable.USER_COMMENTS_NOTIFICATIONS_PREFERENCE]: 'cnp',
       [MetaTable.JOBS]: 'job',
+      [MetaTable.INTEGRATIONS]: 'int',
     };
 
     const prefix = prefixMap[target] || 'nc';
@@ -297,7 +298,7 @@ export class MetaService {
         });
       }
     } else {
-      if (!base_id) {
+      if (!base_id && base_id !== RootScopes.WORKSPACE) {
         NcError.metaError({
           message: 'Base ID is required',
           sql: '',
@@ -370,7 +371,7 @@ export class MetaService {
         });
       }
     } else {
-      if (!base_id) {
+      if (!base_id && base_id !== RootScopes.WORKSPACE) {
         NcError.metaError({
           message: 'Base ID is required',
           sql: '',
@@ -453,7 +454,7 @@ export class MetaService {
         });
       }
     } else {
-      if (!base_id) {
+      if (!base_id && base_id !== RootScopes.WORKSPACE) {
         NcError.metaError({
           message: 'Base ID is required',
           sql: '',
@@ -527,7 +528,7 @@ export class MetaService {
         });
       }
     } else {
-      if (!base_id) {
+      if (!base_id && base_id !== RootScopes.WORKSPACE) {
         NcError.metaError({
           message: 'Base ID is required',
           sql: '',
@@ -588,7 +589,7 @@ export class MetaService {
         });
       }
     } else {
-      if (!base_id) {
+      if (!base_id && base_id !== RootScopes.WORKSPACE) {
         NcError.metaError({
           message: 'Base ID is required',
           sql: '',

packages/nocodb/src/models/Integration.ts

@@ -237,8 +237,8 @@ export default class Integration implements IntegrationType {
       integration.meta = parseMetaProp(integration, 'meta');
     }
 
-    const integrations = integrationList?.map((baseData) => {
+    const integrations = integrationList?.map((integrationData) => {
-      return this.castType(baseData);
+      return this.castType(integrationData);
     });
 
     // if includeDatabaseInfo is true then get the database info for each integration
@@ -280,7 +280,7 @@ export default class Integration implements IntegrationType {
     force = false,
     ncMeta = Noco.ncMeta,
   ): Promise<Integration> {
-    const baseData = await ncMeta.metaGet2(
+    const integrationData = await ncMeta.metaGet2(
       context.workspace_id,
       context.workspace_id === RootScopes.BYPASS
         ? RootScopes.BYPASS
@@ -308,11 +308,11 @@ export default class Integration implements IntegrationType {
           },
     );
 
-    if (baseData) {
+    if (integrationData) {
-      baseData.meta = parseMetaProp(baseData, 'meta');
+      integrationData.meta = parseMetaProp(integrationData, 'meta');
     }
 
-    return this.castType(baseData);
+    return this.castType(integrationData);
   }
 
   public async getConnectionConfig(): Promise<any> {

packages/nocodb/src/services/integrations.service.ts

@@ -5,7 +5,7 @@ import type { NcContext, NcRequest } from '~/interface/config';
 import { AppHooksService } from '~/services/app-hooks/app-hooks.service';
 import { validatePayload } from '~/helpers';
 import { Base, Integration } from '~/models';
-import { NcError } from '~/helpers/catchError';
+import { NcBaseError, NcError } from '~/helpers/catchError'
 import { Source } from '~/models';
 import { CacheScope, MetaTable, RootScopes } from '~/utils/globals';
 import Noco from '~/Noco';
@@ -28,6 +28,10 @@ export class IntegrationsService {
   ) {
     const integration = await Integration.get(context, param.integrationId);
 
+    if (!integration) {
+      NcError.integrationNotFound(param.integrationId);
+    }
+
     integration.config = await integration.getConnectionConfig();
 
     if (param.includeSources) {
@@ -108,32 +112,26 @@ export class IntegrationsService {
         ncMeta,
       );
 
-      // check linked sources
+      if (!integration) {
-      const sources = await ncMeta.metaList2(
+        NcError.integrationNotFound(param.integrationId);
-        RootScopes.WORKSPACE,
+      }
-        RootScopes.WORKSPACE,
+
-        MetaTable.BASES,
+      // get linked sources
-        {
+      const sourceListQb = ncMeta
-          condition: {
+        .knex(MetaTable.BASES)
-            fk_workspace_id: integration.fk_workspace_id,
+        .where({
           fk_integration_id: integration.id,
-          },
+        })
-          xcCondition: {
+        .where((qb) => {
-            _or: [
+          qb.where('deleted', false).orWhere('deleted', null);
-              {
+        });
-                deleted: {
+
-                  eq: false,
+      if (integration.fk_workspace_id) {
-                },
+        sourceListQb.where('fk_workspace_id', integration.fk_workspace_id);
-              },
+      }
-              {
+
-                deleted: {
+      const sources: Pick<Source, 'id' | 'base_id'>[] =
-                  eq: null,
+        await sourceListQb.select('id', 'base_id');
-                },
-              },
-            ],
-          },
-        },
-      );
 
       if (sources.length > 0 && !param.force) {
         const bases = await Promise.all(
@@ -176,6 +174,7 @@ export class IntegrationsService {
       await ncMeta.commit();
     } catch (e) {
       await ncMeta.rollback(e);
+      if (e instanceof NcError || e instanceof NcBaseError) throw e;
       NcError.badRequest(e);
     }
     return true;
@@ -187,6 +186,9 @@ export class IntegrationsService {
   ) {
     try {
       const integration = await Integration.get(context, param.integrationId);
+      if (!integration) {
+        NcError.integrationNotFound(param.integrationId);
+      }
       await integration.softDelete();
     } catch (e) {
       NcError.badRequest(e);

packages/nocodb/src/utils/globals.ts

@@ -290,4 +290,6 @@ export const RootScopeTables = {
     MetaTable.AUDIT,
   ],
   [RootScopes.BASE]: [MetaTable.PROJECT],
+  // It's a special case and Workspace is equivalent to org in oss
+  [RootScopes.WORKSPACE]: [MetaTable.INTEGRATIONS],
 };

packages/nocodb/tests/unit/rest/index.test.ts

@@ -20,7 +20,7 @@ let ssoTest = () => {};
 let cloudOrgTest = () => {};
 let bulkAggregationTest = () => {};
 let columnTest = () => {};
-let integrationTest = () => {};
+let integrationTest = require('./tests/integration.test').default;
 if (process.env.EE === 'true') {
   workspaceTest = require('./tests/ee/workspace.test').default;
   ssoTest = require('./tests/ee/sso.test').default;

packages/nocodb/tests/unit/rest/tests/integration.test.ts

@@ -0,0 +1,243 @@
+import { expect } from 'chai'
+import 'mocha'
+import request from 'supertest'
+import { IntegrationsType } from 'nocodb-sdk'
+import { createProject } from '../../factory/base'
+import init from '../../init'
+
+function integrationTests() {
+  let context
+
+  beforeEach(async function() {
+    console.time('#### integrationTests')
+    context = await init()
+    await createProject(context)
+    console.timeEnd('#### integrationTests')
+  })
+
+  it('Create Integration', async () => {
+    const title = 'Sakila01'
+    const color = '#4351E8'
+
+    const response = await request(context.app)
+      .post(`/api/v2/meta/integrations`)
+      .set('xc-auth', context.token)
+      .send({
+        title,
+        meta: {
+          color,
+        },
+        config: {},
+        type: IntegrationsType.Database,
+      })
+      .expect(201)
+
+    expect(response.body).to.have.property('id')
+    expect(response.body).to.have.property('title')
+    expect(response.body).to.have.property('meta')
+    expect(response.body.meta).to.have.property('color')
+
+    if (response.body.title !== title || response.body.meta.color !== color) {
+      throw new Error('Integration creation failed')
+    }
+  })
+
+  it('List Integrations', async () => {
+    const title = 'Sakila01'
+    const color = '#4351E8'
+
+    await request(context.app)
+      .post(`/api/v2/meta/integrations`)
+      .set('xc-auth', context.token)
+      .send({
+        title,
+        meta: {
+          color,
+        },
+        config: {},
+        type: IntegrationsType.Database,
+      })
+      .expect(201)
+
+    const response = await request(context.app)
+      .get(`/api/v2/meta/integrations`)
+      .set('xc-auth', context.token)
+      .expect(200)
+
+    if (
+      response.body.list[0].title !== 'Sakila01' ||
+      response.body.list[0].meta.color !== '#4351E8'
+    ) {
+      throw new Error('Integration listing failed')
+    }
+  })
+
+  it('Delete Integration', async () => {
+    const title = 'Sakila01'
+    const color = '#4351E8'
+
+    const integration = await request(context.app)
+      .post(`/api/v2/meta/integrations`)
+      .set('xc-auth', context.token)
+      .send({
+        title,
+        meta: {
+          color,
+        },
+        config: {},
+        type: IntegrationsType.Database,
+      })
+      .expect(201)
+    await request(context.app)
+      .delete(`/api/v2/meta/integrations/${integration.body.id}`)
+      .set('xc-auth', context.token)
+      .expect(200)
+  })
+
+  it('Update Integration', async () => {
+    const title = 'Sakila01'
+    const color = '#4351E8'
+
+    const integration = await request(context.app)
+      .post(`/api/v2/meta/integrations`)
+      .set('xc-auth', context.token)
+      .send({
+        title,
+        meta: {
+          color,
+        },
+        config: {},
+        type: IntegrationsType.Database,
+      })
+      .expect(201)
+
+    await request(context.app)
+      .patch(`/api/v2/meta/integrations/${integration.body.id}`)
+      .set('xc-auth', context.token)
+      .send({ title: 'Sakila02', config: {}, type: IntegrationsType.Database })
+      .expect(200)
+
+    const response = await request(context.app)
+      .get(`/api/v2/meta/integrations/${integration.body.id}`)
+      .set('xc-auth', context.token)
+      .expect(200)
+
+    if (response.body.title !== 'Sakila02') {
+      throw new Error('Integration update failed')
+    }
+  })
+
+  it('Update Integration Error Test', async () => {
+    await request(context.app)
+      .patch(`/api/v2/meta/integrations/xxxxxxxxx`)
+      .set('xc-auth', context.token)
+      .send({ title: 'Sakila02', config: {}, type: IntegrationsType.Database })
+      .expect(404)
+  })
+
+  it('Delete Integration Error Test', async () => {
+    await request(context.app)
+      .delete(`/api/v2/meta/integrations/xxxxxxxxx`)
+      .set('xc-auth', context.token)
+      .expect(404)
+  })
+
+  it('Create Integration Error Test', async () => {
+    await request(context.app)
+      .post(`/api/v2/meta/integrations`)
+      .set('xc-auth', context.token)
+      .send()
+      .expect(400)
+  })
+
+  it('Create Integration Unauthorized User Test', async () => {
+    const title = 'Sakila01'
+    const color = '#4351E8'
+
+    await request(context.app)
+      .post(`/api/v2/meta/integrations`)
+      .send({
+        title,
+        meta: {
+          color,
+        },
+      })
+      .expect(401)
+  })
+
+  it('List Integration Unauthorized User Test', async () => {
+    await request(context.app)
+      .get(`/api/v2/meta/integrations`)
+      .expect(401)
+  })
+
+  it('Update Integration - where multiple sources are using the integration', async () => {
+    const title = 'Sakila01'
+    const color = '#4351E8'
+
+    const integration = await request(context.app)
+      .post(`/api/v2/meta/integrations`)
+      .set('xc-auth', context.token)
+      .send({
+        title,
+        meta: {
+          color,
+        },
+        config: {},
+        type: IntegrationsType.Database,
+      })
+      .expect(201)
+
+    await request(context.app)
+      .patch(`/api/v2/meta/integrations/${integration.body.id}`)
+      .set('xc-auth', context.token)
+      .send({ title: 'Sakila02', config: {}, type: IntegrationsType.Database })
+      .expect(200)
+
+    const response = await request(context.app)
+      .get(`/api/v2/meta/integrations/${integration.body.id}`)
+      .set('xc-auth', context.token)
+      .expect(200)
+
+    if (response.body.title !== 'Sakila02') {
+      throw new Error('Integration update failed')
+    }
+  })
+
+  it('Integration list based on user roles under workspace and base', async () => {
+    const title = 'Sakila01'
+    const color = '#4351E8'
+
+    const integration = await request(context.app)
+      .post(`/api/v2/meta/integrations`)
+      .set('xc-auth', context.token)
+      .send({
+        title,
+        meta: {
+          color,
+        },
+        config: {},
+        type: IntegrationsType.Database,
+      })
+      .expect(201)
+
+    await request(context.app)
+      .patch(`/api/v2/meta/integrations/${integration.body.id}`)
+      .set('xc-auth', context.token)
+      .send({ title: 'Sakila02', config: {}, type: IntegrationsType.Database })
+      .expect(200)
+
+    const response = await request(context.app)
+      .get(`/api/v2/meta/integrations/${integration.body.id}`)
+      .set('xc-auth', context.token)
+      .expect(200)
+
+    if (response.body.title !== 'Sakila02') {
+      throw new Error('Integration update failed')
+    }
+  })
+}
+
+export default function() {
+  describe('Integration', integrationTests)
+}