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

fix: Extend blueprint to include RequestBody properties #38

Merged
merged 7 commits into from
Aug 1, 2024

Conversation

kainpets
Copy link
Contributor

@kainpets kainpets commented Jul 26, 2024

  • The seam-blueprint.test.ts.md snapshot now includes request parameters with appropriate values.
  • @seamapi/types were bumped to latest version which is the cause of other changes to snapshot

Closes #25

@kainpets kainpets requested a review from a team as a code owner July 26, 2024 07:58
Copy link

vercel bot commented Jul 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
blueprint ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 1, 2024 5:18pm

@kainpets kainpets requested a review from razor-x July 26, 2024 07:59
@kainpets kainpets changed the title fix: Extend blueprint to include requestbody properties fix: Extend blueprint to include RequestBody properties Jul 26, 2024
Comment on lines +342 to +343
type: parsedProperty.type,
format: property.format,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to define these in the blueprint spec. I'm not sure if we need type and format. I guess the main use case for type would be "json type" and format is "semantic meaning". But if we know something is a date or a uuid, then we also know it is a string.

I think at the blueprint level, it's more important to build on the semantics. So we IMO we want

export type Property =
  | StringProperty
  | EnumProperty
  | RecordProperty
  | ListProperty
  | ObjectProperty
  | BooleanProperty
  | DatetimeProperty
  | IdProperty

but we can still include the low level type info. How about we remove type and do

interface StringProperty extends BaseProperty {
  format: 'string'
  jsonType: 'string'
}

interface DatetimeProperty extends BaseProperty {
  format: 'datetime'
  jsonType: 'string'
}

// ...

@kainpets kainpets enabled auto-merge July 27, 2024 04:36
@kainpets kainpets merged commit dcb966b into main Aug 1, 2024
5 of 6 checks passed
@kainpets kainpets deleted the parse-request-parameters branch August 1, 2024 17:17
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.

Add missing parameters to request
3 participants