Roundup Tracker - Issues

Issue 2550926

classification
Title: Original author adding a second message shouldn't set status to 'chatting'
Type: behavior Severity: minor
Components: Web interface, Mail interface Versions: 2.0.0alpha
process
Status: fixed Resolution: fixed
Dependencies: Superseder:
Assigned To: rouilj Nosy List: ThomasAH, ber, justus.winter, rouilj
Priority: normal Keywords: patch

Created on 2016-09-27 09:00 by justus.winter, last changed 2019-11-08 10:01 by ThomasAH.

Files
File name Uploaded Description Edit Remove
chat_only_on_new_user.patch rouilj, 2016-10-01 20:09
Messages
msg5902 Author: [hidden] (justus.winter) Date: 2016-09-27 09:00
This is an issue that I find annoying at our Roundup installation.  Bug
reporters often add another message because they forgot some
information, and then the status is set to 'chatting'.

This matches neither my expectations nor the user guide, which describes
"chatting" as "under review or seeking clarification".
msg5903 Author: [hidden] (rouilj) Date: 2016-09-28 02:28
Hi Justus:

Would the following logic work for you:

  when receiving a new message on an issue, 
     check to see if the status of the issue is new (1)
     check to see if the submitter of the first message is the same as
         the new message
     if so keep the status as new
     if not change the status to chatting

This assumes that the "new" status is never reassigned. If it is
reassigned, then I guess scanning the history would be required.
So (1) would then read:

   check if the status of the issue is new and if
      the status has always been new (i.e. never changed)

so once the status of the ticket is changed, any email from anybody
would change it to chatting from new, done-cbb etc.

-- rouilj
msg5904 Author: [hidden] (justus.winter) Date: 2016-09-29 08:53
The simpler logic you described sounds just fine.  Thanks :)
msg5905 Author: [hidden] (ber) Date: 2016-09-30 13:56
Hi Justus, hi John,

the code for these state changes is part of the configuration,
thus it is part of the standard template bugs.gnupg.org is using.
roundup/share/roundup/templates/classic/detectors/statusauditor.py

You could change the logic in there for your tracker.
Some trackers switch it off alltogether. 

I think "chatting" often means that some activity has started on the
issue and action is needed. This is why "resolved" and "done-cc" switch
to it. When searching for new or chatting issues you should find all
that require user input.

For the use in bugs.gnupg.org where most users can only modify the
issues they have created themselfs, I agree that it may make sense to
not switch
to chatting if the status is new and the new message comes from the creator
of the issue. If you peek at statusauditor.py you probably figure out how
to add that logic.

So I'm unsure if we should change the default for roundups distributions.
Regards,
Bernhard
msg5906 Author: [hidden] (rouilj) Date: 2016-10-01 20:09
Hi Justus:

I have added a patch to statusauditor.py that implements the first
option discussed above.

I am assuming you are using an unmodified version of the classic tracker
statusauditor.py file.

To test it, cd to your tracker's detectors directory and run:

 patch < chat_only_on_new_user.patch

There is a variable:

 allow_multiple_initial_emails

at the beginning of the chatty function that is set to False.
To test the new functionality, you must set that variable to True
and restart your tracker.

Then you should be able to create a new issue (by email or browser).
As long as:

 the state of the issue is 'unread'

and

 the person who added the first message to the issue is the person
 updating the issue

the status should remain 'unread'. If somebody else adds a message,
the state will change to 'chatting'. If the starting state is done-cbb
or resolved, the state will be changed to chatting regardless of the
user updating the issue.

If this looks good, I will consider adding it to the roundup release,
but the 'allow_multiple_initial_emails' will be set to False so it's
disabled.

This allows people to upgrade but not have any change to their system
unless they take a specific step to enable it.

Also I am looking for a new name for 'allow_multiple_initial_emails'.
That's not exactly a great description of what it does. So suggestions
welcome.

