-
Notifications
You must be signed in to change notification settings - Fork 1
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
Write and use OpenAPI specification #352
Conversation
7caa0cf
to
8cfb9ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Have some mostly minor questions and comments.
Code Climate has analyzed commit 39a4c1a and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 97.6% (0.0% change). View more on Code Climate. |
@aaron-collier can you rebase please? |
@jcoyne rebased. Now broken tests... I'll try to get time on this today |
Add inital GET paths for current endpoints Add DELETE to /objects/druid/workflows Remove tags Add POST operations Add process schema Add older repo based paths
Switch 404 to 400 in show_workflow_template_spec Do not require version on versionClose Add start-accession to valid process list Add delete to the repo workflow path Add path for workflow_templates without parameter Switch to 400 for show_step_spec Remove required from completed parameter Clean up rubocop Remove helper comments
Remove unrelated comment Finalize openapi and replace all TODO tags
@jcoyne have another look? |
@@ -60,7 +60,7 @@ | |||
end | |||
|
|||
it 'draws milestones from the current version' do | |||
get "/objects/#{druid}/lifecycle?active-only=true&version=2" | |||
get "/dor/objects/#{druid}/lifecycle?active-only=true&version=2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you adding this in. Using the /dor/
prefix is deprecated.
@@ -103,7 +103,7 @@ | |||
end | |||
|
|||
it 'draws milestones from the all versions' do | |||
get "/objects/#{druid}/lifecycle" | |||
get "/dor/objects/#{druid}/lifecycle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you adding this in. Using the /dor/
prefix is deprecated.
get "/dor/objects/#{step.druid}/workflows/#{step.workflow}/#{step.process}x" | ||
|
||
expect(response).to be_not_found | ||
expect(response).to have_http_status(:bad_request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did this response change to 400?
</style> | ||
</head> | ||
<body> | ||
<redoc spec-url='https://raw.githubusercontent.com/sul-dlss/workflow_server-rails/master/openapi.yml'></redoc> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to change master
to main
Why was this change made?
Fixes #349
This:
Was the documentation (API, README, DevOpsDocs, wiki, consul, etc.) updated?
README - Badge added
/docs folder added to populate github pages
Does this change affect how this application integrates with other services?
I trolled the stage and prod logs to verify all paths were captured. It may be prudent to deploy this to stage and run the integration tests, but any one-off requests may still be missed.