Merge pull request #69 from zapier/non-required-label

fix(schema): Make label & description optional for hidden actions
zapier · Sep 24, 2019 · 8328a0d · 8328a0d
2 parents b4ac15e + 8c9c2f0
commit 8328a0d
Show file tree

Hide file tree

Showing 7 changed files with 153 additions and 24 deletions.
diff --git a/packages/schema/docs/build/schema.md b/packages/schema/docs/build/schema.md
@@ -346,15 +346,14 @@ Represents user information for a trigger, search, or create.
 
 #### Anti-Examples
 
-* `{ label: 'New Thing' }`
 * `{ label: 'New Thing', description: 'Gets a new thing for you.', important: 1 }`
 
 #### Properties
 
 Key | Required | Type | Description
 --- | -------- | ---- | -----------
-`label` | **yes** | `string` | A short label like "New Record" or "Create Record in Project".
-`description` | **yes** | `string` | A description of what this trigger, search, or create does.
+`label` | **yes** (with exceptions, see description) | `string` | A short label like "New Record" or "Create Record in Project". Optional if `hidden` is true.
+`description` | **yes** (with exceptions, see description) | `string` | A description of what this trigger, search, or create does. Optional if `hidden` is true.
 `directions` | no | `string` | A short blurb that can explain how to get this working. EG: how and where to copy-paste a static hook URL into your application. Only evaluated for static webhooks.
 `important` | no | `boolean` | Affects how prominently this operation is displayed in the UI. Only mark a few of the most popular operations important.
 `hidden` | no | `boolean` | Should this operation be unselectable by users?

diff --git a/packages/schema/exported-schema.json b/packages/schema/exported-schema.json
@@ -581,19 +581,30 @@
       "id": "/BasicDisplaySchema",
       "description": "Represents user information for a trigger, search, or create.",
       "type": "object",