Bern I think this is a useful patch to add, but I agree with you that it
should be disabled by default. One thing I am considering is if we
should ship a config.ini in the detectors directory and have optional
features such as this be enabled via the ini file. There are no
examples of this AFAIK, but

 
http://roundup.sourceforge.net/docs/customizing.html#extending-the-configuration-file

and

   https://sourceforge.net/p/roundup/mailman/message/11511368/

indicate this should work.

I think having a detectors/config.ini file is a good mechanism for addon
authors to use to customize the code and we could provide a web
interface to the ini files so that administrators can manage features
through the web interface.

Also Bern one other feature I have been asked about is preventing
reopening after some number of days. So if he issue is in the
resolved state and an update comes in say 30 days later,
the issue is not reopened.

I'll open a new issue for this as there are a number of questions about
how this should function.

-- rouilj
msg5908 Author: [hidden] (justus.winter) Date: 2016-10-04 09:12
Hi Bernhard, hey John :)

I didn't realize that this logic is part of the configuration/state. 
I'm new to roundup.

John, thanks for whipping up the patch.  I have applied it to our tracker.

Thanks for your time,
Justus
msg5909 Author: [hidden] (ber) Date: 2016-10-05 20:09
Am Samstag, 1. Oktober 2016 22:09:51 schrieb John Rouillard:
> Bern I think this is a useful patch to add, but I agree with you that it
> should be disabled by default. 

Fine with me.

> One thing I am considering is if we 
> should ship a config.ini in the detectors directory and have optional
> features such as this be enabled via the ini file.

This is a question probably best brought up on the devel mailinglist as well.
msg6796 Author: [hidden] (ThomasAH) Date: 2019-11-06 07:02
Related to this:

- I often create issues that I assign to myself. Here the status
  "unread" doesn't apply, because I already read it.

- Users often create issues without assigning them to anyone, because
  it is not their job to do that. When I'm assigning the issue e.g.
  to ber with a comment like "Bernhard, can you take care of this?"
  This sets the status from unread to chatting, but the one who would
  do the real work on the issue did not read it.
  Or a second user would say "I have the same problem", causing the
  status to be set to chatting.
  So my question is: Should chatting (optionally) only be set if the
  message is sent by the person in the assignedto field?

(Regarding detectors/config.ini: I'll comment on the -devel list)
msg6797 Author: [hidden] (ThomasAH) Date: 2019-11-06 07:23
Regarding the patch:
You're using db.msg.get(firstmessageid, 'author'), but the issue
itself already has an author attribute, which I think should be
used instead.

Rationale:
1. The user might have created the issue without a message. If now the
   user provides more details it should not be chatting, but if someone
   else reacts, chatting should be set.
2. The user might have unlinked (removed) the first message from the
   issue (e.g. to provide a better initial description)
3. Don't access parts of the database you don't really need :)
msg6798 Author: [hidden] (ThomasAH) Date: 2019-11-06 07:27
Clarification: the issue already has a "creator" attribute
msg6799 Author: [hidden] (rouilj) Date: 2019-11-07 02:15
In message <1573025028.22.0.786040366562.issue2550926@roundup.psfhosted.org>,
Thomas Arendsen Hein writes:

>Regarding the patch:
>You're using db.msg.get(firstmessageid, 'author'), but the issue
>itself already has an author attribute, which I think should be
>used instead.

Code changed to use creator (same as author of first message
usually). Good idea.
msg6800 Author: [hidden] (rouilj) Date: 2019-11-07 02:32
Hi Thomas:

In message <1573023745.09.0.541804250831.issue2550926@roundup.psfhosted.org>,
Thomas Arendsen Hein writes:
>Related to this:
>
>- I often create issues that I assign to myself. Here the status
>  "unread" doesn't apply, because I already read it.

Well the status doesn't make any sense in this use case. So maybe
change the 'unread' status to 'new' in the db and make a corresponding
change in the detector.

