1268305 - Integrate Readable Bug Status in Bug Detail Page

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: User Interface, defect)

Description

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Integrate readable bug status module into bug detail page. Bug detail page will expose enough information such at the bug status module's method for generating a status can be called in JavaScript, and the module will be loaded as JavaScript on the page.

Comment 1

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1268305_3.patch

Comment 2

[David Lawrence [:dkl]]

Comment on attachment 8753952 [details] [diff] [review]
1268305_3.patch

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

Nothing major. Did not do a thorough review of bugzilla-readable-status.js but looked OK from quick look over.

::: extensions/BugModal/Extension.pm
@@ +198,5 @@
> +        'Firefox for Android',
> +        'Firefox for iOS',
> +        'Bugzilla',
> +        'bugzilla.mozilla.org'
> +    );

Make this a constant In Constants.pm

@@ +200,5 @@
> +        'Bugzilla',
> +        'bugzilla.mozilla.org'
> +    );
> +    my $use_readable_bug_status = any { $bug->product eq $_ } @readable_bug_status_products;
> +    if ($use_readable_bug_status) {

Nit: 

if (any { $bug->product eq $_ } @readable_bug_status_products) {

::: extensions/BugModal/template/en/default/bug_modal/edit.html.tmpl
@@ +310,5 @@
>  
>  [%# === status === %]
> +[% IF readable_bug_status_json %]
> +    [% readable_bug_status_span = BLOCK -%]
> +        [%- %]<span id="readable-bug-status" data-readable-bug-status="[% readable_bug_status_json FILTER html %]"></span>

something to consider is thet HTML4 specifies a 65536 character limit for attribute values whereas HTML5 is unlimited in size. I don't think we will get close to that limit since we are not including large attributes such as comments, etc. Keywords or lots of tracking flags may get large but probably still OK. Suppose if we cared we could check length. We will be switching to HTML5 with the upstream merge so after that not an issue.

Comment 3

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

(In reply to David Lawrence [:dkl] from comment #2)
> something to consider is thet HTML4 specifies a 65536 character limit for
> attribute values whereas HTML5 is unlimited in size. I don't think we will
> get close to that limit since we are not including large attributes such as
> comments, etc. Keywords or lots of tracking flags may get large but probably
> still OK. Suppose if we cared we could check length. We will be switching to
> HTML5 with the upstream merge so after that not an issue.

Could we truncate long strings if needed? I don't think we'd ever get that big.

Comment 4

[Dylan Hardison [:dylan] (he/him)]

well, not truncate. But if it will be too big I'll just set it to the empty string.

Comment 5

[David Lawrence [:dkl]]

Comment on attachment 8753952 [details] [diff] [review]
1268305_3.patch

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

looked over readable-bugzilla-status.js some more and aside from needing minifcation and removal of whitespace, looks good. Should be quick r+ once other comments are addressed.

Comment 6

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Let me see if I can get browserify to minifiy when it packages it.

Comment 7

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

https://github.com/emceeaich/bugzilla-readable-status/commit/0b093600eddf04cad195d279e5a2349850240a81 enables minification https://github.com/emceeaich/bugzilla-readable-status/issues/14

Comment 8

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Comment on attachment 8753952 [details] [diff] [review]
1268305_3.patch

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

::: extensions/BugModal/web/bug_modal.js
@@ +735,4 @@
>              var other = $(that.attr('id') == 'dup_id' ? '#bottom-dup_id' : '#dup_id');
>              other.val(that.val());
>          });
> +    var rbs = $("#readable-bug-status");

Because browserify does not stub missing HTML5 APIs, these three lines should be in an `if` statement that checks for window.fetch's existence. 

We could add a fetch polyfill for IE and Safari, but this is progressive enhancement.

Comment 9

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Comment on attachment 8753952 [details] [diff] [review]
1268305_3.patch

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

::: extensions/BugModal/web/bug_modal.js
@@ +735,4 @@
>              var other = $(that.attr('id') == 'dup_id' ? '#bottom-dup_id' : '#dup_id');
>              other.val(that.val());
>          });
> +    var rbs = $("#readable-bug-status");

This can be disregarded, you're not using fetch on the page.

Comment 10

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1268305_4.patch

I don't feel comfortable moving the constant to a different file -- but the list of products is a constant now. What do you think?

Comment 11

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1268305_5.patch

without deleted file...

Comment 12

[David Lawrence [:dkl]]

Comment on attachment 8755669 [details] [diff] [review]
1268305_5.patch

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

Remove js/bugzilla-readable-status-min.js.map. r=dkl

::: extensions/BugModal/Extension.pm
@@ +20,5 @@
>  use Bugzilla::Util qw(trick_taint datetime_from html_quote time_ago);
>  use List::MoreUtils qw(any);
>  use Template::Stash;
> +use JSON::XS qw(encode_json);
> +use Scalar::Util qw(blessed);

blessed not used anywhere. remove this line.

Comment 13

[Dylan Hardison [:dylan] (he/him)]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   915f99d..4434d09  master -> master

I jumped the gun, so also this nit:

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   4434d09..ae11e60  master -> master

Comment 14

[David Lawrence [:dkl]]

This is live.