From 83121ac75df44a4d5663b593f915581c4ef07f3c Mon Sep 17 00:00:00 2001 From: shunshunNi Date: Fri, 10 Jul 2026 20:34:38 +0900 Subject: [PATCH] Allow updating the delay of an existing precedes/follows relation. --- app/controllers/issue_relations_controller.rb | 34 ++++- app/helpers/issues_helper.rb | 32 +++-- app/models/issue_relation.rb | 6 + app/views/issue_relations/update.js.erb | 7 + app/views/issues/_relations.html.erb | 20 +++ config/locales/en.yml | 1 + config/locales/ja.yml | 1 + config/routes.rb | 2 +- .../issue_relations_controller_test.rb | 125 ++++++++++++++++++ .../api_test/issue_relations_test.rb | 33 +++++ 10 files changed, 246 insertions(+), 15 deletions(-) create mode 100644 app/views/issue_relations/update.js.erb diff --git a/app/controllers/issue_relations_controller.rb b/app/controllers/issue_relations_controller.rb index e64d9a1f7..35bc62128 100644 --- a/app/controllers/issue_relations_controller.rb +++ b/app/controllers/issue_relations_controller.rb @@ -21,9 +21,9 @@ class IssueRelationsController < ApplicationController helper :issues before_action :find_issue, :authorize, :only => [:index, :create] - before_action :find_relation, :only => [:show, :destroy] + before_action :find_relation, :only => [:show, :update, :destroy] - accept_api_auth :index, :show, :create, :destroy + accept_api_auth :index, :show, :create, :update, :destroy def index @relations = @issue.relations @@ -81,6 +81,36 @@ class IssueRelationsController < ApplicationController end end + def update + raise Unauthorized unless @relation.editable? + + @issue = + [@relation.issue_from, @relation.issue_to].compact.detect do |issue| + issue.id.to_s == params[:issue_id].to_s + end || @relation.issue_from + @project = @issue.project + + @relation.init_journals(User.current) + # Only the delay can be updated, other attributes require the relation + # to be recreated + @relation.safe_attributes = params.expect(:relation => [:delay]) + saved = @relation.save + + respond_to do |format| + format.html {redirect_to issue_path(@issue)} + format.js do + @relations = select_relations(@issue) + end + format.api do + if saved + render_api_ok + else + render_validation_errors(@relation) + end + end + end + end + def destroy raise Unauthorized unless @relation.deletable? diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 243c82c13..c0f1247b6 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -222,20 +222,28 @@ module IssuesHelper relations.each do |relation| other_issue = relation.other_issue(issue) css = "issue hascontextmenu #{other_issue.css_classes} #{relation.css_classes_for(other_issue)}" - buttons = - if manage_relations - link_to( - sprite_icon('link-break', l(:label_relation_remove)), - relation_path(relation, issue_id: issue.id), - :remote => true, - :method => :delete, - :data => {:confirm => l(:text_are_you_sure)}, - :title => l(:label_relation_remove), - :class => 'icon-only icon-link-break' + buttons = "".html_safe + if manage_relations + if relation.relation_type == IssueRelation::TYPE_PRECEDES + buttons << link_to_function( + sprite_icon('edit', l(:label_relation_edit_delay)), + "$('.edit-relation-form').hide();" \ + "$('#edit-relation-#{relation.id}-form').show();" \ + "$('#relation_#{relation.id}_delay').focus();", + :title => l(:label_relation_edit_delay), + :class => 'icon-only icon-edit' ) - else - "".html_safe end + buttons << link_to( + sprite_icon('link-break', l(:label_relation_remove)), + relation_path(relation, issue_id: issue.id), + :remote => true, + :method => :delete, + :data => {:confirm => l(:text_are_you_sure)}, + :title => l(:label_relation_remove), + :class => 'icon-only icon-link-break' + ) + end buttons << link_to_context_menu subject_content = relation.to_s(@issue) { |other| link_to_issue other, project: Setting.cross_project_issue_relations? }.html_safe diff --git a/app/models/issue_relation.rb b/app/models/issue_relation.rb index 80af22b89..b1e889693 100644 --- a/app/models/issue_relation.rb +++ b/app/models/issue_relation.rb @@ -110,6 +110,12 @@ class IssueRelation < ApplicationRecord (issue_to.nil? || user.allowed_to?(:manage_issue_relations, issue_to.project))) end + def editable?(user=User.current) + visible?(user) && + ((issue_from.nil? || user.allowed_to?(:manage_issue_relations, issue_from.project)) || + (issue_to.nil? || user.allowed_to?(:manage_issue_relations, issue_to.project))) + end + def initialize(attributes=nil, *args) super if new_record? diff --git a/app/views/issue_relations/update.js.erb b/app/views/issue_relations/update.js.erb new file mode 100644 index 000000000..b4dcd5bcc --- /dev/null +++ b/app/views/issue_relations/update.js.erb @@ -0,0 +1,7 @@ +<% if @relation.errors.any? %> +alert('<%= escape_javascript(@relation.errors.full_messages.join("\n")) %>'); +<% else %> +$('#relations').html('<%= escape_javascript(render :partial => 'issues/relations') %>'); +$('#relation_delay').val(''); +$('#relation_issue_to_id').val(''); +<% end %> diff --git a/app/views/issues/_relations.html.erb b/app/views/issues/_relations.html.erb index 2ea689a2f..46831f2ce 100644 --- a/app/views/issues/_relations.html.erb +++ b/app/views/issues/_relations.html.erb @@ -15,6 +15,26 @@ <%= render_issue_relations(@issue, @relations) %> <% end %> +<% if User.current.allowed_to?(:manage_issue_relations, @project) %> + <% @relations.each do |relation| %> + <% next unless relation.relation_type == IssueRelation::TYPE_PRECEDES && relation.other_issue(@issue) %> + <%= form_for relation, { + :as => :relation, :remote => true, + :url => relation_path(relation, :issue_id => @issue.id), + :method => :put, + :html => {:id => "edit-relation-#{relation.id}-form", + :class => 'edit-relation-form', + :style => 'display: none;'} + } do |f| %> +

