Skip to content

Commit

Permalink
Remove no-more-needed broken controller after_save hook. Remove no-mo…
Browse files Browse the repository at this point in the history
…re-needed issue reload workarounds. Fix ixti#228.
  • Loading branch information
martincizek committed May 3, 2021
1 parent 2c0c66c commit 4ee1040
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 18 deletions.
19 changes: 1 addition & 18 deletions lib/redmine_tags/hooks/model_issue_hook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,12 @@ def controller_issues_bulk_edit_before_save(context = {})
bulk_update_tags_to_issues context
end

# Issue has an after_save method that calls reload (update_nested_set_attributes)
# This makes it impossible for a new record to get a tag_list, it's
# cleared on reload. So instead, hook in after the Issue#save to update
# this issue's tag_list and call #save ourselves.
def controller_issues_new_after_save(context = {})
save_tags_to_issue context, false
context[:issue].save
end

def save_tags_to_issue(context, create_journal)
def save_tags_to_issue(context, create_journal = true)
params = context[:params]
issue = context[:issue]
if params && params[:issue] && !params[:issue][:tag_list].nil?
old_tags = Issue.find(context[:issue].id).tag_list.to_s
issue.tag_list = params[:issue][:tag_list]
new_tags = issue.tag_list.to_s
# without this when reload called in Issue#save all changes will be
# gone :(
issue.save_tags
create_journal_entry(issue, old_tags, new_tags) if create_journal

Issue.remove_unused_tags!
Expand All @@ -54,10 +41,6 @@ def bulk_update_tags_to_issues(context)
old_tags = current_tags.to_s
new_tags = current_tags.add(tags_to_add).remove(tags_to_remove)

issue.tag_list = new_tags
# without this when reload called in Issue#save all changes will be
# gone :(
issue.save_tags
create_journal_entry(issue, old_tags, new_tags)

Issue.remove_unused_tags!
Expand Down
67 changes: 67 additions & 0 deletions test/functional/issues_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -257,4 +257,71 @@ def test_bulk_edit_journal_with_tag_changing
assert_equal ['tag_list'], Issue.find(2).journals.last.details.map(&:prop_key)
assert_equal ['tag_list'], Issue.find(7).journals.last.details.map(&:prop_key)
end

def test_post_create_with_tags
@request.session[:user_id] = 2
assert_difference 'Issue.count' do
assert_no_difference 'Journal.count' do
post(
:create,
:params => {
:project_id => 1,
:issue => {
:tracker_id => 3,
:status_id => 2,
:subject => 'This is the test_post_create_with_tags issue',
:description => 'This is the description',
:priority_id => 5,
:estimated_hours => '',
:custom_field_values => {
'2' => 'Value for field 2'
},
:tag_list => ['Production', 'Functional'],
}
}
)
end
end
assert_redirected_to :controller => 'issues', :action => 'show', :id => Issue.last.id

issue = Issue.find_by_subject('This is the test_post_create_with_tags issue')
assert_not_nil issue
# custom fields might be special
v = issue.custom_values.where(:custom_field_id => 2).first
assert_not_nil v
assert_equal 'Value for field 2', v.value
# tags should not be cleared with any sort of reload call in Issue after_save methods
assert_equal ['Production', 'Functional'], issue.tag_list
end

def test_create_as_copy_should_copy_tags
issue = Issue.generate! {|i| i.tag_list = ['Production', 'Security']}
child = Issue.generate!(:parent_issue_id => issue.id) {|i| i.tag_list = ['Functional']}
@request.session[:user_id] = 1
assert_difference 'Issue.count', 2 do
post(
:create,
:params => {
:project_id => 1,
:copy_from => issue.id,
:issue => {
:project_id => '1',
:tracker_id => '3',
:status_id => '1',
:subject => 'Copy with subtask and tags'
},
:copy_subtasks => '1'
}
)
end
child_copy, issue_copy = Issue.order(:id => :desc).limit(2).to_a
# make sure we have the newly copied issues
assert [issue_copy.id, child_copy.id].min > [issue.id, child.id].max
# https://github.com/ixti/redmine_tags/issues/228
assert_equal child_copy.parent_issue_id, issue_copy.id
assert_nil issue_copy.parent_issue_id

assert_equal ['Production', 'Security'], issue_copy.tag_list
assert_equal ['Functional'], child_copy.tag_list
end
end

0 comments on commit 4ee1040

Please sign in to comment.