feat(ui): better field types #6396
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
psychedelicious
requested review from
blessedcoolant,
maryhipp,
hipsterusername and
lstein
as code owners
github-actions
bot
added
frontend
hipsterusername
approved these changes
May 20, 2024
Replace the `isCollection` and `isCollectionOrScalar` flags with a single enum value `cardinality`. Valid values are `SINGLE`, `COLLECTION` and `SINGLE_OR_COLLECTION`. Why: - The two flags were mutually exclusive, but this wasn't enforce. You could create a field type that had both `isCollection` and `isCollectionOrScalar` set to true, whuch makes no sense. - There was no explicit declaration for scalar/single types. - Checking if a type had only a single flag was tedious. Thanks to a change a couple months back in which the workflows schema was revised, field types are internal implementation details. Changes to them are non-breaking.
Update business logic and tests.
At some point, I made a mistake and imported the wrong types to some files for the old v1 and v2 workflow schema migration data. The relevant zod schemas and inferred types have been restored. This change doesn't alter runtime behaviour. Only type annotations.
psychedelicious
force-pushed
the
psyche/feat/ui/better-field-types
branch
from
fb51e9e
to
f1206c6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
While working on #6386, I was reminded of some awkwardness around field types and cardinality (single vs collection). I've been meaning to make this change for some time but was afraid to do it without good tests in place. Now that we have solid test coverage for schema parsing and connection validation, this is no longer scary.
From one of the commits:
That means this PR doesn't include any user-facing changes and doesn't change the workflow schema or structure. It only changes the templates format (templates are generated at runtime).
I did need to fix a past mistake with some type annotations for workflow migration. These are type annotations only, there's no change to runtime workflow migration behaviour.
Related Issues / Discussions
n/a
QA Instructions
Give 'er a whirl.
Merge Plan
n/a
Checklist