Repository: Yoctol/bottender
Pull request #64 (2017-12-02 17:22:39)

Fix CLI profile bug && add test for it
CLI will get not correct profile format for `client.setMessengerProfile` using.
```
✖ Failed to set messenger_profile settings
✖ status: 400
✖ data: {
  "error": {
    "message": "(#100) param persistent_menu must be an array.",
    "type": "OAuthException",
    "code": 100,
    "fbtrace_id": "HgrWBUgPhNR"
  }
}
```

Due to the `addedDiff`, `updatedDiff` return format is not what we want.
https://github.com/mattphillips/deep-object-diff#addeddiff
https://github.com/mattphillips/deep-object-diff#updateddiff

So we just make sure which key in profile need to be set and get it from `config.profile` for setting profile.
This PR will fix it and add more test!


Patch:
diff --git a/src/bot/ConsoleConnector.js b/src/bot/ConsoleConnector.js
index 8d677b30..78073511 100644
--- a/src/bot/ConsoleConnector.js
+++ b/src/bot/ConsoleConnector.js
@@ -1,7 +1,5 @@
-/*
-  eslint-disable class-methods-use-this
-  @flow
-*/
+/* @flow */
+
 import ConsoleContext from '../context/ConsoleContext';
 import ConsoleEvent, { type ConsoleRawEvent } from '../context/ConsoleEvent';
 import type { Session } from '../session/Session';
diff --git a/src/bot/LineConnector.js b/src/bot/LineConnector.js
index 52b999aa..1ba57f24 100644
--- a/src/bot/LineConnector.js
+++ b/src/bot/LineConnector.js
@@ -1,7 +1,5 @@
-/*
-  eslint-disable class-methods-use-this
-  @flow
-*/
+/* @flow */
+
 import { LineClient } from 'messaging-api-line';
 
 import LineContext from '../context/LineContext';
diff --git a/src/bot/MessengerConnector.js b/src/bot/MessengerConnector.js
index 7f5b4144..b8d60c44 100644
--- a/src/bot/MessengerConnector.js
+++ b/src/bot/MessengerConnector.js
@@ -1,7 +1,5 @@
-/*
-  eslint-disable class-methods-use-this
-  @flow
-*/
+/* @flow */
+
 import crypto from 'crypto';
 
 import { MessengerClient } from 'messaging-api-messenger';
diff --git a/src/bot/SlackConnector.js b/src/bot/SlackConnector.js
index 0d567860..186046b1 100644
--- a/src/bot/SlackConnector.js
+++ b/src/bot/SlackConnector.js
@@ -1,7 +1,5 @@
-/*
-  eslint-disable class-methods-use-this
-  @flow
-*/
+/* @flow */
+
 import { SlackOAuthClient } from 'messaging-api-slack';
 
 import SlackContext from '../context/SlackContext';
diff --git a/src/bot/TelegramConnector.js b/src/bot/TelegramConnector.js
index 8189dfa7..0a308b8c 100644
--- a/src/bot/TelegramConnector.js
+++ b/src/bot/TelegramConnector.js
@@ -1,7 +1,5 @@
-/*
-  eslint-disable class-methods-use-this
-  @flow
-*/
+/* @flow */
+
 import { TelegramClient } from 'messaging-api-telegram';
 
 import TelegramContext from '../context/TelegramContext';