>- Users often create issues without assigning them to anyone, because
>  it is not their job to do that. When I'm assigning the issue e.g.
>  to ber with a comment like "Bernhard, can you take care of this?"
>  This sets the status from unread to chatting, but the one who would
>  do the real work on the issue did not read it.

If you put Bern on the nosy list, and then make the update he will see
it right? But I see what you mean, he will get the "Bern can you take
care of this" message not the original opening message. In my tracker
when the assignedto is changed, an email with the first and last three
messages is sent to the new assignedto person.

>  Or a second user would say "I have the same problem", causing the
>  status to be set to chatting.
>  So my question is: Should chatting (optionally) only be set if the
>  message is sent by the person in the assignedto field?

I don't know about that. We have a number of discussions going on in
the roundup tracker that have no assignedto party. Note the tracker
uses new, open etc not unread, but if it were to use unread, the first
response from sombody would still be a chat. I can see your assignedto
requirement being valid business logic though. Especially if the first
employee who responds to an issue gets assigned.

I think these would be good examples for the wiki.
msg6801 Author: [hidden] (ThomasAH) Date: 2019-11-07 06:53
Copy&Paste of the naming discussion from the -devel list:

* John P. Rouillard <rouilj@cs.umb.edu> [20191107 03:13]:
> In message <20191106074249.193392358.thomas@intevation.de>,
> Thomas Arendsen Hein writes:
> >"chating_in_new_user" (based on the patch name in the issue, I guess
> >you meant "chatting_on_new_user") needs a better name though.
> >I suggest "chatting_ignore_issue_creator" and inverting the logic,
> >because "chatting_on_issue_creator" might more easily imply that
> >chatting is already set when creating the issue.
> 
> In the patch the variable that enables this feature is called:
> 
>   allow_multiple_initial_emails
> 
> so all initial emails (as determined by issue creator/ message author
> match) will keep the status at unread.

Unless I'm reading the patch wrong it does not check if the new
message is another initial message or is just a regular reply by the
issue creator (after someone else wrote something and changed the
status back to unread)

> Alternate names:
> 
>   chatting_requires_two_people

I really like that, though I'd recommend "users" instead of
"peeople", because users is what Roundup knows about:

chatting_requires_two_users

>   chatting_status_on_second_user_update

-1 on that

> I really need an entire sentence to make it clear. I am considering
> calling it Fred and just adding:

+1 for Fred ;-)

>    [statusauditor]
>    # Options for statusauditor.py
>    #
>    # Assume an issue has a status of unread and was created by the
>    # demo user.
>    # If False, a new message on an issue sets status to chatting.
>    # If True, a new message sets the status to chatting only if the
>    #   author of the new message is not demo. A message sent by demo
>    #   keeps the issue status at unread.
>    fred = False                                        

An alternative could be to include the value in the "sentence",
something like:

[statusauditor]
# Options for statusauditor.py
#
# Automatically set an issue with the current status "unread" to
# "chatting" if a new message is added.
# Possible settings:
#   never:
#     Disable this feature
#   always:
#     Do this for any message
#   others:
#     Do this only if users other than the initial creator of the
#     issue send a reply
#   assignedto:
#     Do this only if the user who is listed in the assignedto
#     attribute of the issue replies
auto_chatting = {never,always,others,assignedto}
msg6802 Author: [hidden] (rouilj) Date: 2019-11-07 22:33
Hi Thomas:

In message <1573109597.54.0.247286385558.issue2550926@roundup.psfhosted.org>,
Thomas Arendsen Hein writes:
>> Alternate names:
>> 
>>   chatting_requires_two_people
>
>I really like that, though I'd recommend "users" instead of
>"peeople", because users is what Roundup knows about:
>chatting_requires_two_users

That works for me.

>> I really need an entire sentence to make it clear. I am considering
>> calling it Fred and just adding:
>
>+1 for Fred ;-)

