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

allow ordering static dropdowns #30

Merged
merged 1 commit into from
Dec 15, 2017
Merged

allow ordering static dropdowns #30

merged 1 commit into from
Dec 15, 2017

Conversation

xavdid
Copy link
Contributor

@xavdid xavdid commented Dec 13, 2017

fixes zapier/zapier-platform-cli#125 in a less-hacky way.

Side note, this was a bear to test. Had to hack apart the CLI and server to forego any form of validation so I could figure out what structure(s) worked before writing the schema for it.

@xavdid xavdid requested a review from eliangcs December 13, 2017 22:08
Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

This seems to be the right way to support ordered static field choices. Two questions:

  1. Can we add a test for it?
  2. Does the backend already support this new schema?

@xavdid
Copy link
Contributor Author

xavdid commented Dec 14, 2017

Schema is tested via the examples and antiExamples arrays in the schema itself. The test iterates through all the schemas and makes sure that all examples validate and none of the antiExamples do (link). So each of the new ones has some simple tests built in!

Backend does support it! Once the app builds, it ships that json straight to the backend. It does server validation and then stores it. From there, it gets processed by formatic (which is the software that powers the editor). So we have to match the format it's expecting, but that's all we need.

My app:

screen shot 2017-12-14 at 11 12 06 am

local editor:

screen shot 2017-12-14 at 11 12 38 am

Test result (just returns input):

screen shot 2017-12-14 at 11 13 09 am

I couldn't test against prod since the server validation uses the published version (I think), but i could disable that locally and the editor handled the json correctly.

Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification! I think you need to rebuild doc and schema then it's good to merge:

npm run docs
npm run export

@xavdid
Copy link
Contributor Author

xavdid commented Dec 15, 2017

yep! i was going to let the release take care of that so the docs aren't in master before the code is deployed (which would be confusing)

@eliangcs
Copy link
Member

@xavdid sounds good!

@xavdid xavdid merged commit e46e763 into master Dec 15, 2017
@xavdid xavdid deleted the ordered-choices branch December 15, 2017 02:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Field's choices aren't sorted
2 participants