*** Bug 422341 has been marked as a duplicate of this bug. ***

LOC Estimation:

This is going to be revised from its current implementation as I don't
like the current implementation :) So I am going to do this as FP estimation.

Inputs : multi edit form (status change to NEEDINFO), single edit form (status
change to NEEDINFO), xmlrpc method (changeStatus), xmlrpc method (updateFlags),
standard flag interface = 4
Outputs : user interface acknowledgement of change, xmlrpc response = 2
Inquiries = check current needinfo flag status for given bug = 1
Internal Logical files : flags table = 1
External Logical files : 0

FP (total count) = (4*4)+(5*2)+(4*1)+(10*1)+(0)
                    16 + 10 + 4 + 10 + 0 = 40

FP = 40 * 1.11 = 45

LOC = 45 * 60 = 2700

Selenium tests to verify proper operation of needinfo in UI and XMLRPC: 12 hours

Targetting milestone 3.

Survey required. Need to sell it to Gampe.

Paul,

can you comment on this bug please? 

The goal here is to simplify and return to the upstream behaviour around 
needinfo. Red Hat has an explicit state but the upstream sets a flag on any 
state to indicate that it needs info. Do you have any objection to this? Or 
commentary on how Red Hat got to have a NEEDINFO state?

thanks

Kev

I never liked the NEEDINFO state and will happily see it removed.  We will 
need to document and advise folks on the use of the flag though to ensure 
everyone understands this change is not a loss of meta-data.

