From d73a83506def7c6b43e5ad720777ec700f6af2e8 Mon Sep 17 00:00:00 2001 From: Jesse Clark Date: Sun, 7 Feb 2010 16:10:03 -0800 Subject: Bug 251556: Allow "Bug ID" fields to have one-way mutual relationships (like blocks/dependson) r=mkanat, a=mkanat --- .bzrignore | 1 + Bugzilla/Bug.pm | 47 ++++++++++++++- Bugzilla/DB/Schema.pm | 1 + Bugzilla/Field.pm | 66 ++++++++++++++++++++++ Bugzilla/Install/DB.pm | 3 + editfields.cgi | 2 + skins/standard/admin.css | 12 ++++ .../en/default/admin/custom_fields/cf-js.js.tmpl | 10 ++++ .../default/admin/custom_fields/create.html.tmpl | 37 ++++++++---- .../en/default/admin/custom_fields/edit.html.tmpl | 36 +++++++++--- template/en/default/bug/edit.html.tmpl | 8 ++- template/en/default/bug/field.html.tmpl | 23 ++++++-- template/en/default/bug/show-multiple.html.tmpl | 7 +++ template/en/default/global/user-error.html.tmpl | 6 ++ 14 files changed, 233 insertions(+), 26 deletions(-) diff --git a/.bzrignore b/.bzrignore index e42398ecc..8415e356b 100644 --- a/.bzrignore +++ b/.bzrignore @@ -29,3 +29,4 @@ /skins/contrib/Dusk/summarize-time.css /skins/contrib/Dusk/voting.css /skins/contrib/Dusk/yui +.DS_Store diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index fd28b5b82..1e418aa18 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -25,6 +25,7 @@ # Max Kanat-Alexander # Frédéric Buclin # Lance Larsh +# Elliotte Martin package Bugzilla::Bug; @@ -1761,7 +1762,42 @@ sub _check_select_field { sub _check_bugid_field { my ($invocant, $value, $field) = @_; return undef if !$value; - return $invocant->check($value, $field)->id; + + # check that the value is a valid, visible bug id + my $checked_id = $invocant->check($value, $field)->id; + + # check for loop (can't have a loop if this is a new bug) + if (ref $invocant) { + _check_relationship_loop($field, $invocant->bug_id, $checked_id); + } + + return $checked_id; +} + +sub _check_relationship_loop { + # Generates a dependency tree for a given bug. Calls itself recursively + # to generate sub-trees for the bug's dependencies. + my ($field, $bug_id, $dep_id, $ids) = @_; + + # Don't do anything if this bug doesn't have any dependencies. + return unless defined($dep_id); + + # Check whether we have seen this bug yet + $ids = {} unless defined $ids; + $ids->{$bug_id} = 1; + if ($ids->{$dep_id}) { + ThrowUserError("relationship_loop_single", { + 'bug_id' => $bug_id, + 'dep_id' => $dep_id, + 'field_name' => $field}); + } + + # Get this dependency's record from the database + my $dbh = Bugzilla->dbh; + my $next_dep_id = $dbh->selectrow_array( + "SELECT $field FROM bugs WHERE bug_id = ?", undef, $dep_id); + + _check_relationship_loop($field, $dep_id, $next_dep_id, $ids); } ##################################################################### @@ -2553,6 +2589,15 @@ sub blocked { # Even bugs in an error state always have a bug_id. sub bug_id { $_[0]->{'bug_id'}; } +sub related_bugs { + my ($self, $relationship) = @_; + return [] if $self->{'error'}; + + my $field_name = $relationship->name; + $self->{'related_bugs'}->{$field_name} ||= $self->match({$field_name => $self->id}); + return $self->{'related_bugs'}->{$field_name}; +} + sub cc { my ($self) = @_; return $self->{'cc'} if exists $self->{'cc'}; diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index 8bee5dfe1..44d224d57 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -693,6 +693,7 @@ use constant ABSTRACT_SCHEMA => { value_field_id => {TYPE => 'INT3', REFERENCES => {TABLE => 'fielddefs', COLUMN => 'id'}}, + reverse_desc => {TYPE => 'TINYTEXT'}, ], INDEXES => [ fielddefs_name_idx => {FIELDS => ['name'], diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index 2f14037ab..17e4194c2 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -101,6 +101,7 @@ use constant DB_COLUMNS => qw( visibility_field_id visibility_value_id value_field_id + reverse_desc ); use constant REQUIRED_CREATE_FIELDS => qw(name description); @@ -120,6 +121,7 @@ use constant VALIDATORS => { use constant UPDATE_VALIDATORS => { value_field_id => \&_check_value_field_id, visibility_value_id => \&_check_control_value, + reverse_desc => \&_check_reverse_desc, }; use constant UPDATE_COLUMNS => qw( @@ -132,6 +134,7 @@ use constant UPDATE_COLUMNS => qw( visibility_field_id visibility_value_id value_field_id + reverse_desc type ); @@ -357,6 +360,22 @@ sub _check_control_value { return $value_obj->id; } +sub _check_reverse_desc { + my ($invocant, $reverse_desc, $type) = @_; + + if (blessed $invocant) { + $type = $invocant->type; + } + + if ($type != FIELD_TYPE_BUG_ID) { + return undef; # store NULL for non-reversible field types + } + + $reverse_desc = clean_text($reverse_desc); + return $reverse_desc; +} + + =pod =head2 Instance Properties @@ -637,6 +656,44 @@ sub controls_values_of { return $self->{controls_values_of}; } +=over + +=item C + +Applies only to fields of type FIELD_TYPE_BUG_ID. +Checks to see if a reverse relationship description has been set. +This is the canonical condition to enable reverse link display, +dependency tree display, and similar functionality. + +=back + +=cut + +sub is_relationship { + my $self = shift; + my $desc = $self->reverse_desc; + if (defined $desc && $desc ne "") { + return 1; + } + return 0; +} + +=over + +=item C + +Applies only to fields of type FIELD_TYPE_BUG_ID. +Describes the reverse relationship of this field. +For example, if a BUG_ID field is called "Is a duplicate of", +the reverse description would be "Duplicates of this bug". + +=back + +=cut + +sub reverse_desc { return $_[0]->{reverse_desc} } + + =pod =head2 Instance Mutators @@ -661,6 +718,8 @@ They will throw an error if you try to set the values to something invalid. =item C +=item C + =item C =item C @@ -677,6 +736,7 @@ sub set_obsolete { $_[0]->set('obsolete', $_[1]); } sub set_sortkey { $_[0]->set('sortkey', $_[1]); } sub set_in_new_bugmail { $_[0]->set('mailhead', $_[1]); } sub set_buglist { $_[0]->set('buglist', $_[1]); } +sub set_reverse_desc { $_[0]->set('reverse_desc', $_[1]); } sub set_visibility_field { my ($self, $value) = @_; $self->set('visibility_field_id', $value); @@ -868,6 +928,12 @@ sub run_create_validators { $class->_check_value_field_id($params->{value_field_id}, ($type == FIELD_TYPE_SINGLE_SELECT || $type == FIELD_TYPE_MULTI_SELECT) ? 1 : 0); + + $params->{reverse_desc} = $class->_check_reverse_desc( + $params->{reverse_desc}, $type); + + + return $params; } diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index f6d6edcb1..d150a4e9b 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -102,6 +102,9 @@ sub update_fielddefs_definition { $dbh->do('UPDATE fielddefs SET buglist = 1 WHERE custom = 1 AND type != ' . FIELD_TYPE_MULTI_SELECT); } + #2008-08-26 elliotte_martin@yahoo.com - Bug 251556 + $dbh->bz_add_column('fielddefs', 'reverse_desc', {TYPE => 'TINYTEXT'}); + # Remember, this is not the function for adding general table changes. # That is below. Add new changes to the fielddefs table above this diff --git a/editfields.cgi b/editfields.cgi index 09147d69a..4e8733752 100755 --- a/editfields.cgi +++ b/editfields.cgi @@ -68,6 +68,7 @@ elsif ($action eq 'new') { visibility_field_id => scalar $cgi->param('visibility_field_id'), visibility_value_id => scalar $cgi->param('visibility_value_id'), value_field_id => scalar $cgi->param('value_field_id'), + reverse_desc => scalar $cgi->param('reverse_desc'), }); delete_token($token); @@ -113,6 +114,7 @@ elsif ($action eq 'update') { $field->set_visibility_field($cgi->param('visibility_field_id')); $field->set_visibility_value($cgi->param('visibility_value_id')); $field->set_value_field($cgi->param('value_field_id')); + $field->set_reverse_desc($cgi->param('reverse_desc')); $field->update(); delete_token($token); diff --git a/skins/standard/admin.css b/skins/standard/admin.css index 184c4961a..6b330ef6d 100644 --- a/skins/standard/admin.css +++ b/skins/standard/admin.css @@ -113,3 +113,15 @@ th.title { text-align: center; vertical-align: middle; } + +#edit_custom_field tr { + vertical-align: top; +} + +#edit_custom_field th { + text-align: right; +} + +#edit_custom_field th.narrow_label { + white-space: normal; +} diff --git a/template/en/default/admin/custom_fields/cf-js.js.tmpl b/template/en/default/admin/custom_fields/cf-js.js.tmpl index 778dd3373..4c95a1690 100644 --- a/template/en/default/admin/custom_fields/cf-js.js.tmpl +++ b/template/en/default/admin/custom_fields/cf-js.js.tmpl @@ -50,6 +50,16 @@ function onChangeType(type_field) { else { value_field.disabled = true; } + + var reverse_desc = document.getElementById('reverse_desc'); + if (type_field.value == [% constants.FIELD_TYPE_BUG_ID %]) + { + reverse_desc.disabled = false; + } + else { + reverse_desc.disabled = true; + reverse_desc.value = ''; + } } function onChangeVisibilityField() { diff --git a/template/en/default/admin/custom_fields/create.html.tmpl b/template/en/default/admin/custom_fields/create.html.tmpl index a2db4708b..d23be0fe2 100644 --- a/template/en/default/admin/custom_fields/create.html.tmpl +++ b/template/en/default/admin/custom_fields/create.html.tmpl @@ -28,8 +28,14 @@ onload = "document.getElementById('new_bugmail').disabled = true;" javascript_urls = [ 'js/util.js' ] doc_section = "custom-fields.html#add-custom-fields" + style_urls = ['skins/standard/admin.css'] %] +[%# set initial editability of fields such as Reverse Relationship Description %] + +