diff --git a/src/cli/providers/line/menu.js b/src/cli/providers/line/menu.js
index f2dc7c14..6682489e 100644
--- a/src/cli/providers/line/menu.js
+++ b/src/cli/providers/line/menu.js
@@ -24,6 +24,7 @@ export const help = () => {
 
     ${chalk.dim('Actions:')}
 
+      check         Check if LINE rich menus setting in bottender.config.js valids
       get           Get the LINE rich menus
       set           Set the LINE rich menu by diff
       del, delete   Delete the LINE rich menus
@@ -45,6 +46,16 @@ export const help = () => {
   `);
 };
 
+export function checkLineMenu() {
+  try {
+    getConfig('bottender.config.js', 'line');
+    print('LINE rich menu check done.');
+  } catch (e) {
+    error(e.message);
+    return process.exit(1);
+  }
+}
+
 export async function getLineMenu() {
   try {
     const config = getConfig('bottender.config.js', 'line');
@@ -223,6 +234,9 @@ export async function deleteLineMenu(ctx) {
 export default async function main(ctx) {
   const subcommand = ctx.argv._[2];
   switch (subcommand) {
+    case 'check':
+      checkLineMenu();
+      break;
     case 'get':
       await getLineMenu();
       break;
@@ -237,7 +251,9 @@ export default async function main(ctx) {
       help();
       break;
     default:
-      error('Please specify a valid subcommand: get, set, delete, help.');
+      error(
+        'Please specify a valid subcommand: check, get, set, delete, help.'
+      );
       help();
   }
 }
diff --git a/src/cli/providers/messenger/profile.js b/src/cli/providers/messenger/profile.js
index 5e1e7b0d..ae501d41 100644
--- a/src/cli/providers/messenger/profile.js
+++ b/src/cli/providers/messenger/profile.js
@@ -24,6 +24,7 @@ export const help = () => {
 
     ${chalk.dim('Actions:')}
 
+      check         Check if messenger profile setting in bottender.config.js valids
       get           Get the messenger profile
       set           Set the messenger profile by diff
       del, delete   Delete all the messenger profile fields
@@ -55,6 +56,16 @@ export const trimDomain = profile => {
   return clone;
 };
 
+export function checkMessengerProfile() {
+  try {
+    getConfig('bottender.config.js', 'messenger');
+    print('Messenger profile check done.');
+  } catch (e) {
+    error(e.message);
+    return process.exit(1);
+  }
+}
+
 export async function getMessengerProfile() {
   try {
     const config = getConfig('bottender.config.js', 'messenger');
@@ -115,18 +126,28 @@ export async function setMessengerProfile(ctx) {
       const existedProfile = trimDomain(_existedProfile);
 
       const diffResult = diff(existedProfile, profile);
+
       if (Object.keys(diffResult).length !== 0) {
-        const shouldDeleteProfile = deletedDiff(existedProfile, profile);
-        const shouldSetProfile = {
-          ...addedDiff(existedProfile, profile),
-          ...updatedDiff(existedProfile, profile),
-        };
-        if (Object.keys(shouldDeleteProfile).length > 0) {
-          await client.deleteMessengerProfile(Object.keys(shouldDeleteProfile));
-          const deleteFileds = Object.keys(shouldDeleteProfile).join(', ');
+        const shouldDeleteFields = Object.keys(
+          deletedDiff(existedProfile, profile)
+        );
+
+        const shouldSetFields = [
+          ...Object.keys(addedDiff(existedProfile, profile)),
+          ...Object.keys(updatedDiff(existedProfile, profile)),
+        ];
+
+        if (shouldDeleteFields.length > 0) {
+          await client.deleteMessengerProfile(shouldDeleteFields);
+          const deleteFileds = shouldDeleteFields.join(', ');
           print(`Successfully delete ${bold(deleteFileds)} settings`);
         }
-        if (Object.keys(shouldSetProfile).length > 0) {
+
+        if (shouldSetFields.length > 0) {
+          const shouldSetProfile = {};
+          shouldSetFields.forEach(field => {
+            shouldSetProfile[field] = profile[field];
+          });
           await client.setMessengerProfile(shouldSetProfile);
           const setFields = Object.keys(shouldSetProfile).join(', ');
           print(`Successfully set ${bold(setFields)} settings`);
@@ -183,6 +204,9 @@ export async function deleteMessengerProfile() {
 export default async function main(ctx) {
   const subcommand = ctx.argv._[2];
   switch (subcommand) {
+    case 'check':
+      checkMessengerProfile();
+      break;
     case 'get':
       await getMessengerProfile();
       break;
@@ -197,7 +221,7 @@ export default async function main(ctx) {
       help();
       break;
     default:
-      error(`Please specify a valid subcommand: get, set, delete, help`);
+      error(`Please specify a valid subcommand: check, get, set, delete, help`);
       help();
   }
 }
diff --git a/src/cli/shared/schema.js b/src/cli/shared/schema.js
index 284c95c6..cb09e0d2 100644
--- a/src/cli/shared/schema.js
+++ b/src/cli/shared/schema.js
@@ -65,49 +65,51 @@ const schema = Joi.object().keys({
   line: Joi.object().keys({
     channelSecret: Joi.string().required(),
     accessToken: Joi.string().required(),
-    richMenus: Joi.array().items(
-      Joi.object().keys({
-        size: Joi.object()
-          .keys({
-            width: Joi.number()
-              .integer()
-              .required(),
-            height: Joi.number()
-              .integer()
-              .required(),
-          })
-          .required(),
-        selected: Joi.boolean().required(),
-        name: Joi.string().required(),
-        chatBarText: Joi.string().required(),
-        areas: Joi.array()
-          .items(
-            Joi.object().keys({
-              bounds: Joi.object()
-                .keys({
-                  x: Joi.number()
-                    .integer()
-                    .required(),
-                  y: Joi.number()
-                    .integer()
-                    .required(),
-                  width: Joi.number()
-                    .integer()
-                    .required(),
-                  height: Joi.number()
-                    .integer()
-                    .required(),
-                })
+    richMenus: Joi.array()
+      .items(
+        Joi.object().keys({
+          size: Joi.object()
+            .keys({
+              width: Joi.number()
+                .integer()
+                .required(),
+              height: Joi.number()
+                .integer()
                 .required(),
-              action: Joi.object().keys({
-                type: Joi.string().required(),
-                data: Joi.string().required(),
-              }),
             })
-          )
-          .required(),
-      })
-    ),
+            .required(),
+          selected: Joi.boolean().required(),
+          name: Joi.string().required(),
+          chatBarText: Joi.string().required(),
+          areas: Joi.array()
+            .items(
+              Joi.object().keys({
+                bounds: Joi.object()
+                  .keys({
+                    x: Joi.number()
+                      .integer()
+                      .required(),
+                    y: Joi.number()
+                      .integer()
+                      .required(),
+                    width: Joi.number()
+                      .integer()
+                      .required(),
+                    height: Joi.number()
+                      .integer()
+                      .required(),
+                  })
+                  .required(),
+                action: Joi.object().keys({
+                  type: Joi.string().required(),
+                  data: Joi.string().required(),
+                }),
+              })
+            )
+            .required(),
+        })
+      )
+      .max(10),
   }),
   telegram: Joi.object().keys({
     accessToken: Joi.string().required(),


Test patch:
diff --git a/src/cli/providers/line/__tests__/checkLineMenu.spec.js b/src/cli/providers/line/__tests__/checkLineMenu.spec.js
new file mode 100644
index 00000000..79c72d7d
--- /dev/null
+++ b/src/cli/providers/line/__tests__/checkLineMenu.spec.js
@@ -0,0 +1,75 @@
+import { checkLineMenu } from '../menu';
+
+jest.mock('../../../shared/log');
+jest.mock('../../../shared/getConfig');
+
+const log = require('../../../shared/log');
+const getConfig = require('../../../shared/getConfig');
+
+const MOCK_FILE_WITH_PLATFORM = {
+  messenger: {
+    accessToken: '__FAKE_TOKEN__',
+  },
+  line: {
+    accessToken: '__FAKE_TOKEN__',
+    richMenus: [
+      {
+        size: {
+          width: 2500,
+          height: 1686,
+        },
+        selected: false,
+        name: 'Nice richmenu',
+        chatBarText: 'Tap here',
+        areas: [
+          {
+            bounds: {
+              x: 0,
+              y: 0,
+              width: 2500,
+              height: 1686,
+            },
+            action: {
+              type: 'postback',
+              data: 'action=buy&itemid=123',
+            },
+          },
+        ],
+      },
+    ],
+  },
+};
+
+beforeEach(() => {
+  log.error = jest.fn();
+  log.print = jest.fn();
+});
+
+it('be defined', () => {
+  expect(checkLineMenu).toBeDefined();
+});
+
+describe('resolved', () => {
+  it('call getConfig', async () => {
+    getConfig.mockReturnValue(MOCK_FILE_WITH_PLATFORM.line);
+
+    checkLineMenu();
+
+    expect(log.print).toBeCalledWith('LINE rich menu check done.');
+  });
+});
+
+describe('reject', () => {
+  it('handle error', async () => {
+    getConfig.mockImplementation(() => {
+      throw new Error('error');
+    });
+
+    process.exit = jest.fn();
+
+    checkLineMenu();
+
+    expect(log.error).toBeCalledWith('error');
+    expect(process.exit).toBeCalledWith(1);
+  });
+});
diff --git a/src/cli/providers/messenger/__tests__/checkMessengerProfile.spec.js b/src/cli/providers/messenger/__tests__/checkMessengerProfile.spec.js
new file mode 100644
index 00000000..df2a96b5
--- /dev/null
+++ b/src/cli/providers/messenger/__tests__/checkMessengerProfile.spec.js
@@ -0,0 +1,48 @@
+import { checkMessengerProfile } from '../profile';
+
+jest.mock('../../../shared/log');
+jest.mock('../../../shared/getConfig');
+
+const log = require('../../../shared/log');
+const getConfig = require('../../../shared/getConfig');
+
+const MOCK_FILE_WITH_PLATFORM = {
+  messenger: {
+    accessToken: '__FAKE_TOKEN__',
+  },
+  line: {},
+};
+
+beforeEach(() => {
+  log.error = jest.fn();
+  log.print = jest.fn();
+});
+
+it('be defined', () => {
+  expect(checkMessengerProfile).toBeDefined();
+});
+
+describe('resolved', () => {
+  it('call getConfig', async () => {
+    getConfig.mockReturnValue(MOCK_FILE_WITH_PLATFORM.messenger);
+
+    checkMessengerProfile();
+
+    expect(log.print).toBeCalledWith('Messenger profile check done.');
+  });
+});
+
+describe('reject', () => {
+  it('handle error', async () => {
+    getConfig.mockImplementation(() => {
+      throw new Error('error');
+    });
+
+    process.exit = jest.fn();
+
+    checkMessengerProfile();
+
+    expect(log.error).toBeCalledWith('error');
+    expect(process.exit).toBeCalledWith(1);
+  });
+});
diff --git a/src/cli/providers/messenger/__tests__/profile.spec.js b/src/cli/providers/messenger/__tests__/profile.spec.js
new file mode 100644
index 00000000..268bf6fe
--- /dev/null
+++ b/src/cli/providers/messenger/__tests__/profile.spec.js
@@ -0,0 +1,20 @@
+import { trimDomain } from '../profile';
+
+it('should be defined', () => {
+  expect(trimDomain).toBeDefined();
+});
+
+describe('#trimDomain', () => {
+  it('should remove whitelisted_domains end slash', () => {
+    const testProfile = {
+      whitelisted_domains: [
+        'https://www.facebook.com/',
+        'https://facebook.com',
+      ],
+    };
+    const trimProfile = trimDomain(testProfile);
+    expect(trimProfile).toEqual({
+      whitelisted_domains: ['https://www.facebook.com', 'https://facebook.com'],
+    });
+  });
+});
diff --git a/src/cli/providers/messenger/__tests__/setMessengerProfile.spec.js b/src/cli/providers/messenger/__tests__/setMessengerProfile.spec.js
index e7e539fc..22a78d44 100644
--- a/src/cli/providers/messenger/__tests__/setMessengerProfile.spec.js
+++ b/src/cli/providers/messenger/__tests__/setMessengerProfile.spec.js
@@ -112,10 +112,56 @@ describe('resolve', () => {
       ],
     });
   });
-  it('successfully set diff fileds `get_started`', async () => {
+
+  it('successfully set diff fields `persistent_menu`', async () => {
     const ctx = {
       argv: {},
     };
+    _client.getMessengerProfile.mockReturnValue([
+      {
+        persistent_menu: [
+          {
+            locale: 'default',
+            composer_input_disabled: false,
+            call_to_actions: [
+              {
+                type: 'web_url',
+                title: 'Powered by Facebook',
+                url: 'https://www.facebook.com/',
+              },
+            ],
+          },
+        ],
+      },
+    ]);
+
+    await setMessengerProfile(ctx);
+
+    expect(_client.setMessengerProfile).toBeCalledWith({
+      get_started: {
+        payload: '<GET_STARTED_PAYLOAD>',
+      },
+      persistent_menu: [
+        {
+          locale: 'default',
+          composer_input_disabled: false,
+          call_to_actions: [
+            {
+              type: 'web_url',
+              title: 'Powered by Example',
+              url: 'https://www.example.com/',
+            },
+          ],
+        },
+      ],
+    });
+  });
+
+  it('successfully set diff fields `get_started`', async () => {
+    const ctx = {
+      argv: {},
+    };
+
     await setMessengerProfile(ctx);
 
     expect(_client.setMessengerProfile).toBeCalledWith({
@@ -124,7 +170,8 @@ describe('resolve', () => {
       },
     });
   });
-  it('successfully delete diff fileds `whitelisted_domains`', async () => {
+
+  it('successfully delete diff fields `whitelisted_domains`', async () => {
     _client.getMessengerProfile.mockReturnValue([
       {
         persistent_menu: [
@@ -149,11 +196,14 @@ describe('resolve', () => {
     const ctx = {
       argv: {},
     };
+
     await setMessengerProfile(ctx);
+
     expect(_client.deleteMessengerProfile).toBeCalledWith([
       'whitelisted_domains',
     ]);
   });
+
   it('do nothing and log info', async () => {
     _client.getMessengerProfile.mockReturnValue([
       {
@@ -178,7 +228,9 @@ describe('resolve', () => {
     const ctx = {
       argv: {},
     };
+
     await setMessengerProfile(ctx);
+
     expect(_client.deleteMessengerProfile).not.toBeCalled();
     expect(_client.setMessengerProfile).not.toBeCalled();
     expect(log.print).toHaveBeenCalledWith(
diff --git a/src/context/__tests__/TelegramContext.spec.js b/src/context/__tests__/TelegramContext.spec.js
index 5fda3360..ca558dba 100644
--- a/src/context/__tests__/TelegramContext.spec.js
+++ b/src/context/__tests__/TelegramContext.spec.js
@@ -1,5 +1,5 @@
 jest.mock('delay');
-jest.mock('messaging-api-messenger');
+jest.mock('messaging-api-telegram');
 jest.mock('warning');
 
 let TelegramContext;