gtt-project / redmine_custom_fields_groups

Redmine plugin for grouping custom fields.
GNU General Public License v3.0
12 stars 4 forks source link

Reordering custom_field_groups item causes 404 routing error #23

Closed sanak closed 3 months ago

sanak commented 4 months ago

Problem Reordering custom_fields_groups item by drag&drop causes 404 routing error , and doesn't although reordering itself is done successfully after browser reload.

Started PUT "/custom_fields_groups/1" for 127.0.0.1 at 2024-04-19 10:16:07 +0900
Processing by CustomFieldsGroupsController#update as JS
  Parameters: {"custom_fields_group"=>{"position"=>"2"}, "id"=>"1"}
   (0.7ms)  SELECT MAX("tokens"."updated_on") FROM "tokens" WHERE "tokens"."user_id" = $1 AND "tokens"."value" = $2 AND "tokens"."action" = $3  [["user_id", 1], ["value", "d7bc71d40ffa1a84d2a7be55db18f76ecb5d30d9"], ["action", "session"]]
  ↳ app/models/user.rb:478:in `verify_session_token'
   (1.8ms)  SELECT MAX("settings"."updated_on") FROM "settings"
  ↳ app/models/setting.rb:280:in `check_cache'
  User Load (0.8ms)  SELECT "users".* FROM "users" WHERE "users"."type" IN ($1, $2) AND "users"."status" = $3 AND "users"."id" = $4 LIMIT $5  [["type", "User"], ["type", "AnonymousUser"], ["status", 1], ["id", 1], ["LIMIT", 1]]
  ↳ app/controllers/application_controller.rb:117:in `find_current_user'
  Current user: admin (id=1)
  CustomFieldsGroup Load (0.5ms)  SELECT "custom_fields_groups".* FROM "custom_fields_groups" WHERE "custom_fields_groups"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
  ↳ plugins/redmine_custom_fields_groups/app/controllers/custom_fields_groups_controller.rb:35:in `update'
  TRANSACTION (0.4ms)  BEGIN
  ↳ plugins/redmine_custom_fields_groups/app/controllers/custom_fields_groups_controller.rb:37:in `block in update'
  CustomFieldsGroup Exists? (0.9ms)  SELECT 1 AS one FROM "custom_fields_groups" WHERE "custom_fields_groups"."name" = $1 AND "custom_fields_groups"."id" != $2 LIMIT $3  [["name", "Test1"], ["id", 1], ["LIMIT", 1]]
  ↳ plugins/redmine_custom_fields_groups/app/controllers/custom_fields_groups_controller.rb:37:in `block in update'
  CustomFieldsGroup Update (3.2ms)  UPDATE "custom_fields_groups" SET "position" = $1, "updated_at" = $2 WHERE "custom_fields_groups"."id" = $3  [["position", 2], ["updated_at", "2024-04-19 10:16:07.248882"], ["id", 1]]
  ↳ plugins/redmine_custom_fields_groups/app/controllers/custom_fields_groups_controller.rb:37:in `block in update'
  CustomFieldsGroup Update All (0.7ms)  UPDATE "custom_fields_groups" SET position = position + -1 WHERE (id <> 1 AND position BETWEEN 1 AND 2)
  ↳ lib/redmine/acts/positioned.rb:109:in `shift_positions'
  TRANSACTION (0.6ms)  COMMIT
  ↳ plugins/redmine_custom_fields_groups/app/controllers/custom_fields_groups_controller.rb:36:in `update'
Redirected to http://localhost:3000/custom_fields_groups
Completed 302 Found in 50ms (ActiveRecord: 9.6ms | Allocations: 6635)

Started PUT "/custom_fields_groups" for 127.0.0.1 at 2024-04-19 10:16:07 +0900

ActionController::RoutingError (No route matches [PUT] "/custom_fields_groups"):

To Reproduce

  1. Create 2 custom fields groups from /custom_fields_groups view.
  2. Change 2nd item position to 1st by drag&drop green up/down arrow.

Expectation Routing error should not be happen and order is changed.

sanak commented 4 months ago

The routing error cause seems to be redirecting to /custom_fields_groups with PUT method, and Redmine core, GTT and TextBlocks plugins seem to avoid it by separating format response between html and js for both manual and position (*.erb.js) update in those controllers. So, I will apply similar changes for this, later.

* Redmine 5.1: https://github.com/redmine/redmine/blob/5.1-stable/app/controllers/custom_fields_controller.rb#L66-L80 ```rb if @custom_field.save call_hook(:controller_custom_fields_edit_after_save, :params => params, :custom_field => @custom_field) respond_to do |format| format.html do flash[:notice] = l(:notice_successful_update) redirect_back_or_default edit_custom_field_path(@custom_field) end format.js {head 200} end else respond_to do |format| format.html {render :action => 'edit'} format.js {head 422} end end ``` * GTT plugin: https://github.com/gtt-project/redmine_gtt/blob/main/app/controllers/gtt_map_layers_controller.rb#L33-L49 ```rb if r.map_layer_updated? respond_to do |format| format.html { flash[:notice] = l(:notice_successful_update) redirect_to gtt_map_layers_path } format.js { head 200 } end else respond_to do |format| format.html { @map_layer = r.map_layer render 'edit' } format.js { head 422 } end end ``` * TextBlocks plugin: https://github.com/gtt-project/redmine_text_blocks/blob/main/app/controllers/text_blocks_controller.rb#L42-L57 ```rb if r.text_block_saved? respond_to do |format| format.html { flash[:notice] = l(:notice_successful_update) redirect_to index_path } format.js { head 200 } end else respond_to do |format| format.html { render 'edit' } format.js { head 422 } end end ```
sanak commented 3 months ago

Close this because PR:#26 was merged.