+ <%= relation.to_s(@issue) {|other| "##{other.id}"} %>: + <%= l(:field_delay) %> <%= f.text_field :delay, :size => 3, :id => "relation_#{relation.id}_delay" %> <%= l(:label_day_plural) %> + <%= submit_tag l(:button_save) %> + <%= link_to_function l(:button_cancel), "$('#edit-relation-#{relation.id}-form').hide();" %> +

+ <% end %> + <% end %> +<% end %> <% end %> <%= form_for @relation, { diff --git a/config/locales/en.yml b/config/locales/en.yml index d0d93746a..eab68ecd0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -917,6 +917,7 @@ en: label_loading: Loading... label_relation_new: New relation label_relation_remove: Remove relation + label_relation_edit_delay: Edit delay label_relates_to: Related to label_subtask_remove: Remove subtask label_duplicates: Is duplicate of diff --git a/config/locales/ja.yml b/config/locales/ja.yml index e7f8bbe9d..dc215d2b1 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -1489,6 +1489,7 @@ ja: webhook_event_deleted: "%{object_name} を削除しました" label_auto_watch_on_issue_assigned_to_me: 自分が担当者になったチケット label_relation_remove: 関連を解除 + label_relation_edit_delay: 遅延を変更 label_subtask_remove: 子チケットを解除 button_remove: 解除 label_open_in_full_view: 全画面で表示 diff --git a/config/routes.rb b/config/routes.rb index ce15da420..c4e356b82 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -236,7 +236,7 @@ Rails.application.routes.draw do end resources :time_entries, :controller => 'timelog', :only => [:new, :create] shallow do - resources :relations, :controller => 'issue_relations', :only => [:index, :show, :create, :destroy] + resources :relations, :controller => 'issue_relations', :only => [:index, :show, :create, :update, :destroy] end end # Used when updating the form of a new issue outside a project diff --git a/test/functional/issue_relations_controller_test.rb b/test/functional/issue_relations_controller_test.rb index 35a02a1df..18f3b00fa 100644 --- a/test/functional/issue_relations_controller_test.rb +++ b/test/functional/issue_relations_controller_test.rb @@ -283,6 +283,131 @@ class IssueRelationsControllerTest < Redmine::ControllerTest assert_include 'Related issue doesn't belong to the same project', response.body end + def test_update + relation = IssueRelation.create!( + :issue_from => Issue.find(1), :issue_to => Issue.find(2), + :relation_type => IssueRelation::TYPE_PRECEDES, :delay => 2 + ) + + assert_no_difference 'IssueRelation.count' do + put( + :update, + :params => { + :id => relation.id, + :relation => {:delay => '5'} + } + ) + end + assert_redirected_to '/issues/1' + assert_equal 5, relation.reload.delay + end + + def test_update_xhr + relation = IssueRelation.create!( + :issue_from => Issue.find(1), :issue_to => Issue.find(2), + :relation_type => IssueRelation::TYPE_PRECEDES, :delay => 2 + ) + + put( + :update, + :params => { + :id => relation.id, + :issue_id => 1, + :relation => {:delay => '5'} + }, + :xhr => true + ) + assert_response :success + assert_equal 'text/javascript', response.media_type + assert_include '#relations', response.body + assert_include "id=\\\"edit-relation-#{relation.id}-form\\\"", response.body + assert_equal 5, relation.reload.delay + end + + def test_update_xhr_with_failure + relation = IssueRelation.create!( + :issue_from => Issue.find(1), :issue_to => Issue.find(2), + :relation_type => IssueRelation::TYPE_PRECEDES, :delay => 2 + ) + + put( + :update, + :params => { + :id => relation.id, + :issue_id => 1, + :relation => {:delay => 'foo'} + }, + :xhr => true + ) + assert_response :success + assert_equal 'text/javascript', response.media_type + assert_include 'Delay is not a number', response.body + assert_equal 2, relation.reload.delay + end + + def test_update_should_not_change_attributes_other_than_delay + relation = IssueRelation.create!( + :issue_from => Issue.find(1), :issue_to => Issue.find(2), + :relation_type => IssueRelation::TYPE_PRECEDES, :delay => 2 + ) + + put( + :update, + :params => { + :id => relation.id, + :relation => { + :delay => '5', + :relation_type => 'blocks', + :issue_to_id => '3' + } + } + ) + relation.reload + assert_equal 5, relation.delay + assert_equal IssueRelation::TYPE_PRECEDES, relation.relation_type + assert_equal 2, relation.issue_to_id + end + + def test_update_should_reschedule_following_issue + issue1 = Issue.generate!(:start_date => Date.parse('2026-07-01'), + :due_date => Date.parse('2026-07-02')) + issue2 = Issue.generate! + relation = IssueRelation.create!( + :issue_from => issue1, :issue_to => issue2, + :relation_type => IssueRelation::TYPE_PRECEDES, :delay => 0 + ) + + with_settings :non_working_week_days => [] do + put( + :update, + :params => { + :id => relation.id, + :relation => {:delay => '2'} + } + ) + end + assert_equal 2, relation.reload.delay + assert_equal Date.parse('2026-07-05'), issue2.reload.start_date + end + + def test_update_without_permission_should_fail + relation = IssueRelation.create!( + :issue_from => Issue.find(1), :issue_to => Issue.find(2), + :relation_type => IssueRelation::TYPE_PRECEDES, :delay => 2 + ) + Role.all.each {|r| r.remove_permission! :manage_issue_relations} + + put( + :update, + :params => { + :id => relation.id, + :relation => {:delay => '5'} + } + ) + assert_response :forbidden + assert_equal 2, relation.reload.delay + end + def test_destroy assert_difference 'IssueRelation.count', -1 do delete(:destroy, :params => {:id => '2'}) diff --git a/test/integration/api_test/issue_relations_test.rb b/test/integration/api_test/issue_relations_test.rb index 9797cc34b..2dcc20bc3 100644 --- a/test/integration/api_test/issue_relations_test.rb +++ b/test/integration/api_test/issue_relations_test.rb @@ -90,6 +90,39 @@ class Redmine::ApiTest::IssueRelationsTest < Redmine::ApiTest::Base assert_select 'relation id', :text => '2' end + test "PUT /relations/:id.xml should update the delay" do + relation = IssueRelation.create!( + :issue_from => Issue.find(1), :issue_to => Issue.find(2), + :relation_type => IssueRelation::TYPE_PRECEDES, :delay => 2 + ) + + put( + "/relations/#{relation.id}.xml", + :params => {:relation => {:delay => 5}}, + :headers => credentials('jsmith') + ) + + assert_response :no_content + assert_equal 5, relation.reload.delay + end + + test "PUT /relations/:id.xml with invalid delay should return errors" do + relation = IssueRelation.create!( + :issue_from => Issue.find(1), :issue_to => Issue.find(2), + :relation_type => IssueRelation::TYPE_PRECEDES, :delay => 2 + ) + + put( + "/relations/#{relation.id}.xml", + :params => {:relation => {:delay => 'foo'}}, + :headers => credentials('jsmith') + ) + + assert_response :unprocessable_content + assert_select 'errors error', :text => /Delay is not a number/ + assert_equal 2, relation.reload.delay + end + test "DELETE /relations/:id.xml should delete the relation" do assert_difference('IssueRelation.count', -1) do delete '/relations/2.xml', :headers => credentials('jsmith') -- 2.55.0