-      "required": ["label", "description"],
       "properties": {
         "label": {
-          "description": "A short label like \"New Record\" or \"Create Record in Project\".",
+          "description": "A short label like \"New Record\" or \"Create Record in Project\". Optional if `hidden` is true.",
           "type": "string",
           "minLength": 2,
-          "maxLength": 64
+          "maxLength": 64,
+          "docAnnotation": {
+            "required": {
+              "type": "replace",
+              "value": "**yes** (with exceptions, see description)"
+            }
+          }
         },
         "description": {
-          "description": "A description of what this trigger, search, or create does.",
+          "description": "A description of what this trigger, search, or create does. Optional if `hidden` is true.",
           "type": "string",
           "minLength": 1,
-          "maxLength": 1000
+          "maxLength": 1000,
+          "docAnnotation": {
+            "required": {
+              "type": "replace",
+              "value": "**yes** (with exceptions, see description)"
+            }
+          }
         },
         "directions": {
           "description": "A short blurb that can explain how to get this working. EG: how and where to copy-paste a static hook URL into your application. Only evaluated for static webhooks.",

diff --git a/packages/schema/lib/functional-constraints/index.js b/packages/schema/lib/functional-constraints/index.js
@@ -9,13 +9,14 @@
  *     name, // optional, the validation type that failed. Can make something up like 'invalidUrl'
  *     argument // optional
  *   );
-*/
+ */
 const checks = [
   require('./searchOrCreateKeys'),
   require('./deepNestedFields'),
   require('./mutuallyExclusiveFields'),
   require('./requiredSamples'),
-  require('./matchingKeys')
+  require('./matchingKeys'),
+  require('./labelWhenVisible')
 ];
 
 const runFunctionalConstraints = (definition, mainSchema) => {

diff --git a/packages/schema/lib/functional-constraints/labelWhenVisible.js b/packages/schema/lib/functional-constraints/labelWhenVisible.js
@@ -0,0 +1,31 @@
+const _ = require('lodash');
+const jsonschema = require('jsonschema');
+
+const actionTypes = ['triggers', 'searches', 'creates'];
+
+const labelWhenVisible = definition => {
+  const errors = [];
+
+  for (const actionType of actionTypes) {
+    const group = definition[actionType] || {};
+    _.each(group, (action, key) => {
+      const { display } = action;
+      if (!display.hidden && !(display.label && display.description)) {
+        errors.push(
+          new jsonschema.ValidationError(
+            `visible actions must have a label and description`,
+            action,
+            `/BasicDisplaySchema`,
+            `instance.${actionType}.${key}.display`,
+            'invalid',
+            'key'
+          )
+        );
+      }
+    });
+  }
+
+  return errors;
+};
+
+module.exports = labelWhenVisible;
diff --git a/packages/schema/lib/schemas/BasicDisplaySchema.js b/packages/schema/lib/schemas/BasicDisplaySchema.js
@@ -17,28 +17,38 @@ module.exports = makeSchema({
     }
   ],
   antiExamples: [
-    { label: 'New Thing' },
     {
       label: 'New Thing',
       description: 'Gets a new thing for you.',
       important: 1
     }
   ],
-  required: ['label', 'description'],
   properties: {
     label: {
       description:
-        'A short label like "New Record" or "Create Record in Project".',
+        'A short label like "New Record" or "Create Record in Project". Optional if `hidden` is true.',
       type: 'string',
       minLength: 2,
-      maxLength: 64
+      maxLength: 64,
+      docAnnotation: {
+        required: {
+          type: 'replace',
+          value: '**yes** (with exceptions, see description)'
+        }
+      }
     },
     description: {
       description:
-        'A description of what this trigger, search, or create does.',
+        'A description of what this trigger, search, or create does. Optional if `hidden` is true.',
       type: 'string',
       minLength: 1,
-      maxLength: 1000
+      maxLength: 1000,
+      docAnnotation: {
+        required: {
+          type: 'replace',
+          value: '**yes** (with exceptions, see description)'
+        }
+      }
     },
     directions: {
       description:

diff --git a/packages/schema/test/functional-constraints/labelWhenVisible.js b/packages/schema/test/functional-constraints/labelWhenVisible.js
@@ -0,0 +1,73 @@
+const should = require('should');
+const schema = require('../../schema');
+
+describe('labelWhenVisible', () => {
+  it('should not error when label is gone and action is hidden', () => {
+    const definition = {
+      version: '1.0.0',
+      platformVersion: '1.0.0',
+      creates: {
+        foo: {
+          key: 'foo',
+          noun: 'Foo',
+          display: {
+            hidden: true
+          },
+          operation: {
+            perform: '$func$2$f$'
+          }
+        }
+      }
+    };
+
+    const results = schema.validateAppDefinition(definition);
+    results.errors.should.have.length(0);
+  });
+
+  it('should error when label is missing and action is visible ', () => {
+    const definition = {
+      version: '1.0.0',
+      platformVersion: '1.0.0',
+      creates: {
+        foo: {
+          key: 'foo',
+          noun: 'Foo',
+          display: {
+            hidden: false
+          },
+          operation: {
+            perform: '$func$2$f$'
+          }
+        }
+      }
+    };
+
+    const results = schema.validateAppDefinition(definition);
+    results.errors.should.have.length(1);
+    const err = results.errors[0];
+    should(err.codeLinks[0].includes('BasicDisplaySchema')).be.true();
+    err.property.should.eql('instance.creates.foo.display');
+  });
+
+  it('should error when label is alone when create is visible ', () => {
+    const definition = {
+      version: '1.0.0',
+      platformVersion: '1.0.0',
+      creates: {
+        foo: {
+          key: 'foo',
+          noun: 'Foo',
+          display: {
+            label: 'nea'
+          },
+          operation: {
+            perform: '$func$2$f$'
+          }
+        }
+      }
+    };
+
+    const results = schema.validateAppDefinition(definition);
+    results.errors.should.have.length(1);
+  });
+});
diff --git a/packages/schema/test/utils.js b/packages/schema/test/utils.js
@@ -13,21 +13,25 @@ const testInlineSchemaExamples = name => {
       if (!goods.length) {
         this.skip();
       } else {
-        goods.filter(t => !t[SKIP_KEY]).forEach(good => {
-          const errors = Schema.validate(good).errors;
-          errors.should.have.length(0);
-        });
+        goods
+          .filter(t => !t[SKIP_KEY])
+          .forEach(good => {
+            const { errors } = Schema.validate(good);
+            errors.should.have.length(0);
+          });
       }
     });
 
     it('invalid example schemas should fail validation', function() {
       if (!bads.length) {
         this.skip();
       } else {
-        bads.filter(t => !t[SKIP_KEY]).forEach(bad => {
-          const errors = Schema.validate(bad).errors;
-          errors.should.not.have.length(0);
-        });
+        bads
+          .filter(t => !t[SKIP_KEY])
+          .forEach(bad => {
+            const { errors } = Schema.validate(bad);
+            errors.should.not.have.length(0);
+          });
       }
     });
   });