Skip to content

Move existing integration to JADNC #1076

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

Merged
merged 37 commits into from
Sep 17, 2021
Merged

Move existing integration to JADNC #1076

merged 37 commits into from
Sep 17, 2021

Conversation

maurei
Copy link
Member

@maurei maurei commented Sep 8, 2021

Closes #1048

In the effort of migrating the existing integration that lived in the private repository:

  • Moved all of the code from the existing integration to JsonApiDotNetCore.OpenApi
  • Introduced the JsonApiDotNetCore.OpenApi.Client project, which currently only introduces support for partial write requests for c# clients generated with NSwag.
  • Added a client library test suite to OpenApiTests.
  • Introduced the JsonApiDotNetCoreExampleClient project that generates a client for the example project based off the OpenAPI specification that was created using the OpenAPI integration in the example project.
  • Updated docs about the usage of a generated OpenAPI client.

Closes #1049

  • A matter of replacing ResourceNameFormatterProxy with ResourceNameFormatter.

Closes #1050

QUALITY CHECKLIST

@maurei maurei changed the base branch from master to openapi September 8, 2021 17:47
@maurei maurei force-pushed the oa/existing-integration branch from 3f4fd7b to c6d691d Compare September 10, 2021 09:44
@maurei maurei requested a review from bart-degreed September 10, 2021 09:45
@maurei maurei marked this pull request as ready for review September 10, 2021 09:45
@codecov
Copy link

codecov bot commented Sep 10, 2021

Codecov Report

Merging #1076 (d3ebfaf) into openapi (d226a0e) will increase coverage by 0.40%.
The diff coverage is 92.46%.

Impacted file tree graph

@@             Coverage Diff             @@
##           openapi    #1076      +/-   ##
===========================================
+ Coverage    88.42%   88.82%   +0.40%     
===========================================
  Files          258      306      +48     
  Lines         7039     8057    +1018     
===========================================
+ Hits          6224     7157     +933     
- Misses         815      900      +85     
Impacted Files Coverage Δ
...jects/Documents/PrimaryResourceResponseDocument.cs 0.00% <0.00%> (ø)
...ts/Documents/ResourceCollectionResponseDocument.cs 0.00% <0.00%> (ø)
...ts/ResourceIdentifierCollectionResponseDocument.cs 0.00% <0.00%> (ø)
...ts/Documents/ResourceIdentifierResponseDocument.cs 0.00% <0.00%> (ø)
...cts/Documents/SecondaryResourceResponseDocument.cs 0.00% <0.00%> (ø)
...DotNetCore.OpenApi/JsonApiObjects/JsonapiObject.cs 0.00% <0.00%> (ø)
.../JsonApiObjects/Links/LinksInRelationshipObject.cs 0.00% <0.00%> (ø)
...Objects/Links/LinksInResourceCollectionDocument.cs 0.00% <0.00%> (ø)
...pi/JsonApiObjects/Links/LinksInResourceDocument.cs 0.00% <0.00%> (ø)
...nks/LinksInResourceIdentifierCollectionDocument.cs 0.00% <0.00%> (ø)
... and 89 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9209642...d3ebfaf. Read the comment docs.

@maurei maurei force-pushed the oa/existing-integration branch from 322ad77 to bad9aad Compare September 14, 2021 13:41
Copy link
Contributor

@bart-degreed bart-degreed left a comment

Choose a reason for hiding this comment

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

Looked at the docs so far, will review more another day.

…not yet supported and will cause the integration to crash
…otNetCoreExample project. The only call that currently doesn't crash is the one associated to deleting a primary resource
@maurei maurei force-pushed the oa/existing-integration branch from e26dac4 to 7b3d1ef Compare September 15, 2021 07:10
Copy link
Contributor

@bart-degreed bart-degreed left a comment

Choose a reason for hiding this comment

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

Done with review pass. Please don't force-push anymore after the code review has started.

@maurei maurei force-pushed the oa/existing-integration branch from 856af3b to da1db37 Compare September 15, 2021 20:34
Copy link
Contributor

@bart-degreed bart-degreed left a comment

Choose a reason for hiding this comment

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

Done with another review pass. Still pending:

  • review tests, based on the doc you sent me
  • update install steps in .md
  • diff the swagger.json against internal prototype

Copy link
Contributor

@bart-degreed bart-degreed left a comment

Choose a reason for hiding this comment

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

Reviewed client tests

@maurei maurei force-pushed the oa/existing-integration branch from fbfdf56 to 5a4a06f Compare September 17, 2021 15:51
Copy link
Contributor

@bart-degreed bart-degreed left a comment

Choose a reason for hiding this comment

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

Gone over client tests again. Looks good now, except what's listed. Pending: swagger.json

@maurei maurei merged commit 362bf5f into openapi Sep 17, 2021
@maurei maurei deleted the oa/existing-integration branch September 17, 2021 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Import existing OpenApi integration
2 participants