Patch #27663

Same relates relation can be created twice

Added by Gregor Schmidt 16 days ago. Updated 8 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Issues
Target version:3.3.6

Description

By switching from and to, it is possible to create the same relates relation twice. Within the Redmine UI, those relations look like duplicates.

For all other relationship types it's not allowed to create duplicates, so I assume that this is a bug. Attached you may find a proposed fix. It adds a validation to make sure, that reverse relations are detected. It also orders the from and to IDs to ensure, that even during concurrent access, the duplicates are detected on DB level.

0001-Disallow-creating-inverse-relates-issue-relations.patch Magnifier (3 KB) Gregor Schmidt, 2017-11-28 10:22

duplicated-related-issue@2x.png (13 KB) Go MAEDA, 2017-11-28 10:59

Associated revisions

Revision 17054
Added by Go MAEDA 15 days ago

Disallow creating inverse relates issue relations (#27663).

Patch by Gregor Schmidt.

Revision 17055
Added by Go MAEDA 15 days ago

Reverted r17054 (#27663).

The change breaks test/functional/issue_relations_controller_test.rb.

Revision 17056
Added by Go MAEDA 8 days ago

Disallow creating inverse relates issue relations (#27663).

Patch by Gregor Schmidt.

Revision 17057
Added by Go MAEDA 8 days ago

Merged r17056 to 3.4-stable (#27663).

Revision 17058
Added by Go MAEDA 8 days ago

Merged r17056 to 3.3-stable (#27663).

History

#1 Updated by Go MAEDA 16 days ago

I confirmed the problem.

sqlite> select * from issue_relations where issue_from_id in (42, 43);
id          issue_from_id  issue_to_id  relation_type  delay
----------  -------------  -----------  -------------  ----------
5           42             43           relates
4           43             42           relates

#2 Updated by Go MAEDA 16 days ago

  • Target version changed from Candidate for next minor release to 3.3.6

LGTM. Setting target version to 3.3.6.

#3 Updated by Go MAEDA 15 days ago

  • Status changed from New to Resolved
  • Assignee set to Go MAEDA

#4 Updated by Go MAEDA 15 days ago

  • Status changed from Resolved to New
  • Assignee deleted (Go MAEDA)

The patch breaks a test.

$ bin/rails test test/functional/issue_relations_controller_test.rb
Run options: --seed 33551

# Running:

...F

Failure:
IssueRelationsControllerTest#test_create_xhr [/Users/maeda/redmines/redmine-trunk/test/functional/issue_relations_controller_test.rb:85]:
Expected: 3
  Actual: 1

bin/rails test test/functional/issue_relations_controller_test.rb:70

........

Finished in 4.495535s, 2.6693 runs/s, 6.6733 assertions/s.
12 runs, 30 assertions, 1 failures, 0 errors, 0 skips

#5 Updated by Go MAEDA 15 days ago

Fix for the test failure described in #27663#note-4.
issue_from_id is always smaller than issue_to_id after applying the patch.

Index: test/functional/issue_relations_controller_test.rb
===================================================================
--- test/functional/issue_relations_controller_test.rb    (revision 17054)
+++ test/functional/issue_relations_controller_test.rb    (working copy)
@@ -82,8 +82,8 @@
       assert_equal 'text/javascript', response.content_type
     end
     relation = IssueRelation.order('id DESC').first
-    assert_equal 3, relation.issue_from_id
-    assert_equal 1, relation.issue_to_id
+    assert_equal 1, relation.issue_from_id
+    assert_equal 3, relation.issue_to_id

     assert_include 'Bug #1', response.body
   end

#6 Updated by Go MAEDA 8 days ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA

Committed in the trunk and merged to stable branches. Thank you.

#7 Updated by Gregor Schmidt 8 days ago

Thanks a lot.

I am very sorry for breaking the test suite. I hope this did not cause too much trouble. I'll make sure to run the whole test suite next time.

Also available in: Atom PDF