Feature #33418 » 0001-Rework-patch-from-33418.patch
| app/controllers/issue_relations_controller.rb | ||
|---|---|---|
| 44 | 44 |
end |
| 45 | 45 | |
| 46 | 46 |
def create |
| 47 |
@relation = IssueRelation.new |
|
| 48 |
@relation.issue_from = @issue |
|
| 49 |
@relation.safe_attributes = params[:relation] |
|
| 50 |
@relation.init_journals(User.current) |
|
| 47 |
saved = false |
|
| 48 |
params_relation = params[:relation] |
|
| 49 |
unsaved_relations = [] |
|
| 50 | ||
| 51 |
relation_issues_to_id.each do |issue_to_id| |
|
| 52 |
params_relation[:issue_to_id] = issue_to_id |
|
| 53 | ||
| 54 |
@relation = IssueRelation.new |
|
| 55 |
@relation.issue_from = @issue |
|
| 56 |
@relation.safe_attributes = params_relation |
|
| 57 |
@relation.init_journals(User.current) |
|
| 51 | 58 | |
| 52 |
begin |
|
| 53 |
saved = @relation.save |
|
| 54 |
rescue ActiveRecord::RecordNotUnique |
|
| 55 |
saved = false |
|
| 56 |
@relation.errors.add :base, :taken |
|
| 59 |
unless saved = @relation.save |
|
| 60 |
saved = false |
|
| 61 |
unsaved_relations << @relation |
|
| 62 |
end |
|
| 57 | 63 |
end |
| 58 | 64 | |
| 59 | 65 |
respond_to do |format| |
| 60 | 66 |
format.html {redirect_to issue_path(@issue)}
|
| 61 | 67 |
format.js do |
| 62 | 68 |
@relations = @issue.reload.relations.select {|r| r.other_issue(@issue) && r.other_issue(@issue).visible?}
|
| 69 |
@unsaved_relations = unsaved_relations |
|
| 63 | 70 |
end |
| 64 | 71 |
format.api do |
| 65 | 72 |
if saved |
| ... | ... | |
| 98 | 105 |
rescue ActiveRecord::RecordNotFound |
| 99 | 106 |
render_404 |
| 100 | 107 |
end |
| 108 | ||
| 109 |
def relation_issues_to_id |
|
| 110 |
params[:relation].require(:issue_to_id).split(',').reject {|v| v.blank?}
|
|
| 111 |
rescue ActionController::ParameterMissing => e |
|
| 112 |
# We return a empty array just to loop once and return a validation error |
|
| 113 |
# ToDo: Find a better method to return an error if the param is missing. |
|
| 114 |
[''] |
|
| 115 |
end |
|
| 101 | 116 |
end |
| app/helpers/issue_relations_helper.rb | ||
|---|---|---|
| 22 | 22 |
values = IssueRelation::TYPES |
| 23 | 23 |
values.keys.sort_by{|k| values[k][:order]}.collect{|k| [l(values[k][:name]), k]}
|
| 24 | 24 |
end |
| 25 | ||
| 26 |
def relation_error_messages(relations) |
|
| 27 |
messages = {}
|
|
| 28 |
relations.each do |item| |
|
| 29 |
item.errors.full_messages.each do |message| |
|
| 30 |
messages[message] ||= [] |
|
| 31 |
messages[message] << item |
|
| 32 |
end |
|
| 33 |
end |
|
| 34 | ||
| 35 |
messages.map do |message, items| |
|
| 36 |
ids = items.map(&:issue_to_id).compact |
|
| 37 |
if ids.empty? |
|
| 38 |
message |
|
| 39 |
else |
|
| 40 |
"#{message}: ##{ids.join(', ')}"
|
|
| 41 |
end |
|
| 42 |
end |
|
| 43 |
end |
|
| 25 | 44 |
end |
| app/views/issue_relations/_form.html.erb | ||
|---|---|---|
| 1 |
<%= error_messages_for 'relation' %> |
|
| 2 | ||
| 1 |
<% unsaved_relations_ids = '' %> |
|
| 2 |
<% if @unsaved_relations && @unsaved_relations.any? %> |
|
| 3 |
<% unsaved_relations_ids = @unsaved_relations.map(&:issue_to_id).compact.join(", ") %>
|
|
| 4 |
<div id="errorExplanation"> |
|
| 5 |
<ul> |
|
| 6 |
<% relation_error_messages(@unsaved_relations).each do |message| %> |
|
| 7 |
<li><%= message %></li> |
|
| 8 |
<% end %> |
|
| 9 |
</ul> |
|
| 10 |
</div> |
|
| 11 |
<% end %> |
|
| 3 | 12 |
<p><%= f.select :relation_type, collection_for_relation_type_select, {}, :onchange => "setPredecessorFieldsVisibility();" %>
|
| 4 |
<%= l(:label_issue) %> #<%= f.text_field :issue_to_id, :size => 10 %> |
|
| 13 |
<%= l(:label_issue) %> #<%= f.text_field :issue_to_id, :value => unsaved_relations_ids, :size => 10 %>
|
|
| 5 | 14 |
<span id="predecessor_fields" style="display:none;"> |
| 6 | 15 |
<%= l(:field_delay) %>: <%= f.text_field :delay, :size => 3 %> <%= l(:label_day_plural) %> |
| 7 | 16 |
</span> |
| test/functional/issue_relations_controller_test.rb | ||
|---|---|---|
| 216 | 216 |
assert_include 'Related issue cannot be blank', response.body |
| 217 | 217 |
end |
| 218 | 218 | |
| 219 |
def test_bulk_create_with_multiple_issue_to_id_issues |
|
| 220 |
assert_difference 'IssueRelation.count', +3 do |
|
| 221 |
post :create, :params => {
|
|
| 222 |
:issue_id => 1, |
|
| 223 |
:relation => {
|
|
| 224 |
# js autocomplete adds a comma at the end |
|
| 225 |
# issue to id should accept both id and hash with id |
|
| 226 |
:issue_to_id => '2,3,#7, ', |
|
| 227 |
:relation_type => 'relates', |
|
| 228 |
:delay => '' |
|
| 229 |
} |
|
| 230 |
}, |
|
| 231 |
:xhr => true |
|
| 232 |
end |
|
| 233 | ||
| 234 |
assert_response :success |
|
| 235 |
relations = IssueRelation.where(:issue_from_id => 1, :issue_to_id => [2, 3, 7]) |
|
| 236 |
assert_equal 3, relations.count |
|
| 237 |
# all relations types should be 'relates' |
|
| 238 |
relations.map {|r| assert_equal 'relates', r.relation_type}
|
|
| 239 | ||
| 240 |
# no error messages should be returned in the response |
|
| 241 |
assert_not_include 'id=\"errorExplanation\"', response.body |
|
| 242 |
end |
|
| 243 | ||
| 244 |
def test_bulk_create_should_show_errors |
|
| 245 |
assert_difference 'IssueRelation.count', +3 do |
|
| 246 |
post :create, :params => {
|
|
| 247 |
:issue_id => 1, |
|
| 248 |
:relation => {
|
|
| 249 |
:issue_to_id => '1,2,3,4,5,7', |
|
| 250 |
:relation_type => 'relates', |
|
| 251 |
:delay => '' |
|
| 252 |
} |
|
| 253 |
}, |
|
| 254 |
:xhr => true |
|
| 255 |
end |
|
| 256 | ||
| 257 |
assert_response :success |
|
| 258 |
assert_equal 'text/javascript', response.media_type |
|
| 259 |
# issue #1 is invalid |
|
| 260 |
assert_include 'Related issue is invalid: #1', response.body |
|
| 261 |
# issues #4 and #5 can't be related by default |
|
| 262 |
assert_include 'Related issue cannot be blank', response.body |
|
| 263 |
assert_include 'Related issue doesn't belong to the same project', response.body |
|
| 264 |
end |
|
| 265 | ||
| 219 | 266 |
def test_destroy |
| 220 | 267 |
assert_difference 'IssueRelation.count', -1 do |
| 221 | 268 |
delete(:destroy, :params => {:id => '2'})
|