[PDE-724] (BREAKING) Change omitEmptyParams to removeMissingValuesFrom

[stevelikesmusic]

Description

We're changing omitEmptyParams to removeMissingValuesFrom for more flexibility and descriptiveness. Linked to the core PR doing the same.

Jira

https://zapierorg.atlassian.net/browse/PDE-724

[stevelikesmusic]

Two questions:

1. Is it worth creating a separate schema for this definition? I thought so since it's a new object, but can we define object literals within the schema? It seemed more confusing to me to say it's an object and then try to describe the object in the description.

2. If we should create a new schema type here, any suggestions on a better name? I'm not the biggest fan of this one 🤔

[eliangcs]

I agree it's better to define object literals within the schema because it's only used by RequestSchema and not elsewhere.

[stevelikesmusic]

Just for clarity @eliangcs, are you saying go with defining an object literal in the description, or stick with something like RequestTokenOptionsSchema?

[eliangcs]

I'm saying instead of a separate file for RequestTokenOptionsSchema, we can define removeMissingValuesFrom schema right below its description, so we don't have to worry about the name. Something like:

// in RequestSchema.js
      removeMissingValuesFrom: {
        description:
          'Should missing values be sent? (empty strings, `null`, and `undefined` only — `[]`, `{}`, and `false` will still be sent)',
        properties: {
          params: {
            description:
              'Refers to data sent via a requests query params (`req.params`)',
            type: 'boolean',
            default: false
          },
          body: {
            description: 'Refers to tokens sent via a requsts body (`req.body`)',
            type: 'boolean',
            default: false
          }
        }
      }

[stevelikesmusic]

Ok perfect! thanks @eliangcs

[stevelikesmusic]

I'm not sure I see where we would get the keys added @eliangcs. Properties doesn't get added to the docs if they aren't a part of a separate schema. So, we could just put everything in the description of removeMissingValuesFrom in addition to properties?

[eliangcs]

I'm fine with both options: 1) put everything in the description or 2) have a separate schema. @xavdid do you have a preference?

If we go with option 2, my only opinion would be the schema name should avoid the term "token". We could use a name like "RequestMissingValuesRemovalOptionsSchema".

If we go with option 1, remember to add an additionalProperties: false next to properties so it forbids properties other than params and body.

[xavdid]

I don't have strong feelings. I think we've got other one-off schemas, so if we go that route it's not a big deal. Inline with good descriptions is also perfectly fine. If we need it in another spot, we can break it out then.

[stevelikesmusic]

Thanks @xavdid @eliangcs. Ended up going with the inline option.

[eliangcs]

I agree it's better to define object literals within the schema because it's only used by RequestSchema and not elsewhere.

[eliangcs]

Shouldn't it be params instead of queryParams?