1140049 - Add a note about MozReview to the Create Attachment page

Status: RESOLVED FIXED

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

Description

[Mark Côté [:mcote]]

To get more people trying out MozReview, we should add a little note to the top of the Create New Attachment page saying something like

Are you attaching a patch?  Consider trying out <a href="https://reviewboard.mozilla.org/">MozReview</a>, Mozilla's new repository-based code-review tool.  <a href="http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview.html">Read the docs</a> to get started.

For now, this should only be shown if the bug's product is Core, Firefox, or Firefox for Android.

Comment 1

[Mark Côté [:mcote]]

Attachment: Advertise MozReview

I don't like the inline CSS, but it looks like allowing extra CSS files, like in show_bug, would require a lot of refactoring.

Comment 2

[Mark Côté [:mcote]]

Comment on attachment 8573708 [details] [diff] [review]
Advertise MozReview

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

::: template/en/default/attachment/create.html.tmpl
@@ +45,4 @@
>  -->
>  </script>
>  
> +[%# BMO hook for display MozReview message %]

This should, of course, be "displaying"; I've already fixed it locally.

Comment 3

[:glob ✱]

Comment on attachment 8573708 [details] [diff] [review]
Advertise MozReview

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

::: extensions/MozReview/Extension.pm
@@ +36,5 @@
> +            }
> +        } else {
> +            my @rrids;
> +            my $attachments =
> +                Bugzilla::Attachment->get_attachments_by_bug($bug);

nit: no need to wrap this line

::: extensions/MozReview/template/en/default/hook/attachment/create-before_form.html.tmpl
@@ +6,5 @@
> +  # defined by the Mozilla Public License, v. 2.0.
> +  #%]
> +
> +[% USE Bugzilla %]
> +[% cgi = Bugzilla.cgi %]

you aren't using cgi here

@@ +10,5 @@
> +[% cgi = Bugzilla.cgi %]
> +
> +[% RETURN UNLESS mozreview_enabled %]
> +
> +<div style="background-color:#fff9db;color:#666458;padding:5px;margin-bottom:10px">

i don't like inline css either.

hook the global header and add your stylesheet there.
look at Needinfo/template/en/default/hook/global/header-start.html.tmpl for an example

Comment 4

[Mark Côté [:mcote]]

Attachment: Advertise MozReview, v2

Not sure if I should split the CSS file into two now, since the last rule is only used by the attachments page (which doesn't use any other rules).  I left it in for simplicity but lemme know if you think that I should split it out.

Comment 5

[Mark Côté [:mcote]]

Attachment: Advertise MozReview, v3

Meh I decided to go ahead and split the CSS into two files.  Might as well save a few bytes of traffic.

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8574070 [details] [diff] [review]
Advertise MozReview, v3

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

I support the proposed messaging.

Comment 7

[:glob ✱]

Comment on attachment 8574070 [details] [diff] [review]
Advertise MozReview, v3

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

r=glob

::: extensions/MozReview/template/en/default/hook/attachment/create-before_form.html.tmpl
@@ +5,5 @@
> +  # This Source Code Form is "Incompatible With Secondary Licenses", as
> +  # defined by the Mozilla Public License, v. 2.0.
> +  #%]
> +
> +[% USE Bugzilla %]

you don't need to use bugzilla either; remove on commit.

Comment 8

[Mark Côté [:mcote]]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   b199f49..c25b318  master -> master