Skip to content
This repository has been archived by the owner on May 17, 2024. It is now read-only.

Allow docs to match examples from #39

Merged
merged 4 commits into from
Apr 3, 2018
Merged

Allow docs to match examples from #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
da5c563
skip inline tests for some examples
xavdid Apr 3, 2018
53c9eea
document mutually exclusive fields
xavdid Apr 3, 2018
461e338
tweak copy and formatting
xavdid Apr 3, 2018
f0281f9
code clarity and organization
xavdid Apr 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
268 changes: 54 additions & 214 deletions docs/build/schema.md
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions exported-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
"FieldSchema": {
"id": "/FieldSchema",
"description":
"Defines a field an app either needs as input, or gives as output.",
"Defines a field an app either needs as input, or gives as output. In addition to the requirements below, the following keys are mutually exclusive:\n\n* `children` & `list`\n* `children` & `dict`\n* `children` & `type`\n* `children` & `placeholder`\n* `children` & `helpText`\n* `children` & `default`\n* `dict` & `list`\n* `dynamic` & `dict`\n* `dynamic` & `choices`",
"type": "object",
"required": ["key"],
"properties": {
Expand Down Expand Up @@ -206,7 +206,8 @@
"$ref": "/FieldSchema"
},
"description":
"An array of child fields that define the structure of a sub-object for this field. Usually used for line items."
"An array of child fields that define the structure of a sub-object for this field. Usually used for line items.",
"minItems": 1
},
"dict": {
"description": "Is this field a key/value input?",
Expand Down Expand Up @@ -1101,7 +1102,8 @@
"required": ["key", "display", "search", "create"],
"properties": {
"key": {
"description": "A key to uniquely identify this search-or-create.",
"description":
"A key to uniquely identify this search-or-create. Must match the search key.",
"$ref": "/KeySchema"
},
"display": {
Expand Down
16 changes: 15 additions & 1 deletion lib/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
module.exports = {
ROOT_GITHUB: 'https://github.com/zapier/zapier-platform-schema',
DOCS_PATH: 'docs/build/schema.md'
DOCS_PATH: 'docs/build/schema.md',
SKIP_KEY: '_skipTest',
// the following pairs of keys can't be used together in FieldSchema
// they're stored here because they're used in a few places
INCOMPATIBLE_FIELD_SCHEMA_KEYS: [
['children', 'list'], // This is actually a Feature Request (https://github.com/zapier/zapier-platform-cli/issues/115)
['children', 'dict'], // dict is ignored
['children', 'type'], // type is ignored
['children', 'placeholder'], // placeholder is ignored
['children', 'helpText'], // helpText is ignored
['children', 'default'], // default is ignored
['dict', 'list'], // Use only one or the other
['dynamic', 'dict'], // dict is ignored
['dynamic', 'choices'] // choices are ignored
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These exclusive fields are only for FieldSchema, correct? Putting them here in constants.js is a bit confusing. Without looking at the other modules, it's not easy to tell what it is for only by the name of MUTUALLY_EXCLUSIVE_FIELDS. I suggest keeping incompatibleFields in mutuallyExclusiveFields.js and import it in FieldSchema.js. If that creates a circular import, maybe we can do the opposite, i.e., initialize incompatibleFields in FieldSchema.js and import it in mutuallyExclusiveFields.js. Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, yeah. The main issue is that the checks and schemas are each expected to export a single check/schema. We can export more things, but that requires code changes wherever schemas are imported (which is a more involved change). To that end, it would make sense to have it in its own common file, which is basically constants. I'll tweak the name and add comments to make it more clear why it's there though.

};
14 changes: 2 additions & 12 deletions lib/functional-constraints/mutuallyExclusiveFields.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,13 @@ const jsonschema = require('jsonschema');
// https://stackoverflow.com/questions/28162509/mutually-exclusive-property-groups#28172831
// it was harder to read and understand.

const incompatibleFields = [
['children', 'list'], // This is actually a Feature Request (https://github.com/zapier/zapier-platform-cli/issues/115)
['children', 'dict'], // dict is ignored
['children', 'type'], // type is ignored
['children', 'placeholder'], // placeholder is ignored
['children', 'helpText'], // helpText is ignored
['children', 'default'], // default is ignored
['dict', 'list'], // Use only one or the other
['dynamic', 'dict'], // dict is ignored
['dynamic', 'choices'] // choices are ignored
];
const { INCOMPATIBLE_FIELD_SCHEMA_KEYS } = require('../constants');

const verifyIncompatibilities = (inputFields, path) => {
const errors = [];

_.each(inputFields, (inputField, index) => {
_.each(incompatibleFields, ([firstField, secondField]) => {
_.each(INCOMPATIBLE_FIELD_SCHEMA_KEYS, ([firstField, secondField]) => {
if (_.has(inputField, firstField) && _.has(inputField, secondField)) {
errors.push(
new jsonschema.ValidationError(
Expand Down
30 changes: 25 additions & 5 deletions lib/schemas/FieldSchema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,22 @@ const RefResourceSchema = require('./RefResourceSchema');

const FieldChoicesSchema = require('./FieldChoicesSchema');

const { SKIP_KEY, INCOMPATIBLE_FIELD_SCHEMA_KEYS } = require('../constants');

// the following takes an array of string arrays (string[][]) and returns the follwing string:
// * `a` & `b`
// * `c` & `d`
// ... etc
const wrapInBackticks = s => `\`${s}\``;
const formatBullet = f => `* ${f.map(wrapInBackticks).join(' & ')}`;
const incompatibleFieldsList = INCOMPATIBLE_FIELD_SCHEMA_KEYS.map(
formatBullet
).join('\n');

module.exports = makeSchema(
{
id: '/FieldSchema',
description:
'Defines a field an app either needs as input, or gives as output.',
description: `Defines a field an app either needs as input, or gives as output. In addition to the requirements below, the following keys are mutually exclusive:\n\n${incompatibleFieldsList}`,
type: 'object',
examples: [
{ key: 'abc' },
Expand All @@ -20,9 +31,7 @@ module.exports = makeSchema(
key: 'abc',
choices: [{ label: 'Red', sample: '#f00', value: '#f00' }]
},
{ key: 'abc', children: [] },
{ key: 'abc', children: [{ key: 'abc' }] },
{ key: 'abc', children: [{ key: 'abc', children: [] }] },
{ key: 'abc', type: 'integer' }
],
antiExamples: [
Expand All @@ -33,6 +42,16 @@ module.exports = makeSchema(
{ key: 'abc', choices: [{ label: 'Red', value: '#f00' }] },
{ key: 'abc', choices: 'mobile' },
{ key: 'abc', type: 'loltype' },
{ key: 'abc', children: [] },
{
key: 'abc',
children: [{ key: 'def', children: [] }]
},
{
key: 'abc',
children: [{ key: 'def', children: [{ key: 'dhi' }] }],
[SKIP_KEY]: true
},
{ key: 'abc', children: ['$func$2$f$'] }
],
required: ['key'],
Expand Down Expand Up @@ -112,7 +131,8 @@ module.exports = makeSchema(
type: 'array',
items: { $ref: '/FieldSchema' },
description:
'An array of child fields that define the structure of a sub-object for this field. Usually used for line items.'
'An array of child fields that define the structure of a sub-object for this field. Usually used for line items.',
minItems: 1
},
dict: {
description: 'Is this field a key/value input?',
Expand Down
3 changes: 2 additions & 1 deletion lib/schemas/SearchOrCreateSchema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ module.exports = makeSchema(
required: ['key', 'display', 'search', 'create'],
properties: {
key: {
description: 'A key to uniquely identify this search-or-create.',
description:
'A key to uniquely identify this search-or-create. Must match the search key.',
$ref: KeySchema.id
},
display: {
Expand Down
22 changes: 12 additions & 10 deletions lib/utils/buildDocs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const links = require('./links');

const NO_DESCRIPTION = '_No description given._';
const COMBOS = ['anyOf', 'allOf', 'oneOf'];
const { SKIP_KEY } = require('../constants');

const walkSchemas = (InitSchema, callback) => {
const recurse = (Schema, parents) => {
Expand All @@ -33,6 +34,15 @@ const collectSchemas = InitSchema => {

const quoteOrNa = val => (val ? `\`${val.replace('`', '')}\`` : '_n/a_');

const formatExample = example => {
const ex = _.isPlainObject(example) ? _.omit(example, SKIP_KEY) : example;
return `* ${quoteOrNa(
// GH parses the newlines in bullets correctly, but it's a good thing to fix
// docs say Infinity for no line break at all
util.inspect(ex, { depth: null, breakLength: Infinity })
)}`;
};

// Generate a display of the type (or link to a $ref).
const typeOrLink = schema => {
if (schema.type === 'array' && schema.items) {
Expand Down Expand Up @@ -65,11 +75,7 @@ const makeExampleSection = Schema => {
return `\
#### Examples

${examples
.map(example => {
return `* ${quoteOrNa(util.inspect(example))}`;
})
.join('\n')}
${examples.map(formatExample).join('\n')}
`;
};

Expand All @@ -82,11 +88,7 @@ const makeAntiExampleSection = Schema => {
return `\
#### Anti-Examples

${examples
.map(example => {
return `* ${quoteOrNa(util.inspect(example))}`;
})
.join('\n')}
${examples.map(formatExample).join('\n')}
`;
};

Expand Down
34 changes: 0 additions & 34 deletions test/functional-constraints/deepNestedFields.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,38 +78,4 @@ describe('deepNestedFields', () => {
'instance.creates.foo.inputFields[1] must not contain deeply nested child fields. One level max.'
);
});

it('should error on fields with empty children', () => {
const definition = {
version: '1.0.0',
platformVersion: '1.0.0',
creates: {
foo: {
key: 'foo',
noun: 'Foo',
display: {
label: 'Create Foo',
description: 'Creates a...'
},
operation: {
perform: '$func$2$f$',
sample: { id: 1 },
inputFields: [
{ key: 'orderId', type: 'number' },
{
key: 'line_items',
children: []
}
]
}
}
}
};

const results = schema.validateAppDefinition(definition);
results.errors.should.have.length(1);
results.errors[0].stack.should.eql(
'instance.creates.foo.inputFields[1].children must not be empty.'
);
});
});
7 changes: 4 additions & 3 deletions test/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

require('should');
const { SKIP_KEY } = require('../lib/constants');

const testInlineSchemaExamples = name => {
const Schema = require('../lib/schemas/' + name);
Expand All @@ -12,7 +13,7 @@ const testInlineSchemaExamples = name => {
if (!goods.length) {
this.skip();
} else {
goods.forEach(good => {
goods.filter(t => !t[SKIP_KEY]).forEach(good => {
const errors = Schema.validate(good).errors;
errors.should.have.length(0);
});
Expand All @@ -23,7 +24,7 @@ const testInlineSchemaExamples = name => {
if (!bads.length) {
this.skip();
} else {
bads.forEach(bad => {
bads.filter(t => !t[SKIP_KEY]).forEach(bad => {
const errors = Schema.validate(bad).errors;
errors.should.not.have.length(0);
});
Expand All @@ -33,5 +34,5 @@ const testInlineSchemaExamples = name => {
};

module.exports = {
testInlineSchemaExamples: testInlineSchemaExamples
testInlineSchemaExamples
};