revert #8103b63; fix bugs; restyle; refactor

JonSilver commented 4 years ago

This PR incorporates the following changes I've made, arising from #66:

rolled back this commit originally discussed in #17
fixed all the problems that have prevented my use of the plugin on complex, branching flows for the past 3 years
entirely refactored the code in swagger.js to a modern functional ES6 style with heavy use of destructuring, focusing on code readability, maintainability; a more self-documenting, narrative flow with far less wordy code.
left the swagger-ui version at 2.1.4 because of apparent breaking changes in that library from 2.1.4 to 2.1.5 and seemingly at many points since.
applied a modicum of extra styling to swagger-ui-html to help it feel more modern and fix some UI oddities.

The code has been tested with every flow I have that previously broke the plugin.

This PR should fix or positively affect #43, #44, #57, #58, #59, #60, #61. Due to the rollback of an earlier commit, it also supercedes PRs #62 and #64.

The rollback of the earlier commit means that #17 should probably be reopened and fully discussed before reimplementing the idea.

jsf-clabot commented 4 years ago

All committers have signed the CLA.

knolleary commented 4 years ago

Thanks for this. We've had a report that this node is broken on 1.0 due to its use of some Bootstrap apis we removed.

What version of NR did you test this against? Keen to get it working on 1.0 again, but should probably get this PR merged first as a healthy base-line.

JonSilver commented 4 years ago

@knolleary I last tried it on 0.20.8. I'm just moving everything over to 1.0.2 so if you can leave it with me for a few days I'll take another look against latest and report back.

knolleary commented 4 years ago

Here's the report of the problem. https://discourse.nodered.org/t/did-a-recent-update-break-swagger/17369

I have asked them to raise an issue, but I know what needs doing and just wanted to sanity check you hadn't got there first somehow with this pr.

I'll look at merging this and then fixing the 1.0 issue.

JonSilver commented 4 years ago

@knolleary I've taken a look at it... and I've found that:

The Swagger UI pane shows all my endpoints in their tag groupings with all the Try It Out stuff working
The Edit or Add button on any http in node, whether or not it has existing documentation, shows the Summary, Description, Tags, Consumes, Produces and Deprecated fields, along with two large empty boxes for parameters and responses. The +parameter and +response buttons do nothing.
Console shows this error: oneditprepare fc9d4019.cb0ad swagger-doc TypeError: $(...).popover is not a function in red.min.js:16

This was all working in my fork of node-red-node-swagger on Node-RED 0.20.8 but not in 1.0.x. I concur that this is down to popover() no longer being around having removed Bootstrap.

I also included in the PR some limited styling improvements by adding some retrofit CSS rules that now don't work and appear not to make it through to the browser at all.

JonSilver commented 4 years ago

@knolleary sorry for the delay but I've now merged the PR from @jpwsutton, retested locally with Node-RED 1.0.2, and all seems well.

I can once again create and edit docs
the API testing rig works fine
the swagger.json file is generated

I haven't yet tried bumping the Swagger-UI version - that can probably wait until after we've got a working baseline release, but I'll let you take the lead there.

knolleary commented 4 years ago

Just need to get @jpwsutton to sign the CLA and we can merge this.

jpwsutton commented 4 years ago

Sorry, it looks like the CLA was signed for my other email address. All done now 👍

node-red / node-red-node-swagger

revert #8103b63; fix bugs; restyle; refactor #67