Bug 472872: Add a field where people can put the URLs to Bugzilla bugs (from any…

Bugzilla/Bug.pm

@@ -47,6 +47,8 @@ use Bugzilla::Status;
 
 use List::Util qw(min);
 use Storable qw(dclone);
+use URI;
+use URI::QueryParam;
 
 use base qw(Bugzilla::Object Exporter);
 @Bugzilla::Bug::EXPORT = qw(
@@ -746,6 +748,25 @@ sub update {
         }
     }
 
+    # See Also
+    my ($removed_see, $added_see) = 
+        diff_arrays($old_bug->see_also, $self->see_also);
+
+    if (scalar @$removed_see) {
+        $dbh->do('DELETE FROM bug_see_also WHERE bug_id = ? AND '
+                 . $dbh->sql_in('value', [('?') x @$removed_see]),
+                  undef, $self->id, @$removed_see);
+    }
+    foreach my $url (@$added_see) {
+        $dbh->do('INSERT INTO bug_see_also (bug_id, value) VALUES (?,?)',
+                 undef, $self->id, $url);
+    }
+    # If any changes were found, record it in the activity log
+    if (scalar @$removed_see || scalar @$added_see) {
+        $changes->{see_also} = [join(', ', @$removed_see),
+                                join(', ', @$added_see)];
+    }
+
     # Log bugs_activity items
     # XXX Eventually, when bugs_activity is able to track the dupe_id,
     # this code should go below the duplicates-table-updating code below.
@@ -1691,7 +1712,7 @@ sub fields {
            reporter_accessible cclist_accessible
            classification_id classification
            product component version rep_platform op_sys
-           bug_status resolution dup_id
+           bug_status resolution dup_id see_also
            bug_file_loc status_whiteboard keywords
            priority bug_severity target_milestone
            dependson blocked votes everconfirmed
@@ -2268,6 +2289,61 @@ sub remove_group {
     @$current_groups = grep { $_->id != $group->id } @$current_groups;
 }
 
+sub add_see_also {
+    my ($self, $input) = @_;
+    $input = trim($input);
+
+    # We assume that the URL is an HTTP URL if there is no (something):// 
+    # in front.
+    my $uri = new URI($input);
+    if (!$uri->scheme) {
+        # This works better than setting $uri->scheme('http'), because
+        # that creates URLs like "http:domain.com" and doesn't properly
+        # differentiate the path from the domain.
+        $uri = new URI("http://$input");
+    }
+    elsif ($uri->scheme ne 'http' && $uri->scheme ne 'https') {
+        ThrowUserError('bug_url_invalid', { url => $input, reason => 'http' });
+    }
+
+    if ($uri->path !~ /show_bug\.cgi$/) {
+        ThrowUserError('bug_url_invalid', 
+                       { url => $input, reason => 'show_bug' });
+    }
+    my $bug_id = $uri->query_param('id');
+    # We don't currently allow aliases, because we can't check to see
+    # if somebody's putting both an alias link and a numeric ID link.
+    # When we start validating the URL by accessing the other Bugzilla,
+    # we can allow aliases.
+    detaint_natural($bug_id);
+    if (!$bug_id) {
+        ThrowUserError('bug_url_invalid', { url => $input, reason => 'id' });
+    }
+
+    # Make sure that "id" is the only query parameter.
+    $uri->query("id=$bug_id");
+    # And remove any # part if there is one.
+    $uri->fragment(undef);
+    my $result = $uri->canonical->as_string;
+
+    if (length($result) > MAX_BUG_URL_LENGTH) {
+        ThrowUserError('bug_url_too_long', { url => $result });
+    }
+
+    # We only add the new URI if it hasn't been added yet. URIs are
+    # case-sensitive, but most of our DBs are case-insensitive, so we do
+    # this check case-insensitively.
+    if (!grep { lc($_) eq lc($result) } @{ $self->see_also }) {
+        push(@{ $self->see_also }, $result);
+    }
+}
+
+sub remove_see_also {
+    my ($self, $url) = @_;
+    my $see_also = $self->see_also;
+    @$see_also = grep { lc($_) ne lc($url) } @$see_also;
+}
+
 #####################################################################
 # Instance Accessors
 #####################################################################
@@ -2539,6 +2615,14 @@ sub reporter {
     return $self->{'reporter'};
 }
 
+sub see_also {
+    my ($self) = @_;
+    return [] if $self->{'error'};
+    $self->{'see_also'} ||= Bugzilla->dbh->selectcol_arrayref(
+        'SELECT value FROM bug_see_also WHERE bug_id = ?', undef, $self->id);
+    return $self->{'see_also'};
+}
+
 sub status {
     my $self = shift;
     return undef if $self->{'error'};

Bugzilla/Constants.pm

@@ -124,6 +124,7 @@ use File::Basename;
     FIELD_TYPE_TEXTAREA
     FIELD_TYPE_DATETIME
     FIELD_TYPE_BUG_ID
+    FIELD_TYPE_BUG_URLS
 
     USAGE_MODE_BROWSER
     USAGE_MODE_CMDLINE
@@ -156,6 +157,7 @@ use File::Basename;
     MAX_COMPONENT_SIZE
     MAX_FIELD_VALUE_SIZE
     MAX_FREETEXT_LENGTH
+    MAX_BUG_URL_LENGTH
 
     PASSWORD_DIGEST_ALGORITHM
     PASSWORD_SALT_LENGTH
@@ -361,6 +363,7 @@ use constant FIELD_TYPE_MULTI_SELECT => 3;
 use constant FIELD_TYPE_TEXTAREA  => 4;
 use constant FIELD_TYPE_DATETIME  => 5;
 use constant FIELD_TYPE_BUG_ID  => 6;
+use constant FIELD_TYPE_BUG_URLS => 7;
 
 # The maximum number of days a token will remain valid.
 use constant MAX_TOKEN_AGE => 3;
@@ -447,6 +450,9 @@ use constant MAX_FIELD_VALUE_SIZE => 64;
 # Maximum length allowed for free text fields.
 use constant MAX_FREETEXT_LENGTH => 255;
 
+# The longest a bug URL in a BUG_URLS field can be.
+use constant MAX_BUG_URL_LENGTH => 255;
+
 # This is the name of the algorithm used to hash passwords before storing
 # them in the database. This can be any string that is valid to pass to
 # Perl's "Digest" module. Note that if you change this, it won't take

Bugzilla/DB/Schema.pm

@@ -487,6 +487,17 @@ use constant ABSTRACT_SCHEMA => {
         ],
     },
 
+    bug_see_also => {
+        FIELDS => [
+            bug_id => {TYPE => 'INT3', NOTNULL => 1},
+            value  => {TYPE => 'varchar(255)', NOTNULL => 1},
+        ],
+        INDEXES => [
+            bug_see_also_bug_id_idx => {FIELDS => [qw(bug_id value)], 
+                                        TYPE   => 'UNIQUE'},
+        ],
+    },
+
     # Keywords
     # --------
 
@@ -1500,7 +1511,6 @@ use constant MULTI_SELECT_VALUE_TABLE => {
     ],
 };
 
-
 #--------------------------------------------------------------------------
 
 =head1 METHODS

Bugzilla/Field.pm

@@ -228,6 +228,8 @@ use constant DEFAULT_FIELDS => (
     {name => 'attach_data.thedata',   desc => 'Attachment data'},
     {name => 'attachments.isurl',     desc => 'Attachment is a URL'},
     {name => "owner_idle_time",       desc => "Time Since Assignee Touched"},
+    {name => 'see_also',              desc => "See Also",
+     type => FIELD_TYPE_BUG_URLS},
 );
 
 ################
@@ -309,7 +311,7 @@ sub _check_type {
     my $saved_type = $type;
     # The constant here should be updated every time a new,
     # higher field type is added.
-    (detaint_natural($type) && $type <= FIELD_TYPE_BUG_ID)
+    (detaint_natural($type) && $type <= FIELD_TYPE_BUG_URLS)
       || ThrowCodeError('invalid_customfield_type', { type => $saved_type });
     return $type;
 }

Bugzilla/Install/Requirements.pm

@@ -122,6 +122,11 @@ sub REQUIRED_MODULES {
         module  => 'Email::MIME::Modifier',
         version => '1.442'
     },
+    {
+        package => 'URI',
+        module  => 'URI',
+        version => 0
+    },
     );
 
     my $all_modules = _get_extension_requirements(

Bugzilla/Search.pm

@@ -104,7 +104,8 @@ sub init {
     my @select_fields = 
         Bugzilla->get_fields({ type => FIELD_TYPE_SINGLE_SELECT });
     
-    my @multi_select_fields = Bugzilla->get_fields({ type => FIELD_TYPE_MULTI_SELECT,
+    my @multi_select_fields = Bugzilla->get_fields({
+        type     => [FIELD_TYPE_MULTI_SELECT, FIELD_TYPE_BUG_URLS],
         obsolete => 0 });
     foreach my $field (@select_fields) {
         my $name = $field->name;

process_bug.cgi

@@ -336,6 +336,14 @@ foreach my $b (@bug_objects) {
     $b->reset_assigned_to if $cgi->param('set_default_assignee');
     $b->reset_qa_contact  if $cgi->param('set_default_qa_contact');
 
+    if (should_set('see_also')) {
+        my @see_also = split(',', $cgi->param('see_also'));
+        $b->add_see_also($_) foreach @see_also;
+    }
+    if (should_set('remove_see_also')) {
+        $b->remove_see_also($_) foreach $cgi->param('remove_see_also')
+    }
+
     # And set custom fields.
     foreach my $field (@custom_fields) {
         my $fname = $field->name;

skins/standard/global.css

@@ -462,6 +462,12 @@ div.user_match {
     border: 1px solid #404D6C;
 }
 
+.bug_urls {
+    margin: 0 0 1em 0;
+    padding: 0;
+    list-style-type: none;
+}
+
 form#Create th {
     text-align: right;
 }

template/en/default/bug/edit.html.tmpl

@@ -190,6 +190,8 @@
          
          [% PROCESS section_spacer %]
 
+         [% PROCESS section_see_also %] 
+         
          [% PROCESS section_customfields %]
          
          [% PROCESS section_spacer %]
@@ -902,6 +904,19 @@
 [% END %]
 
 [%############################################################################%]
+[%# Block for See Also                                                       #%]
+[%############################################################################%]
+[% BLOCK section_see_also %]
+  <tr>
+    [% INCLUDE bug/field.html.tmpl 
+         field    = bug_fields.see_also
+         value    = bug.see_also
+         editable = bug.check_can_change_field('see_also', 0, 1)
+    %]
+  </tr>
+[% END %]
+
+[%############################################################################%]
 [%# Block for FLAGS                                                          #%]
 [%############################################################################%]
 

template/en/default/bug/field.html.tmpl

@@ -164,6 +164,23 @@
        [% INCLUDE global/textarea.html.tmpl
            id = field.name name = field.name minrows = 4 maxrows = 8
            cols = 60 defaultcontent = value %]
+     [% CASE constants.FIELD_TYPE_BUG_URLS %]
+       [% '<ul class="bug_urls">' IF value.size %]
+       [% FOREACH url = value %]
+         <li>
+           <a href="[% url FILTER html %]">[% url FILTER html %]</a>
+           <label><input type="checkbox" value="[% url FILTER html %]"
+                         name="remove_[% field.name FILTER html %]">
+             Remove</label>
+         </li>
+       [% END %]
+       [% '</ul>' IF value.size %]
+
+       <label for="[% field.name FILTER html %]">
+         <strong>Add [% terms.Bug %] URLs:</strong>
+       </label><br>
+       <input type="text" id="[% field.name FILTER html %]" 
+              name="[% field.name FILTER html %]" size="40">
   [% END %]
 [% ELSIF field.type == constants.FIELD_TYPE_TEXTAREA %]
   <div class="uneditable_textarea">[% value FILTER wrap_comment(60)

template/en/default/global/field-descs.none.tmpl

@@ -76,6 +76,7 @@
                    "reporter_accessible"     => "Reporter accessible",
                    "requestees.login_name"   => "Flag Requestee",
                    "resolution"              => "Resolution",
+                   "see_also"                => "See Also",
                    "setters.login_name"      => "Flag Setter",
                    "setting"                 => "Setting",
                    "settings"                => "Settings",
@@ -90,12 +91,14 @@
     Description here, by copying their Description from the
     database. If you want to override this for your language
     or your installation, just use a hook. %]
-
+[%# Also create the bug_fields hash. %]
 [% UNLESS Param('shutdownhtml') %]
   [% USE Bugzilla %]
+  [% SET bug_fields = {} %]
   [% FOREACH bz_field = Bugzilla.get_fields() %]
     [% SET field_descs.${bz_field.name} = bz_field.description
        IF !field_descs.${bz_field.name}.defined %]
+    [% SET bug_fields.${bz_field.name} = bz_field %]
   [% END %]
 [% END %]
 

template/en/default/global/user-error.html.tmpl

@@ -229,6 +229,24 @@
                    [% bug_id FILTER url_quote %]&GoAheadAndLogIn=1">log
     in to an account</a> with the appropriate permissions.
 
+  [% ELSIF error == "bug_url_invalid" %]
+    [% title = "Invalid Bug URL" %]
+    <code>[% url FILTER html %]</code> is not a valid URL to [% terms.abug %].
+    [% IF reason == 'http' %]
+      URLs must start with "http" or "https".
+    [% ELSIF reason == 'show_bug' %]
+      [%+ terms.Bug %] URLs should point to <code>show_bug.cgi</code>
+      in a [% terms.Bugzilla %] installation.
+    [% ELSIF reason == 'id' %]
+      There is no valid [% terms.bug %] id in that URL.
+    [% END %]
+
+  [% ELSIF error == "bug_url_too_long" %]
+    [% title = "Invalid Bug URL" %]
+    [% terms.Bug %] URLs can not be longer than 
+    [%+ constants.MAX_BUG_URL_LENGTH FILTER none %] characters long.
+    <code>[% url FILTER html %]</code> is too long.
+
   [% ELSIF error == "buglist_parameters_required" %]
     [% title = "Parameters Required" %]
     [% docslinks = {'query.html' => "Searching for $terms.bugs",

template/en/default/pages/fields.html.tmpl

@@ -315,4 +315,16 @@ When searching for [% terms.bugs %] that have been resolved or
 verified, remember to set the status field appropriately. 
 </p>
 
+<h2><a name="see_also"></a>See Also</h2>
+
+<p>This allows you to refer to [% terms.bugs %] in other installations.
+  You can enter a URL to a [%+ terms.bug %] in the "Add [% terms.Bug %] URLs"
+  field to note that that [% terms.bug %] is related to this one. You can
+  enter multiple URLs at once by separating them with a comma.</p>
+
+<p>You should normally use this field to refer to [% terms.bugs %] in 
+  <em>other</em> installations. For [% terms.bugs %] in this 
+  installation, it is better to use the "Depends On" and "Blocks"
+  fields.</p>
+
 [% INCLUDE global/footer.html.tmpl %]