Sorry Fred.

>>    [statusauditor]
>>    # Options for statusauditor.py
>>    #
>>    # Assume an issue has a status of unread and was created by the
>>    # demo user.
>>    # If False, a new message on an issue sets status to chatting.
>>    # If True, a new message sets the status to chatting only if the
>>    #   author of the new message is not demo. A message sent by demo
>>    #   keeps the issue status at unread.
>>    fred = False                                        
>
>An alternative could be to include the value in the "sentence",
>something like:
>
>[statusauditor]
># Options for statusauditor.py
>#
># Automatically set an issue with the current status "unread" to
># "chatting" if a new message is added.
># Possible settings:
>#   never:
>#     Disable this feature
>#   always:
>#     Do this for any message
>#   others:
>#     Do this only if users other than the initial creator of the
>#     issue send a reply
>#   assignedto:
>#     Do this only if the user who is listed in the assignedto
>#     attribute of the issue replies
>auto_chatting = {never,always,others,assignedto}

That would be a nice enhancement, but I want to get this ticket
closed.  Adding these enhancements lso includes questions like:

  how to control chatting from none, done-cbb and closed statuses
  (more values in the option, or more options??)

  requests to prevent autoopen/chatting if ticket has been
     closed for more than N days

  "thank you" detection to prevent reopening for a thank you note.

Each of these could be a separate enhancement.

If your suggested enhancement is implemented, it can be backwards
compatible with:

   never:
     Disable this feature
   always (or chatting_requires_two_users=False):
     Do this for any message
   others (or chatting_requires_two_users=True):
     Do this only if users other than the initial creator of the
     issue send a reply
   assignedto:
    Do this only if the user who is listed in the assignedto
     attribute of the issue replies

I'll try to get a final cut of chatting_requires_two_users as speced
checked in tonight.
msg6803 Author: [hidden] (rouilj) Date: 2019-11-07 23:37
rev5971:e5acd1843517 has patches including upgrading.txt directions.
Closing.
msg6804 Author: [hidden] (ThomasAH) Date: 2019-11-08 10:01
* John Rouillard <issues@roundup-tracker.org> [20191107 23:33]:
> I'll try to get a final cut of chatting_requires_two_users as speced
> checked in tonight.

OK, sounds good enough. Thank you!
History
Date User Action Args
2019-11-08 10:01:31ThomasAHsetmessages: + msg6804
2019-11-07 23:37:15rouiljsetstatus: open -> fixed
versions: + 2.0.0alpha
messages: + msg6803
assignee: rouilj
components: + Web interface, Mail interface
resolution: fixed
2019-11-07 22:33:09rouiljsetmessages: + msg6802
2019-11-07 06:53:17ThomasAHsetmessages: + msg6801
2019-11-07 02:32:29rouiljsetmessages: + msg6800
2019-11-07 02:15:16rouiljsetmessages: + msg6799
2019-11-06 07:27:10ThomasAHsetmessages: + msg6798
2019-11-06 07:23:48ThomasAHsetmessages: + msg6797
2019-11-06 07:02:25ThomasAHsetnosy: + ThomasAH
messages: + msg6796
2019-05-17 00:08:01rouiljsetpriority: normal
2018-01-15 01:27:48rouiljsetstatus: new -> open
2016-10-05 20:09:29bersetmessages: + msg5909
2016-10-04 09:12:38justus.wintersetmessages: + msg5908
2016-10-01 20:09:50rouiljsetfiles: + chat_only_on_new_user.patch
keywords: + patch
messages: + msg5906
2016-09-30 13:56:58bersetnosy: + ber
messages: + msg5905
2016-09-29 08:53:14justus.wintersetmessages: + msg5904
2016-09-28 02:28:50rouiljsetnosy: + rouilj
messages: + msg5903
2016-09-27 09:00:31justus.wintercreate