Add actions to block add-ons / view authors in scanner results admin #24424
Conversation
| ) | ||
| return '-' | ||
|
|
||
| formatted_channel.short_description = 'Channel' |
There was a problem hiding this comment.
Regaining a little bit of horizontal space
|
|
||
| return ro_fields | ||
|
|
||
| def _get_input_guids(self, request): |
| url = reverse('admin:blocklist_blocklistsubmission_add') | ||
| # blocklist submission page expects guids separated by \n | ||
| parameters = {'guids': '\n'.join(guids)} | ||
| return HttpResponseRedirect(url + f'?{urlencode(parameters)}') |
There was a problem hiding this comment.
A GET redirect is going to hit the url limit quite quickly with the average guid length. Really you want a POST but I'm not sure how you can achieve that with a redirect (HttpResponseTemporaryRedirect requires the POST data to be already in the request, iirc, so it would have to be in the form that lead to the action... ?).
There was a problem hiding this comment.
Yeah this is not great.
The main problem is that even if the method was POST to begin with, I can't tamper with the data in the redirect.
Beginning of an idea, I can deal with the method by setting formmethod on the <input> responsible for the submission of the action, then stick the addon guids in disabled <input type="hidden"> in each row, then enable/disable them when each checkbox is ticked. Pretty convoluted though, and I'm not sure if the POST data will be forwarded to the redirected URL in the first place in all browsers, but I can try.
Alternatively, I could accept add-on pks in blocklist submission admin, and pass that instead via GET...
There was a problem hiding this comment.
yeah I think that's the only way - dump all the guid data into the html so the form can be submitted with it.
Or add a custom entrypoint to BlocklistSubmissionAdmin that accepts ScannerResult ids (I'm assuming the form post the action submits contains instance ids). It's a hack either way
There was a problem hiding this comment.
(my idea is possibly a more hacky hack, I've not thought through it)
There was a problem hiding this comment.
21f0060 still uses a GET request but with pks to cram more add-ons in a single request safely - using POST required a lot more hacks...
| if not acl.action_allowed_for(request.user, amo.permissions.BLOCKLIST_CREATE): | ||
| actions.pop('block_addons_action', None) | ||
| if not acl.action_allowed_for(request.user, amo.permissions.USERS_EDIT): | ||
| actions.pop('search_for_authors_action', None) |
There was a problem hiding this comment.
nit-ish: I wonder if it would be better if these were implemented using allowed_permissions, and a matching has_xxx_permission function, like ModelAdmin seems to prefer (looking at super().get_actions code). I think this would look like allowed_permissions=['blocklist'] in the @admin.action and a has_blocklist_permission function?
2 participants
Fixes mozilla/addons#16047
Testing
Expected: