179334 - removing + from a flag doesn't clear setter

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[Bradley Baetz (:bbaetz)]

I wanted to get a= for my patch for bug 179321. joel had already ticked both
review boxes, so I moved the + to a ?, and entered justdave's name in the box.

The + did change to a ?, but joel's name ('bugreport') stayed as having set the
flag, and I can't get justdave's name into the bracketed bit.

The second thing may be related to the fact that you cant request a flag from
noone, and then add a name to request it from someone specific

'regression' in new feature.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: fixes problem

Ok, this patch fixes the problem and also addresses:

1. requestee fields for both new and existing flags were called
"requestee-xxx", where xxx was the ID of the flag's type.  This doesn't work
with multiplicable flag types, so I changed the requestee field names to
"requestee_type-xxx" for new flags and "requestee-yyy", where yyy is the ID of
the flag itself, for existing flags.

2. The name of the requestee was lost when existing flags were re-requested.

3. There was some unnecessary extra validation code in Flag.pm that validated
requestee fields that we had already validated.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2

This patch also adds a fix for bug 179236, which is best fixed together with
this bug since they are modifying the same code.  interdiff to see the specific
fix.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3

Fixes additional problems, like the inability to change the requestee for a
requested flag.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v4: more updates

More updates to fix a variety of bugs and implement some features.

Comment 5

[Bradley Baetz (:bbaetz)]

Comment on attachment 105824 [details] [diff] [review]
patch v4: more updates

>@@ -473,11 +502,11 @@
>         &::ThrowTemplateError($::template->error());
>     }
>     
>-    my $delivery_mode = Param("sendmailnow") ? "" : "-ODeliveryMode=deferred";
>-    open(SENDMAIL, "|/usr/lib/sendmail $delivery_mode -t -i") 
>-      || die "Can't open sendmail";
>-    print SENDMAIL $message;
>-    close(SENDMAIL);
>+#    my $delivery_mode = Param("sendmailnow") ? "" : "-ODeliveryMode=deferred";
>+#    open(SENDMAIL, "|/usr/lib/sendmail $delivery_mode -t -i") 
>+#      || die "Can't open sendmail";
>+#    print SENDMAIL $message;
>+#    close(SENDMAIL);
> }
> 

Drop that hunk, and r=bbaetz. It seems to work on the test installation, but
I'm sure someone will find a bug of some sort... ;)

Comment 6

[Myk Melez [:myk] [@mykmelez]]

a=myk

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.31; previous revision: 1.30
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.166; previous revision: 1.165
done
Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.4; previous revision: 1.3
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/flag/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/flag/list.html.tmpl,v 
<--  list.html.tmpl
new revision: 1.6; previous revision: 1.5
done