fix(condo) DOMA-7397: fix review comments and introduce AJV to test r…

…esponse schema
open-condo-software · Mar 13, 2024 · 604380f · 604380f
1 parent ddb4f12
commit 604380f
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 35 deletions.
diff --git a/apps/condo/domains/news/access/GetNewsSharingRecipientsService.js b/apps/condo/domains/news/access/GetNewsSharingRecipientsService.js
@@ -19,7 +19,7 @@ async function canGetNewsSharingRecipients ({ authentication: { item: user }, ar
     const organizationId = get(b2bContext, 'organization', null)
     if (!organizationId) { return false }
 
-    return await checkPermissionInUserOrganizationOrRelatedOrganization(user.id, organizationId, 'canReadNewsItems')
+    return await checkPermissionInUserOrganizationOrRelatedOrganization(user.id, organizationId, 'canManageNewsItems')
 }
 
 /*

diff --git a/apps/condo/domains/news/gql.js b/apps/condo/domains/news/gql.js
@@ -38,7 +38,7 @@ const NewsItemSharing = generateGqlQueries('NewsItemSharing', NEWS_ITEM_SHARING_
 
 const GET_NEWS_SHARING_RECIPIENTS_MUTATION = gql`
     query getGetNewsSharingRecipients ($data: GetNewsSharingRecipientsInput!) {
-        result: getNewsSharingRecipients(data: $data) { id name recipients }
+        result: getNewsSharingRecipients(data: $data) { id name receiversCount }
     }
 `
 

diff --git a/apps/condo/domains/news/schema/GetNewsSharingRecipientsService.js b/apps/condo/domains/news/schema/GetNewsSharingRecipientsService.js
@@ -34,6 +34,7 @@
  * <Condo /news> -> <Condo.GetNewsSharingRecipientsService> -> <Miniapp.GetRecipients>
  */
 
+const Ajv = require('ajv')
 const fetch = require('node-fetch')
 
 const { GQLError, GQLErrorCode: { BAD_USER_INPUT, INTERNAL_ERROR } } = require('@open-condo/keystone/errors')
@@ -42,6 +43,23 @@ const { GQLCustomSchema, getById } = require('@open-condo/keystone/schema')
 const { WRONG_VALUE, NETWORK_ERROR } = require('@condo/domains/common/constants/errors')
 const access = require('@condo/domains/news/access/GetNewsSharingRecipientsService')
 
+const SCHEMA = {
+    'type': 'array',
+    'items': {
+        'type': 'object',
+        'properties': {
+            'id': { 'type': 'string' },
+            'name': { 'type': 'string' },
+            'receiversCount': { 'type': 'number' },
+        },
+        'required': ['id', 'name'],
+        'additionalProperties': false,
+    },
+}
+
+const ajv = new Ajv()
+const validateSchema = ajv.compile(SCHEMA)
+
 async function fetchWithTimeout (url, options = {}, timeout = 5000) {
     const controller = new AbortController()
     const timeoutId = setTimeout(() => controller.abort(), timeout)
@@ -96,7 +114,7 @@ const GetNewsSharingRecipientsService = new GQLCustomSchema('GetNewsSharingRecip
         },
         {
             access: true,
-            type: 'type GetNewsSharingRecipientsOutput { id: String!, name: String!, recipients: Int }',
+            type: 'type GetNewsSharingRecipientsOutput { id: String!, name: String!, receiversCount: Int }',
         },
     ],
 
@@ -141,21 +159,17 @@ const GetNewsSharingRecipientsService = new GQLCustomSchema('GetNewsSharingRecip
                 }
 
                 // Check that result data is in good shape
-                const getRecipientsResultData = await getRecipientsResult.json()
+                let getRecipientsResultData
 
-                if (!getRecipientsResultData || !Array.isArray(getRecipientsResultData)) {
+                try {
+                    getRecipientsResultData = await getRecipientsResult.json()
+                } catch (err) {
                     throw new GQLError(ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE)
                 }
 
-                getRecipientsResultData.forEach(x => {
-                    if (
-                        (typeof x.id !== 'string') ||
-                        (typeof x.name !== 'string') ||
-                        (x.recipients && typeof x.recipients !== 'number')
-                    ) {
-                        throw new GQLError(ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE)
-                    }
-                })
+                if (!validateSchema(getRecipientsResultData)) {
+                    throw new GQLError(ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE)
+                }
 
                 return getRecipientsResultData
             },

diff --git a/apps/condo/domains/news/schema/GetNewsSharingRecipientsService.test.js b/apps/condo/domains/news/schema/GetNewsSharingRecipientsService.test.js
@@ -2,7 +2,6 @@
  * Generated by `createservice news.GetNewsItemSharingRecipientsService --type queries`
  */
 
-const conf = require('@open-condo/config')
 const { makeLoggedInAdminClient, makeClient, expectToThrowAuthenticationError, expectToThrowAccessDeniedErrorToResult, expectToThrowGQLError, initTestExpressApp, getTestExpressApp } = require('@open-condo/keystone/test.utils')
 
 const {
@@ -13,18 +12,22 @@ const {
 const { ERRORS } = require('@condo/domains/news/schema/GetNewsSharingRecipientsService')
 const { getNewsSharingRecipientsByTestClient } = require('@condo/domains/news/utils/testSchema')
 const { createTestOrganization } = require('@condo/domains/organization/utils/testSchema')
-
-
 const {
     createTestOrganizationEmployeeRole,
     createTestOrganizationEmployee,
-} = require('../../organization/utils/testSchema')
-const { makeClientWithNewRegisteredAndLoggedInUser } = require('../../user/utils/testSchema')
+} = require('@condo/domains/organization/utils/testSchema')
+const { makeClientWithNewRegisteredAndLoggedInUser } = require('@condo/domains/user/utils/testSchema')
+
 const { 
     SUCCESS_GET_RECIPIENTS_URL,
     FAULTY_GET_RECIPIENTS_URL_404,
     FAULTY_GET_RECIPIENTS_URL_500,
-    INCORRECT_GET_RECIPIENTS_URL,
+
+    INCORRECT_GET_RECIPIENTS_RESULT_URL_WRONG_RETURN_TYPE,
+    INCORRECT_GET_RECIPIENTS_URL_BAD_ID,
+    INCORRECT_GET_RECIPIENTS_URL_BAD_NAME,
+    INCORRECT_GET_RECIPIENTS_URL_OTHER_FIELDS,
+
     SUCCESS_PREVIEW_URL,
     SUCCESS_PUBLISH_URL,
     SUCCESS_GET_RECIPIENTS_RESULT,
@@ -48,7 +51,7 @@ describe('GetNewsSharingRecipientsService', () => {
         dummyO10n = o10n
 
         staffClientWithPermissions = await makeClientWithNewRegisteredAndLoggedInUser()
-        const [roleYes] = await createTestOrganizationEmployeeRole(adminClient, o10n, { canReadNewsItems: true })
+        const [roleYes] = await createTestOrganizationEmployeeRole(adminClient, o10n, { canManageNewsItems: true })
         await createTestOrganizationEmployee(adminClient, o10n, staffClientWithPermissions.user, roleYes)
 
         const [B2BAppNewsSharingConfig] = await createTestB2BAppNewsSharingConfig(adminClient, {
@@ -122,9 +125,63 @@ describe('GetNewsSharingRecipientsService', () => {
         }, 'result')
     })
 
-    test('fails if remote server returns wrong data', async () => {
+    test('fails if remote server returns wrong data (bad type of id field)', async () => {
+        const [B2BAppFailingNewsSharingConfig] = await createTestB2BAppNewsSharingConfig(adminClient, {
+            getRecipientsUrl: `${testExpressAppBaseUrl}${INCORRECT_GET_RECIPIENTS_URL_BAD_ID}`,
+            previewUrl: `${testExpressAppBaseUrl}${SUCCESS_PREVIEW_URL}`,
+            publishUrl: `${testExpressAppBaseUrl}${SUCCESS_PUBLISH_URL}`,
+        })
+        const [B2BApp] = await createTestB2BApp(adminClient, { newsSharingConfig: { connect: { id: B2BAppFailingNewsSharingConfig.id } } })
+        const [B2BContext] = await createTestB2BAppContext(adminClient, B2BApp, dummyO10n)
+
+        await expectToThrowGQLError(async () => {
+            await getNewsSharingRecipientsByTestClient(staffClientWithPermissions, B2BContext)
+        }, {
+            type: ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE.type,
+            code: ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE.code,
+            message: ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE.message,
+        }, 'result')
+    })
+
+    test('fails if remote server returns wrong data (bad type of name field)', async () => {
+        const [B2BAppFailingNewsSharingConfig] = await createTestB2BAppNewsSharingConfig(adminClient, {
+            getRecipientsUrl: `${testExpressAppBaseUrl}${INCORRECT_GET_RECIPIENTS_URL_BAD_NAME}`,
+            previewUrl: `${testExpressAppBaseUrl}${SUCCESS_PREVIEW_URL}`,
+            publishUrl: `${testExpressAppBaseUrl}${SUCCESS_PUBLISH_URL}`,
+        })
+        const [B2BApp] = await createTestB2BApp(adminClient, { newsSharingConfig: { connect: { id: B2BAppFailingNewsSharingConfig.id } } })
+        const [B2BContext] = await createTestB2BAppContext(adminClient, B2BApp, dummyO10n)
+
+        await expectToThrowGQLError(async () => {
+            await getNewsSharingRecipientsByTestClient(staffClientWithPermissions, B2BContext)
+        }, {
+            type: ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE.type,
+            code: ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE.code,
+            message: ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE.message,
+        }, 'result')
+    })
+
+    test('fails if remote server returns wrong data (wrong data type)', async () => {
+        const [B2BAppFailingNewsSharingConfig] = await createTestB2BAppNewsSharingConfig(adminClient, {
+            getRecipientsUrl: `${testExpressAppBaseUrl}${INCORRECT_GET_RECIPIENTS_RESULT_URL_WRONG_RETURN_TYPE}`,
+            previewUrl: `${testExpressAppBaseUrl}${SUCCESS_PREVIEW_URL}`,
+            publishUrl: `${testExpressAppBaseUrl}${SUCCESS_PUBLISH_URL}`,
+        })
+        const [B2BApp] = await createTestB2BApp(adminClient, { newsSharingConfig: { connect: { id: B2BAppFailingNewsSharingConfig.id } } })
+        const [B2BContext] = await createTestB2BAppContext(adminClient, B2BApp, dummyO10n)
+
+        await expectToThrowGQLError(async () => {
+            await getNewsSharingRecipientsByTestClient(staffClientWithPermissions, B2BContext)
+        }, {
+            type: ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE.type,
+            code: ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE.code,
+            message: ERRORS.NEWS_SHARING_APP_REQUEST_BAD_RESPONSE.message,
+        }, 'result')
+    })
+
+    test('fails if remote server returns wrong data (additional fields)', async () => {
         const [B2BAppFailingNewsSharingConfig] = await createTestB2BAppNewsSharingConfig(adminClient, {
-            getRecipientsUrl: `${testExpressAppBaseUrl}${INCORRECT_GET_RECIPIENTS_URL}`,
+            getRecipientsUrl: `${testExpressAppBaseUrl}${INCORRECT_GET_RECIPIENTS_URL_OTHER_FIELDS}`,
             previewUrl: `${testExpressAppBaseUrl}${SUCCESS_PREVIEW_URL}`,
             publishUrl: `${testExpressAppBaseUrl}${SUCCESS_PUBLISH_URL}`,
         })

diff --git a/apps/condo/domains/news/utils/testSchema/NewsSharingTestingApp.js b/apps/condo/domains/news/utils/testSchema/NewsSharingTestingApp.js
@@ -6,36 +6,89 @@ const SUCCESS_PREVIEW_URL = "/test/news-sharing-api/success/preview"
 const FAULTY_GET_RECIPIENTS_URL_404 = '/test/news-sharing-api/fail/getRecipients/404'
 const FAULTY_GET_RECIPIENTS_URL_500 = '/test/news-sharing-api/fail/getRecipients/500'
 
-const INCORRECT_GET_RECIPIENTS_URL = '/test/news-sharing-api/fail/getRecipients'
+const INCORRECT_GET_RECIPIENTS_RESULT_URL_WRONG_RETURN_TYPE = '/test/news-sharing-api/fail/getRecipients/0'
+const INCORRECT_GET_RECIPIENTS_URL_BAD_NAME = '/test/news-sharing-api/fail/getRecipients/1'
+const INCORRECT_GET_RECIPIENTS_URL_BAD_ID = '/test/news-sharing-api/fail/getRecipients/2'
+const INCORRECT_GET_RECIPIENTS_URL_OTHER_FIELDS = '/test/news-sharing-api/fail/getRecipients/3'
 
 const SUCCESS_GET_RECIPIENTS_RESULT = [
     {
         id: '1231-2312-3331-1231',
         name: 'Mayview house chat',
-        recipients: 120,
+        receiversCount: 120,
     },
     {
         id: '5231-2312-3331-1233',
         name: 'Bayview house chat',
-        recipients: 990,
+        receiversCount: 990,
     },
     {
         id: '5231-2312-3331-1233',
         name: 'Bayview house chat',
     },
 ]
 
+const INCORRECT_GET_RECIPIENTS_RESULT_WRONG_RETURN_TYPE = {}
 
-const INCORRECT_GET_RECIPIENTS_RESULT = [
+const INCORRECT_GET_RECIPIENTS_RESULT_BAD_NAME = [
     {
-        // Does not have name
+        // Name is bad
+        name: {},
         id: '1231-2312-3331-1231',
-        recipients: 120,
+        receiversCount: 120,
+    },
+    {
+        // Correct
+        id: '2313',
+        name: 'Bayview house chat',
+        receiversCount: 990,
+    },
+    {
+        // Correct
+        id: '2313',
+        name: 'Bayview house chat',
+        receiversCount: 990,
+    },
+]
+
+const INCORRECT_GET_RECIPIENTS_RESULT_BAD_ID = [
+    {
+        // Correct
+        id: '2313',
+        name: 'Bayview house chat',
+        receiversCount: 990,
     },
     {
         // Does not have ID
         name: 'Bayview house chat',
-        recipients: 990,
+        receiversCount: 990,
+    },
+    {
+        // Correct
+        id: '2313',
+        name: 'Bayview house chat',
+        receiversCount: 990,
+    },
+]
+
+const INCORRECT_GET_RECIPIENTS_RESULT_OTHER_FIELDS = [
+    {
+        // Correct
+        id: '2313',
+        name: 'Bayview house chat',
+        receiversCount: 990,
+    },
+    {
+        // Correct
+        id: '2313',
+        name: 'Bayview house chat',
+        receiversCount: 990,
+    },
+    {
+        // Has other fields
+        name: 'Bayview house chat',
+        receiversCount: 990,
+        recipients: '{}',
     },
 ]
 
@@ -58,8 +111,20 @@ class NewsSharingTestingApp {
             res.status(200).send('OK')
         })
 
-        app.get(INCORRECT_GET_RECIPIENTS_URL, async (req, res, next) => {
-            res.status(200).send(INCORRECT_GET_RECIPIENTS_RESULT)
+        app.get(INCORRECT_GET_RECIPIENTS_URL_BAD_ID, async (req, res, next) => {
+            res.status(200).send(INCORRECT_GET_RECIPIENTS_RESULT_BAD_ID)
+        })
+
+        app.get(INCORRECT_GET_RECIPIENTS_URL_BAD_NAME, async (req, res, next) => {
+            res.status(200).send(INCORRECT_GET_RECIPIENTS_RESULT_BAD_NAME)
+        })
+
+        app.get(INCORRECT_GET_RECIPIENTS_URL_OTHER_FIELDS, async (req, res, next) => {
+            res.status(200).send(INCORRECT_GET_RECIPIENTS_RESULT_OTHER_FIELDS)
+        })
+
+        app.get(INCORRECT_GET_RECIPIENTS_RESULT_URL_WRONG_RETURN_TYPE, async (req, res, next) => {
+            res.status(200).send(INCORRECT_GET_RECIPIENTS_RESULT_WRONG_RETURN_TYPE)
         })
 
         app.get(FAULTY_GET_RECIPIENTS_URL_500, async (req, res, next) => {
@@ -82,8 +147,16 @@ module.exports = {
     SUCCESS_PREVIEW_URL,
     FAULTY_GET_RECIPIENTS_URL_404,
     FAULTY_GET_RECIPIENTS_URL_500,
-    INCORRECT_GET_RECIPIENTS_URL,
+
+    INCORRECT_GET_RECIPIENTS_RESULT_URL_WRONG_RETURN_TYPE,
+    INCORRECT_GET_RECIPIENTS_URL_BAD_NAME,
+    INCORRECT_GET_RECIPIENTS_URL_BAD_ID,
+    INCORRECT_GET_RECIPIENTS_URL_OTHER_FIELDS,
 
     SUCCESS_GET_RECIPIENTS_RESULT,
-    INCORRECT_GET_RECIPIENTS_RESULT,
+
+    INCORRECT_GET_RECIPIENTS_RESULT_WRONG_RETURN_TYPE,
+    INCORRECT_GET_RECIPIENTS_RESULT_BAD_NAME,
+    INCORRECT_GET_RECIPIENTS_RESULT_BAD_ID,
+    INCORRECT_GET_RECIPIENTS_RESULT_OTHER_FIELDS,
 }
diff --git a/apps/condo/index.js b/apps/condo/index.js
@@ -23,7 +23,6 @@ const { getWebhookModels } = require('@open-condo/webhooks/schema')
 const { PaymentLinkMiddleware } = require('@condo/domains/acquiring/PaymentLinkMiddleware')
 const FileAdapter = require('@condo/domains/common/utils/fileAdapter')
 const { VersioningMiddleware } = require('@condo/domains/common/utils/VersioningMiddleware')
-const { NewsSharingTestingApp } = require('@condo/domains/news/utils/testSchema/NewsSharingTestingApp')
 const { UnsubscribeMiddleware } = require('@condo/domains/notification/UnsubscribeMiddleware')
 const { UserExternalIdentityMiddleware } = require('@condo/domains/user/integration/UserExternalIdentityMiddleware')
 const { OIDCMiddleware } = require('@condo/domains/user/oidc')