(In reply to comment #5)
> I never liked the NEEDINFO state and will happily see it removed.  We will 
> need to document and advise folks on the use of the flag though to ensure 
> everyone understands this change is not a loss of meta-data.

We could use some simple javascript to make it more obvious than having a simple
flag but on the backend it would just be setting it to needinfo? with a
particular  requestee. And then clear the flag if comment was made.

Dave

Created attachment 304423 [details]
Patch to add needinfo actor support (v1)

Attaching a patch to add needinfo support to bugzilla 3. Here is what is
happening with this patch.

1. If bug is currently not needing any information, a form section is displayed
below the comment box asking if "Additional information is needed from
<someone>".
2. The use either selects anyone, assignee, reporter, qa contact or they can
enter a specific email address.
3. When the user submits the change, it automatically sets the needinfo? flag
with the requestee set to the proper person or blank if anyone can answer.
4. If the person that is being requested of more information goes to the bug
report and adds a comment, then needinfo? flag is cleared and the comment is
added normally. If anyone can answer then anyone's comment will clear it.
5. If a specific person visits the bug that is not the requestee for the
needinfo? flag, they can check a box under the comment field stating that they 

are providing the needed information and this will clear the needinfo? flag.
6. If the requestee (or anyone if anyone can answer) attaches a new attachment
or comments on an existing attachmentto, the needinfo? flag will be cleared.
7. If the bug is closed while a needinfo? flag is set, it will clear the flag
so we don't have it lingering in the database for dead bugs.

I could streamline this more with the use of javascript such as filling in the
text box with the proper email address based on the drop down selected. This
would cut down on code needed in process_bug.cgi. But the way I have it
implemented currently works with or without javascript. 

The code is not as clean as it could be as with 3.x the flag processing code is
still handled outside of Bugzilla/Bug.pm and relies on values to be set in the
CGI object to function properly.

Please take some time to review and also provide feedback on the UI itself as
this is different than 2.18. This is also allowing us to get rid of the
NEEDINFO status completely.

Thanks
Dave

Comment on attachment 304423 [details]
Patch to add needinfo actor support (v1)

Paul, can you comment on the UI portion of this enhancement?

I forgot to mention that this is running currently on
https://bugdev.devel.redhat.com/bugzilla for you to test out.

Dave

Comment on attachment 304423 [details]
Patch to add needinfo actor support (v1)

Hi Dave,

The patch works perfectly as how you described, I just have few points that i
would like to discuss:

1- shall we have in attachments.cgi page a checkbox to say 
'I am providing the requested information for this bug.'
to appear for users who are not the requestees in the needinfo flag just as how
you have it in showbug.cgi under the comment box?

2- shall we also enable setting the needinfo flag  when changing several bugs
at once but in this case i guess we will not be able to specify that the
needinfo can be from bug assignee, qa contact of reporter as they will be
different for each bug but maybe we can enable just specifying a certain user
or 'anyone' option.

3- I guess for all the existing bugs with NEEDINFO status at the time of the
upgrade we will set their statuses to the statuses they had before the NEEDINFO
status and they will already have the needinfo fal set to '?', then we can set
the NEEDINFO status's isactive field to 0. if that would be the case then i can
create a separate bug for this task and start writing script to be included in
the migration process.

Noura

(In reply to comment #10)
> (From update of attachment 304423 [details] [edit])
> Hi Dave,
> 
> The patch works perfectly as how you described, I just have few points that i
> would like to discuss:
> 
> 1- shall we have in attachments.cgi page a checkbox to say 
> 'I am providing the requested information for this bug.'
> to appear for users who are not the requestees in the needinfo flag just as how
> you have it in showbug.cgi under the comment box?

Good idea. This has been requested before with 2.18 as well as people consider
adding an attachment to a needinfo bug as providing requested information just
like adding a comment. I will create a new patch with this added.

> 2- shall we also enable setting the needinfo flag  when changing several bugs
> at once but in this case i guess we will not be able to specify that the
> needinfo can be from bug assignee, qa contact of reporter as they will be
> different for each bug but maybe we can enable just specifying a certain user
> or 'anyone' option.

Currently the code does not support any flag changes from buglist.cgi and is not
likely to make it into 3.2 according to upstream. So this may prove to be
non-trivial to add since flags currently are only set up for single bug changes.
Unless we add a hack that specifically only does the changes for the 'needinfo?'
flag only.
 
> 3- I guess for all the existing bugs with NEEDINFO status at the time of the
> upgrade we will set their statuses to the statuses they had before the NEEDINFO
> status and they will already have the needinfo fal set to '?', then we can set
> the NEEDINFO status's isactive field to 0. if that would be the case then i can
> create a separate bug for this task and start writing script to be included in
> the migration process.

Yes, good idea. Definitely something that will need to be done before we go
live. Go ahead and create a bug for this task and see how much work the script
would be. We will need to add an activity entry to bugs_activity for the status
change back to the previous state.

Thanks for the review
Dave

Created attachment 305183 [details]
Patch to add needinfo actor support (v2)

All the same as previous patch except this adds the override checkbox on the
attachment edit page so that a non-requestee can specify that they are
providing the requested information.

TODO:
- Need to still add code to update needinfo flag if a new attachment is made
from the requestee or provide override checkbox.
- Add a little jscript to edit bug page that will auto-check the needinfo
checkbox if the drop down for actor is selected or the other text box is
focused.

Please review the new change.
Dave

Created attachment 305190 [details]
bz3 screen shot

(In reply to comment #8)
> (From update of attachment 304423 [details] [edit])
> Paul, can you comment on the UI portion of this enhancement?
> 

I did a test with the bugdev and note that currently the only way to tell if a 
bug is in needinfo is by looking at the Flags: section at the top of the page. 
See attached.

I may not have followed this thread well so if you are proposing to make that 
more visible then yes I agree that is needed.

Created attachment 305326 [details]
Patch to add needinfo actor support (v3)

New patch for review. Changes since last patch:

1. Added code to update needinfo flag if a new attachment is made
from the requestee or user checks the override checkbox.
2. Added simple javascript to edit bug page that will auto-check the needinfo
checkbox if the drop down for actor is selected or the other text box is
focused.
3. If bug is in needinfo?, the text [NEEDINFO] will show up in the summary line
making it more obvious than before.

Paul, please take another look as soon as you can. Everyone, please review.

Thanks
Dave

Forgot to mention, this latest patch is on bugdev.devel.redhat.com/bugzilla
so you can try it out.

Dave

Hey Dave,

The patch looks good to me, I tested it and the attachment comment addition
works nicely only one little thing in the WebService function Bug.update:

Index: Bugzilla/WebService/Bug.pm

+        $bug->set_needinfo($params->{comment});

This should be:  $bug->set_needinfo($updates->{comment});

Noura

just a note that i did the above fix to bugdev.devel.redhat.com/bugzilla

Created attachment 305378 [details]
Patch to add needinfo actor support (v4)

Updated error in Bugzilla/WebService/Bug.pm.

If looks good then set review+. Still waiting on Pgampes opinion on this
before committing.

Dave

Comment on attachment 305378 [details]
Patch to add needinfo actor support (v4)

looks good to me

Commited to CVS since I want this for beta1. Paul please comment as time allows
on the UI, etc. We will also hopefully get feedback from the testers as well.

Worked 8 hours.

Hi Dave, I checked out the beta site and this looks fine. Thx!