Redmine

Works against 2.4-stable rev 12697

• You missed db migrate file.
• mix tabs and spaces
• please add tests

Toshi MARUYAMA wrote:

• mix tabs and spaces

What is the rule for Redmine ?

Gurvan Le Dromaguet wrote:

Toshi MARUYAMA wrote:

• mix tabs and spaces

What is the rule for Redmine ?

two spaces for indent.

dear all, how do I install this ? patch -p0 for the patch ? What about the rb file ? thank you.. I come from the Java world :)

Samuel Samfra wrote:

dear all, how do I install this ? patch -p0 for the patch ? What about the rb file ? thank you.. I come from the Java world :)

1- apply "patch -p0"
2- copy rb file to db/migrate
3- run "RAILS_ENV=production rake db:migrate"

I have a solution for this which does not require an extra column in the database for this, the information can be gathered from the journal_details

I am mentioning it just in case the reason for ignoring this proposal is the extra column.

Luka Lüdicke wrote:

I have a solution for this which does not require an extra column in the database for this, the information can be gathered from the journal_details

Sounds great. Could you attach your patch to this issue?

Alright, this from how I patched it at our company.

As you can see from the comments, it is a work in progress.
If you like it and want to include it, I can make a "cleaner" diff and add a unit test for the last_assignee method.

(my colleagues really love the feature)

things to look out for:

• the option should not come up if current user und last_assignee are the same
• the option should not come up if last_assignee is nil
• when there is old assignee (new issue), the last_assignee should be the author (TODO)
• the option should not appear in issue category and bulk update

Luka Lüdicke wrote:

If you like it and want to include it, I can make a "cleaner" diff and add a unit test for the last_assignee method.

like I said, if you like it I would make the effort for a clean patch that you could "just" apply...

suggestions for specs

Test Plan¶

Edit Issue¶

1. open issue edit form
2. use select box
3. expect: last assignee is shown in the fist options
4. expect: assignment to that user is successful

bulk edit¶

1. open multiple issues with batch edit
2. expect: successful page load and no last assignee option in assigned_to select box

Issue Category¶

1. open project settings configuration
2. open issue categories tab
3. edit issue category
4. expect: successful page load and no last assignee option in assigned_to select box

New Issue¶

1. create new issue with an assignee
2. open edit page of that issue
3. expect: last assignee shows the author of the issue
4. expect: assignment to the user is successful

I applied the patch “last_assignee_wip.diff” to a standard Debian Stretch Redmine (v. 3.3.1), and it works somehow – only the value in the combo reads

<< <span class="translation_missing"; title="translation missing: en.issue.last_assignee">Last Assignee</span> >>

BTW, it would be cool if the Last Assignee could be added to the Field Permissions in the workflow configuration, i.e. as an additional option for the Assignee field, which would actually force setting the previous assignee whenever the ticket is edited. Think of a typical (for me) use case: developer A needs feedback from reporter R, who has only the permission to set the Assignee field to the previous one. So A just sets the assignee to R. Now, when R edits the ticket, it will automatically be assigned back to A, avoiding all problems of a ticket mistakenly being assigned to another person. Would such an implementation be possible?

Albrecht Dreß wrote:

I applied the patch “last_assignee_wip.diff” to a standard Debian Stretch Redmine (v. 3.3.1), and it works somehow – only the value in the combo reads

It appears that changing in app/helpers/application_helper.rb the line

     s << content_tag('option', "<< #{t('issue.last_assignee', name: User.find_by_id(last_assignee_id).try(:name))} >>",
                      value: last_assignee_id) if (last_assignee_id && (last_assignee_id != User.current.id))

     s << content_tag('option', "<< #{l(:last_assignee, name: User.find_by_id(last_assignee_id).try(:name))} >>",
                      value: last_assignee_id) if (last_assignee_id && (last_assignee_id != User.current.id))

However, now the wrong “last assignee” is displayed…

After printing some debugging output, it seems that the journals in app/models/issue.rb, last_assigned_to are actually sorted as latest first. Replacing

    journals.reverse_each do |j|

    journals.each do |j|

Mizuki ISHIKAWA wrote:

Albrecht Dreß wrote:

However, now the wrong “last assignee” is displayed…

After printing some debugging output, it seems that the journals in app/models/issue.rb, last_assigned_to are actually sorted as latest first. Replacing [...] by [...] seems to do the right thing. Can you confirm this is the proper solution?

In my environment I seemed to work normally even with reverse_each. Therefore, I use reverse_each also in the patch attached by me.

Hmmm, strange. Which Redmine version do you use?

Albrecht Dreß wrote:

Hmmm, strange. Which Redmine version do you use?

I tested on trunk( r17463 ) and 3.4 versions.

When I checked sql on rails console, it looks like ascending order.

  [1] pry(main)> puts Journal.all.to_sql
  SELECT "journals".* FROM "journals" 
  => nil
  [2] pry(main)> issue = Issue.first
  ~~
  [3] pry(main)> puts issue.journals.to_sql
  SELECT "journals".* FROM "journals" WHERE "journals"."journalized_id" = 1 AND "journals"."journalized_type" = 'Issue'
  => nil

thanks

previous_assigned_to method in the patch should return nil if no previous assignee is found, but it returns Journal::ActiveRecord_Associations_CollectionProxy which is the return value of journals.reverse_each.

Probably we should add nil at the end of the method.

  # Returns the previous assignee from the issue history
  def previous_assigned_to
    journals.reverse_each do |j|
      previous_assignee_change = j.detail_for_attribute 'assigned_to_id'
      return User.find_by(id: previous_assignee_change.old_value.to_i) if previous_assignee_change && previous_assignee_change.old_value
    end
    nil
  end

• <<me>>
• in "Author / Prior assignee" group because you're the author of the new issue
• in the "User" group

• if we really want to add a new group, maybe it is better to name it "Involved" which is a term already used in the notification setting and it means that the user is author, assignee or prior assignee. Also, this allows us on future development to add new users that are already involved in the issue somehow.
• if we don't want to a new group, maybe it is good enough to add the author and the prior assignee right after <<me>> (which is first entry) and add the label next to the name, something like Redmine Admin (Author), John Doe (Previous assignee).

What do you think?

This is exciting to see being developed. I have a simple question (I think) but appreciate it may totally not fit with this work.

The idea is that on any issue, alongside the EDIT | WATCH | COPY options (on the top+bottom right of the screen), we add another which is REPLY which then triggers much of what you are working towards, namely to ensure the assignee is changed back to the person who just submitted the ticket or the comment.

Does this idea have any validity to others? It seems to simply the steps - namely instead of clicking EDIT and having to do anything to set the assignee to the prior assignee, you just click on REPLY.

I dare say there could be a bunch of workflows I am not considering - but for over 90% of your usage, we are commenting and resetting assignee to the person who previously was the assignee.

Just a thought. Appreciate your efforts.