Rewrite bug modal info gathering block into perl

NEW
Assigned to

Status

()

bugzilla.mozilla.org
User Interface: Modal
P3
normal
2 years ago
a year ago

People

(Reporter: dylan, Assigned: dylan)

Tracking

(Blocks: 1 bug)

Production
Dependency tree / graph

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
There is a very large chunk of TT2 code that should be in perl.
(Assignee)

Comment 1

2 years ago
Created attachment 8791979 [details] [diff] [review]
1302865_1.patch
Attachment #8791979 - Flags: feedback?(dkl)
(Assignee)

Comment 2

2 years ago
I'm going to run at least this test: 

# 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/.
#
# This Source Code Form is "Incompatible With Secondary Licenses", as
# defined by the Mozilla Public License, v. 2.0.

use strict;
use warnings;
use lib qw(lib);
use Test::More;
use Bugzilla;
use Bugzilla::Constants;
BEGIN { Bugzilla->extensions }

use Bugzilla::Extension::BugModal::Util qw(bug_modal_info);

Bugzilla->usage_mode(USAGE_MODE_TEST);
Bugzilla->error_mode(ERROR_MODE_DIE);

my $dbh = Bugzilla->dbh;


my $bug_ids = $dbh->selectcol_arrayref('SELECT bug_id FROM bugs');

foreach my $bug_id (@$bug_ids) {
    Bugzilla->init_page;
    my $bug = Bugzilla::Bug->new($bug_id);
    my $info;
    my $ok = eval { $info = bug_modal_info($bug); 1 };
    my $err = $@;
    ok($ok, "bug_modal_info on bug $bug_id");
    diag $err if $err;
    Bugzilla::_cleanup();
}

done_testing;

before I ask for r? but I wanted to ask you to look at it early to make sure this is a good idea.
(Assignee)

Comment 3

2 years ago
Created attachment 8792043 [details] [diff] [review]
1302865_2.patch
Attachment #8791979 - Attachment is obsolete: true
Attachment #8791979 - Flags: feedback?(dkl)
Attachment #8792043 - Flags: review?(dkl)
Comment on attachment 8792043 [details] [diff] [review]
1302865_2.patch

Review of attachment 8792043 [details] [diff] [review]:
-----------------------------------------------------------------

t/008filter.t ............. 175/727 
#   Failed test '(en/default) extensions/BugModal/template/en/default/bug_modal/edit.html.tmpl has unfiltered directives:
#   10: import(bug_modal_info(bug.defined ? bug : bugs.0)) 
# --ERROR'
#   at t/008filter.t line 118.
t/008filter.t ............. 507/727 # Looks like you failed 1 test of 727.
t/008filter.t ............. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/727 subtests 

Change to:

[% import(bug_modal_info(bug.defined ? bug : bugs.0)) FILTER none %]

::: extensions/BugModal/Extension.pm
@@ +373,5 @@
>      }
>  }
>  
> +
> +

nit: remove extra whitespace here.

::: extensions/BugModal/lib/Util.pm
@@ +82,5 @@
> +    my $custom_fields = Bugzilla->active_custom_fields({
> +        product   => $bug->product_obj,
> +        component => $bug->component_obj,
> +        bug_id    => $bug->id
> +    });

my $custom_fields = [ Bugzilla->active_custom_fields(...) ];

@@ +116,5 @@
> +    foreach my $flag (@$tracking_flags) {
> +        my $is_set = $flag->bug_flag($bug->id)->value ne "---";
> +        if ($flag->flag_type eq 'tracking') {
> +            $tracking_flags_has = 1;
> +            $tracking_flags_set = $is_set;

This should be instead:

$tracking_flags_set = 1 if $is_set;

Same with $project_flags_set. Otherwise it is set to the last result of $flag->bug_flag($bug->id)->value ne "---".

@@ +140,5 @@
> +    # tracking flags title and subtitle
> +    foreach my $row (@$tracking_flags_table) {
> +        use Alive 'alive';
> +        alive(ref $row // "$row");
> +        alive({keys => [keys %$row]});

remove before commit.

@@ +189,5 @@
> +        obsolete_attachments   => $obsolete_attachments,
> +        has_bug_flags          => $has_bug_flags,
> +        tracking_flags_has     => $tracking_flags_has,
> +        tracking_flags_set     => $tracking_flags_set,
> +        project_flags_has      => $project_flags_has,

Missing:

         tracking_flags_title   => $tracking_flags_title,

::: extensions/TrackingFlags/Extension.pm
@@ +51,5 @@
> +    my ($bug) = @_;
> +    return $bug->{_bug_tracking_flags} //= Bugzilla::Extension::TrackingFlags::Flag->match({
> +        product     => $bug->product,
> +        component   => $bug->component,
> +        bug_id      => $bug->id,

nit: s/$bug/$self/
Attachment #8792043 - Flags: review?(dkl) → review-

Updated

a year ago
No longer blocks: 1150541

Updated

a year ago
Blocks: 1273046
(Assignee)

Updated

a year ago
Priority: P1 → P3
(Assignee)

Updated

a year ago
Blocks: 1350096
(Assignee)

Updated

a year ago
No longer blocks: 1350096
See Also: → bug 1350096
You need to log in before you can comment on or make changes to this bug.