Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1508201 - Allow bulk edits to happen in the background, asynchronously #973

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

dylanwh
Copy link
Contributor

@dylanwh dylanwh commented Dec 11, 2018

Updated with (I think) all review changes applied.

@dylanwh dylanwh requested a review from dklawren December 12, 2018 03:34
@dklawren
Copy link
Collaborator

dklawren commented Jan 7, 2019

diff --git a/t/bulk-edit.t b/t/bulk-edit.t index b6aebec10..a97fa7f98 100644 --- a/t/bulk-edit.t +++ b/t/bulk-edit.t @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/.

Copy link
Collaborator

@dklawren dklawren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Install code needs to create the bulk_edit group during checksetup
  • IMO the group should be called can-bulk-edit but not fussed either way.
  • process_bug.cgi does not check that user is in group bulk_edit that I can tell and allows anyone to bulk edit. But the template does check. Which policy do we want?

template/en/default/list/edit-multiple.html.tmpl Outdated Show resolved Hide resolved
template/en/default/task/created.html.tmpl Outdated Show resolved Hide resolved
@dylanwh
Copy link
Contributor Author

dylanwh commented Jan 25, 2019

process_bug.cgi does not check that user is in group bulk_edit that I can tell and allows anyone to bulk edit. But the template does check. Which policy do we want?

It did, but it was probably hard to see. Now it's bz_can_async_bulk_edit


# Make sure there are bugs to process.
ThrowUserError("no_bugs_chosen", {action => 'modify'}) unless @bug_ids;
$async_bulk_edit = 1 if @bug_ids > 10;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this. Seems that even if the user does not check the bulk edit checkbox on buglist.cgi, it will still enable it if @bug_ids > 10 which negates the need for a checkbox altogether.

Maybe this should be instead:

$async_bulk_edit = 0 if @bug_ids < 10;

Or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right now, doing > 10 edits is likely to fail, so it's meant to make them more likely to succeed. Eventually bz_async_bulk_edit should be the same as editbugs. Does that make sense?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine. But is still sort of misleading from a user standpoint. Maybe we should add some text or tooltip that states it is automatic if more than 10 bugs?

@dylanwh dylanwh self-assigned this Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants