Add rotation toggle and filtering to table view #3646
Conversation
havron
requested review from
charhate,
csine-pro,
hosseinsh and
jtschladen
as code owners
| @@ -1516,7 +1519,7 @@ def put(self, certificate_id, data=None): | |||
| Certificates, "/certificates/<int:certificate_id>", endpoint="certificate" | |||
| ) | |||
| api.add_resource( | |||
| Certificates, "/certificates/<int:certificate_id>/update/notify", endpoint="certificateUpdateNotify" | |||
| Certificates, "/certificates/<int:certificate_id>/update/switches", endpoint="certificateUpdateSwitches" | |||
There was a problem hiding this comment.
I don't think it's likely that anyone's using this endpoint programmatically (i.e. not via the UI) right now -- and therefore I think this change is acceptable -- but I do at least want to call out that this is a breaking/backwards incompatible change if they are.
There was a problem hiding this comment.
Thanks @jtschladen, that's a great point; we could bump the version in the changelog for it if desired! Although as you noted, I also suspect this isn't a highly used API endpoint given that PUT /certificates/<id> has been around for much longer, and offers more flexibility to programmatic callers looking to update certs.
I'd be happy to preserve /update/notify and add a new, separate /update/rotation endpoint, if we want to be cautious. I think I'd just need to add a new Resource class for it, split up the service funcs, and use a customPOST in the Angular service call.
There was a problem hiding this comment.
Thanks @havron for contributing the change. Code changes look good to me. We do suspect that it's not a highly used endpoint, so consolidating new changes in existing API makes sense. Given backward incompatibility, bumping version and adding changelog calling out this change will be helpful.
There was a problem hiding this comment.
Sounds good to me! I just added a changelog in 3e94c84, following the new release process. Let me know if I can improve the release notes 🙂 @charhate
There was a problem hiding this comment.
Thanks @havron, really appreciate your efforts in compiling detailed release notes.
Given all the change going in next release and specially API change that is not backward compatible, the minor version change seems more appropriate (so 0.10.0 instead of 0.9.1). I have taken liberty to commit this 7edee9f, hope you do not mind.
I'll merge the PR soon and do a release.
Bumping minor version give all the changes, specially the API change that is not backward compatible
WARNING: Title underline too short
This PR tackles Issue #3633 🙂 it surfaces the
rotationbool switch for all certificates to the certificate table view in Angular, and provides filtering support to let users quickly see which certs have autorotation on, and which do not!It builds on @charhate's new API from PR #3176 for updating switches in the cert table view.