Plugin order #3100
Plugin order #3100
Conversation
|
This is ready for a first look (some test to fix still?)! Please definitely give feedback on the design and UX. The main bit that changed in this PR is the ordering:
Some comments (and replies to PostHog/plugin-server#102):
Other flyby changes with this PR:
|
|
Immediate thought is that the plugins page is getting a little too crowded - not that massive of a problem at this stage though |
|
|
||
| const move = (arr: PluginTypeWithConfig[], from: number, to: number): { id: number; order: number }[] => { | ||
| const clone = [...arr] | ||
| Array.prototype.splice.call(clone, to, 0, Array.prototype.splice.call(clone, from, 1)[0]) |
There was a problem hiding this comment.
Why on Array.prototype.splice instead of just clone?
There was a problem hiding this comment.
Hmm, not sure I fully understand the question here :).
We need to clone, since .splice alters the array. The splice code itself is from a react-sortable-hoc demo and it's the shortest accurate reordering code I've found some far.
|
I fixed the nits as well as I could. Not sure about the Regarding the arrows, The last I also added one more flyby. Now when updates are available, clicking on the tag for Github and Gitlab plugins will open a diff comparing what exactly changed in the code: |
|
And yes, we need a PR for plugin-server as well to use the new order. To be addressed separately... |
posthog/api/plugin.py
Outdated
| @@ -310,18 +310,21 @@ def global_plugins(self, request: request.Request, **kwargs): | |||
| @action(methods=["PATCH"], detail=False) | |||
| def rearrange(self, request: request.Request, **kwargs): | |||
| if not can_configure_plugins_via_api(self.team.organization_id): | |||
| return Response([]) | |||
| return Response(status=403) | |||
There was a problem hiding this comment.
I may totally be in the wrong, but if we're not returning anything here, raise PermissionDenied may work better (error message and all).
There was a problem hiding this comment.
I added a raise ValidationError("Plugin configuration via the web is disabled!"). Not sure if that's the right error, but this same line was used a few lines higher in the code, and all errors dispatched here are those. Maybe something to check another time. I'll merge it now though if/when the tests pass.
Changes
DRAFT!Checklist