Works against 2.4-stable rev 12697
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
* You missed db migrate file.
* mix tabs and spaces
* please add tests
--------------------------------------------------------------------------------
Thanks for pointing out !!! attached file, I will work on adding a test.
--------------------------------------------------------------------------------
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 <notextile>-</notextile>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

h2. Test Plan

h3. Edit Issue

# open issue edit form
# use select box
# expect: last assignee is shown in the fist options
# expect: assignment to that user is successful

h3. bulk edit

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

h3. Issue Category

# open project settings configuration
# open issue categories tab
# edit issue category
# expect: successful page load and no last assignee option in assigned_to select box

h3. New Issue

# create new issue with an assignee
# open edit page of that issue
# expect: last assignee shows the author of the issue
# 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 <pre><< <span class="translation_missing"; title="translation missing: en.issue.last_assignee">Last Assignee</span> >></pre> Anything I missed here?

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 <pre> 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))</pre> to <pre> 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))</pre> fixes this.

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 <pre> journals.reverse_each do |j|</pre> by <pre> journals.each do |j|</pre> seems to do the right thing. Can you confirm this is the proper solution?

--------------------------------------------------------------------------------
Thank you for sharing your patch, Luka Lüdicke.
I think this feature will be useful in projects with many users.

I made some changes, such as adding a test to your patch.( and I removed it because some changes for German were included. )
→ attachment:previous-assignee-v2.0.patch

Since I think that p"revious_assignee" is more appropriate than "last_assignee", I also created a version patch that replaced the "previous".
→ attachment:previous-assignee-v2.1.patch

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.
--------------------------------------------------------------------------------
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.
<pre>
<code class="ruby">
[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
</code>
</pre>

thanks
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
The patch looks good to me. Setting target version to 4.1.0.
--------------------------------------------------------------------------------
@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.

<pre><code class="ruby">
# 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
</code></pre>
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

relates,New,19501,Assign issue to <<author>>
relates,New,14602,Assignee list should be (optionally) ordered by probability
relates,New,23072,Speed-Up the setting of "assigned to" by showing good guesses at the top of the list
duplicates,Closed,24319,Last assigned to option in the Assignee selection list