RESTWS-560 - Upgrade swagger-ui to 3.0.10 #295
Conversation
|
FYI: Only these two files will need to be reviewed: |
|
In the new UI there will be validation errors showing on the top, as shown by the picture. It lists resources that are not yet documented. So for example appframework/rest/AppResource.java This I think is useful, so we know exactly what to documented instead of having to manually validate the spec. |
| .description("OpenMRS RESTful API specification") | ||
| .contact(new Contact().url("http://openmrs.org")) | ||
| .description("OpenMRS RESTful API documentation generated by Swagger") | ||
| .contact(new Contact().name("OpenMRS Talk").url("https://talk.openmrs.org")) |
There was a problem hiding this comment.
What is the reason behind replacing openmrs.org with talk.openmrs.org?
There was a problem hiding this comment.
As there is where one could get support. If that doesn't make much sense, lets leave it as it is with openmrs.org
|
Each of these three commits should be a separate pull request. If we for instance reject the pull request because of not wanting to upgrade swagger, this should not affect updating of the description and contact information. So do you now see why these should be separate pull requests? |
|
@dkayiwa Yeah that makes sense. Since this is about upgrading the UI, may I remove the other two commits? |
|
Yes, separate them. |
|
When you mean separate them, did you mean that we'd need 3 separate issues or just PRs? |
|
Separate in the sense that when one is rejected, it does not lead into the other not being merged. |
|
Three separate PRs could facilitate that requirement. But however do we need to create separate issues for each one as well? :) Can a issue can have a multiple PRs or say that the last two commits are essential, so do we have to create new issues as sub-tasks of the original issue? |
|
If they fit into the original issue as parent, then they can be sub tasks. But for now, if you can create separate pull requests and include the pull request urls on the ticket, it would be much better than before. |
|
I reverted the last two commits. Should I update the PR name/description, and squash things..? |
|
why not? |
gayanW
changed the title
|
I was wondering if I could squash the commits without losing my local commit history? |
|
@gayanW are all these changes to do with upgrading swagger? |
|
apiDocs.jsp is the only changed file. Others are swagger-ui distribution files. The old files are replaced with the new files. |
| }); | ||
| window.onload = function() { | ||
| const ui = SwaggerUIBundle({ | ||
| url: "${pageContext.request.contextPath}/module/webservices/rest/swagger.json", |
There was a problem hiding this comment.
Is this also part of the upgrade?
There was a problem hiding this comment.
Yes. The newer versions use SwaggerUIBundle instead of SwaggerUi.
| <path d="M14.348 14.849c-.469.469-1.229.469-1.697 0L10 11.819l-2.651 3.029c-.469.469-1.229.469-1.697 0-.469-.469-.469-1.229 0-1.697l2.758-3.15-2.759-3.152c-.469-.469-.469-1.228 0-1.697.469-.469 1.228-.469 1.697 0L10 8.183l2.651-3.031c.469-.469 1.228-.469 1.697 0 .469.469.469 1.229 0 1.697l-2.758 3.152 2.758 3.15c.469.469.469 1.229 0 1.698z"/> | ||
| </symbol> | ||
|
|
||
| <symbol viewBox="0 0 20 20" id="large-arrow"> |
There was a problem hiding this comment.
Is this also part of the upgrade?
There was a problem hiding this comment.
No it is not part of the upgrade. It comes with swagger-ui dist as a sample. We have it before omod/src/main/webapp/resources/js/swagger-ui/index.html, so I include it.
| source: "auth", | ||
| level: "warning", | ||
| message: "Authorization may be unsafe, passed state was changed in server Passed state wasn't returned from auth server" | ||
| }); |
There was a problem hiding this comment.
Is this also part of the upgrade?
There was a problem hiding this comment.
May be not. It has been included before. It is your call :)
There was a problem hiding this comment.
Can we remove the samples? That way the changes will be smaller and hence easier to review.
gayanW
changed the title
This upgrades swagger-ui to v3.0.10