Adding custom fields can make the interface of [% terms.Bugzilla %] very complicated. Many admins who are new to [% terms.Bugzilla %] start off @@ -48,14 +54,14 @@

- +
- + - - + - - + - + - + - - + + - [% PROCESS bug/field.html.tmpl value=bug.${field.name} + [% PROCESS bug/field.html.tmpl value = bug.${field.name} editable = bug.check_can_change_field(field.name, 0, 1) value_span = 2 %] + [% IF extra_field_item %] + + + + + [% END %] [% END %] [% END %] diff --git a/template/en/default/bug/field.html.tmpl b/template/en/default/bug/field.html.tmpl index ac62bf7ba..482f88219 100644 --- a/template/en/default/bug/field.html.tmpl +++ b/template/en/default/bug/field.html.tmpl @@ -99,8 +99,8 @@ value="[% value FILTER html %]" size="7"> - [% IF bug.${field.name} %] - [% bug.${field.name} FILTER bug_link(bug.${field.name}) FILTER none %] + [% IF value %] + [% value FILTER bug_link(value, use_alias => 1) FILTER none %] [% END %] (edit) @@ -110,7 +110,7 @@ '[% field.name FILTER js %]_input_area', '[% field.name FILTER js %]_edit_action', '[% field.name FILTER js %]', - "[% bug.${field.name} FILTER js %]"); + "[% value FILTER js %]"); [% CASE [ constants.FIELD_TYPE_SINGLE_SELECT constants.FIELD_TYPE_MULTI_SELECT ] %] @@ -198,11 +198,24 @@
[% value FILTER wrap_comment(60) FILTER html %]
[% ELSIF field.type == constants.FIELD_TYPE_BUG_ID %] - [% IF bug.${field.name} %] - [% bug.${field.name} FILTER bug_link(bug.${field.name}) FILTER none %] + [% IF value %] + [% value FILTER bug_link(value, use_alias => 1) FILTER none %] [% END %] [% ELSE %] [% value.join(', ') FILTER html %] [% END %] [% Hook.process('end_field_column') %] [% '' IF NOT no_tds %] + +[%# for reverse relationships, we show this pseudo-field after the main field %] +[% IF bug.id && field.is_relationship %] + [% extra_field_item = {} %] + [% extra_field_item.header = field.reverse_desc _ ":" FILTER html %] + [% extra_field_item.data = BLOCK %] + [% FOREACH depbug = bug.related_bugs(field) %] + [% depbug.id FILTER bug_link(depbug, use_alias => 1) FILTER none %][% " " %] + [% END %] + [% END %] +[% ELSE %] + [% extra_field_item = '' %] +[% END %] diff --git a/template/en/default/bug/show-multiple.html.tmpl b/template/en/default/bug/show-multiple.html.tmpl index 177bea14f..0949ffc20 100644 --- a/template/en/default/bug/show-multiple.html.tmpl +++ b/template/en/default/bug/show-multiple.html.tmpl @@ -186,6 +186,13 @@ [% PROCESS bug/field.html.tmpl value=bug.${field.name} editable=0 %] [%# Even-numbered fields get a closing %] [% '' IF !(field_counter % 2) %] + [% IF extra_field_item %] + [% field_counter = field_counter + 1 %] + [% '' IF field_counter % 2 %] + + + [% '' IF !(field_counter % 2) %] + [% END %] [% END %] [%# And we have to finish the row if we ended on an odd number. %] [% '' IF field_counter % 2 %] diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index eca7ef058..bdedc4172 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -1450,6 +1450,12 @@ To file this [% terms.bug %], you must first choose a component. If necessary, just guess. + [% ELSIF error == "relationship_loop_single" %] + [% title = "Relationship Loop Detected" %] + [% field_descs.$field_name FILTER html %] + for [% terms.bug %] [%+ bug_id FILTER html %] + has a circular dependency on [% terms.bug %] [%+ dep_id FILTER html %]. + [% ELSIF error == "require_new_password" %] [% title = "New Password Needed" %] You cannot change your password without choosing a new one. -- cgit v1.2.3-24-g4f1b
+ @@ -64,16 +70,16 @@
+
+ @@ -114,7 +120,18 @@
  + + + +
+ Use this label for the list of [% terms.bugs %] that link to a [% terms.bug %] + with this [% field_types.${constants.FIELD_TYPE_BUG_ID} %] field. + For example, if the description is "Is a duplicate of", + the reverse description would be "Duplicates of this [% terms.bug %]". + Leave blank to disable the list for this field. +
[% extra_field_item.header FILTER none %][% extra_field_item.data FILTER none %]
[% extra_field_item.header FILTER none %][% extra_field_item.data FILTER none %]