Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api-on-the-fly - part 3 - cylc types #382

Merged
merged 12 commits into from
Feb 2, 2020

Conversation

oliver-sanders
Copy link
Member

Addresses the "support for custom Cylc types" point in #339

Note - Built atop #359
Note - Works with the data types defined in cylc/cylc-flow#3469

Overview:

Configure Vuetify form components to work with the Cylc data types defined in cylc/cylc-flow#3469.

  • Generate a sample mutation which has one argument for each data type present in the GraphQL schema for testing and evaluation.
  • Mask input values (e.g. ISO8601 date-times).
  • Add informative placeholders where appropriate.
  • Add basic validation where possible.
  • Support composite types (AKA input objects).
  • Improve the list component.
  • Fix the model persistence bug in FormGenerator (so the component can be reused).
  • Make the first steps towards separating Vuetify from the form generator.
  • Bring in documentation for arguments from the schema (render it as markdown).

Unrelated Changes:

  • Upgrade Vuetify (older version missing flat and has some bugs with field outlines)

What You Should Expect To See:

  • Forms should be slicker.
  • No traceback (all default values should appear).
  • All composite (input) types should be working (e.g. BroadcastSettings).
  • It's not going to be perfect yet but should be more than sufficient for beta release.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does contain off-topic changes (listed above).
  • Appropriate tests are included (unit and/or functional).
  • No change log entry required (still a dummy view not for user consumption)
  • No documentation update required.

@oliver-sanders oliver-sanders added this to the 0.3 milestone Jan 23, 2020
@oliver-sanders oliver-sanders self-assigned this Jan 23, 2020
@oliver-sanders oliver-sanders mentioned this pull request Jan 23, 2020
11 tasks
@kinow
Copy link
Member

kinow commented Jan 29, 2020

Sorry for the delay on reviewing it. Tomorrow will keep working on the infinite tree, but will try to review either in the afternoon break after lunch, or by Friday 👍

Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

I still had the Cylc Flow branch for the related change, so I re-synced the branch just now, and decided to review this one too.

Didn't check the branch or run any IDE inspection, but from reading the code found nothing wrong. Tested locally with the whole system, and the form was nicely rendered.

LGTM

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

LGTM, tested as working

src/components/graphqlFormGenerator/FormInput.vue Outdated Show resolved Hide resolved
@hjoliver
Copy link
Member

Minor conflicts

@hjoliver
Copy link
Member

hjoliver commented Feb 2, 2020

(Fixed conflicts).

@hjoliver hjoliver merged commit 1ba08be into cylc:master Feb 2, 2020
@kinow kinow modified the milestones: 0.3, 0.2 Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants