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

Some objects in the OpenAPI spec are incorrect #309

Merged
merged 3 commits into from
Nov 11, 2024

Conversation

jkoenig134
Copy link
Member

@jkoenig134 jkoenig134 commented Nov 11, 2024

Readiness checklist

  • I added/updated tests.
  • I ensured that the PR title is good enough for the changelog.
  • I labeled the PR.

Description

Fixes nmshd/feedback#29

Fixed issues:

  • @type is displayed as required in RequestResponseContent
  • peerIdentity is missing in Relationship

@jkoenig134 jkoenig134 added the bug Something isn't working label Nov 11, 2024
@jkoenig134 jkoenig134 changed the title some objects in the openapi spec are incorrect Some objects in the OpenAPI spec are incorrect Nov 11, 2024
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

see 1 file with indirect coverage changes

@jkoenig134 jkoenig134 requested a review from sebbi08 November 11, 2024 10:56
@Milena-Czierlinski
Copy link
Contributor

@jkoenig134 What information is actually supposed to be given in the openapi.yml file? I don't completely see what pattern we are following there.

@jkoenig134
Copy link
Member Author

I don't get your question. In a perfect world openapi and the results would be 1:1 the same. It's hard to archieve that, so I fixed these bugs.
If you find more feel free to mention that :)

@Milena-Czierlinski
Copy link
Contributor

I had trouble finding the data structure that is returned as a result so that I might compare them. Where do I see how the returned object will occur?

Another question that is principally unrelated to your changes: Why do we have peer and peerIdentity?

@jkoenig134
Copy link
Member Author

I had trouble finding the data structure that is returned as a result so that I might compare them. Where do I see how the returned object will occur?

Another question that is principally unrelated to your changes: Why do we have peer and peerIdentity?

The Data structure is RelationshipDTO.

Regarding peer and peerIdentity: I think for easier / more performant querying.

@Milena-Czierlinski
Copy link
Contributor

And is @type never required?

@jkoenig134
Copy link
Member Author

@type is not rendered out by ts-serval when the property can only have one @type, to save bandwidth.

@britsta
Copy link
Contributor

britsta commented Nov 11, 2024

Regarding peer and peerIdentity: I think for easier / more performant querying.

Should we also add the peerIdentity property to the documentation of the Relationship in the Data Model Overview then? I also thought it was weird to have a peer property in addition to the peerIdentity property. I wonder how we can present this redundance in a meaningful way in the documentation.

@jkoenig134
Copy link
Member Author

we should document it, yes.

@jkoenig134 jkoenig134 merged commit 2910857 into main Nov 11, 2024
10 checks passed
@jkoenig134 jkoenig134 deleted the @type-is-displayed-as-required-in-some-objects branch November 11, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Connector API: RequestContent misses @type
3 participants