diff --git a/Gemfile b/Gemfile index 87bd6f758..a6361763e 100644 --- a/Gemfile +++ b/Gemfile @@ -54,6 +54,7 @@ gem 'js_cookie_rails' # Simple JS API for cookies gem 'spinjs-rails' gem 'activerecord-import' +gem 'acts_as_list' gem 'ajax-datatables-rails', '~> 0.3.1' gem 'aspector' # Aspect-oriented programming for Rails gem 'auto_strip_attributes', '~> 2.1' # Removes unnecessary whitespaces AR diff --git a/Gemfile.lock b/Gemfile.lock index 349c2cd4e..7a104da42 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -132,6 +132,8 @@ GEM i18n (>= 1.6, < 2) minitest (>= 5.1) tzinfo (~> 2.0) + acts_as_list (1.1.0) + activerecord (>= 4.2) addressable (2.8.4) public_suffix (>= 2.0.2, < 6.0) aes_key_wrap (1.1.0) @@ -707,6 +709,7 @@ PLATFORMS DEPENDENCIES active_model_serializers (~> 0.10.7) activerecord-import + acts_as_list ajax-datatables-rails (~> 0.3.1) aspector auto_strip_attributes (~> 2.1) diff --git a/app/controllers/step_elements/checklist_items_controller.rb b/app/controllers/step_elements/checklist_items_controller.rb index fd979a9f6..1320efbbd 100644 --- a/app/controllers/step_elements/checklist_items_controller.rb +++ b/app/controllers/step_elements/checklist_items_controller.rb @@ -27,15 +27,13 @@ module StepElements render json: checklist_item, serializer: ChecklistItemSerializer, user: current_user rescue ActiveRecord::RecordInvalid - render json: checklist_item, serializer: ChecklistItemSerializer, - user: current_user, - status: :unprocessable_entity + render json: { errors: checklist_item.errors }, status: :unprocessable_entity end def update old_text = @checklist_item.text @checklist_item.assign_attributes( - checklist_item_params.merge(last_modified_by: current_user) + checklist_item_params.except(:position, :id).merge(last_modified_by: current_user) ) if @checklist_item.save! @@ -49,9 +47,7 @@ module StepElements render json: @checklist_item, serializer: ChecklistItemSerializer, user: current_user rescue ActiveRecord::RecordInvalid - render json: @checklist_item, serializer: ChecklistItemSerializer, - user: current_user, - status: :unprocessable_entity + render json: { errors: @checklist_item.errors }, status: :unprocessable_entity end def toggle @@ -97,14 +93,14 @@ module StepElements end def reorder - @checklist.with_lock do - params[:checklist_item_positions].each do |id, position| - @checklist.checklist_items.find(id).update_column(:position, position) - end + checklist_item = @checklist.checklist_items.find(checklist_item_params[:id]) + ActiveRecord::Base.transaction do + checklist_item.insert_at(checklist_item_params[:position]) @checklist.touch end - render json: params[:checklist_item_positions], status: :ok + rescue ActiveRecord::RecordInvalid + render json: { errors: checklist_item.errors }, status: :conflict end private @@ -122,7 +118,7 @@ module StepElements end def checklist_item_params - params.require(:attributes).permit(:text, :position) + params.require(:attributes).permit(:text, :position, :id) end def checklist_toggle_item_params diff --git a/app/javascript/vue/shared/content/checklist.vue b/app/javascript/vue/shared/content/checklist.vue index c3fa732e0..d54b18a3e 100644 --- a/app/javascript/vue/shared/content/checklist.vue +++ b/app/javascript/vue/shared/content/checklist.vue @@ -172,8 +172,8 @@ ); if(callback) callback(); - }).error(() => { - HelperModule.flashAlertMsg(this.i18n.t('errors.general'), 'danger'); + }).error((xhr) => { + this.setFlashErrors(xhr.responseJSON.errors) }); this.update(); @@ -190,7 +190,7 @@ updatedItem.attributes.id = item.attributes.id this.$set(this.checklistItems, item.attributes.position, updatedItem) }, - error: () => HelperModule.flashAlertMsg(this.i18n.t('errors.general'), 'danger') + error: (xhr) => setFlashErrors(xhr.responseJSON.errors) }); } else { // create item, then append next one @@ -231,26 +231,26 @@ startReorder() { this.reordering = true; }, - endReorder() { + endReorder(event) { this.reordering = false; - this.saveItemOrder(); + if( + Number.isInteger(event.newIndex) + && Number.isInteger(event.newIndex) + && event.newIndex !== event.oldIndex + ){ + const { id, position } = this.orderedChecklistItems[event.newIndex]?.attributes + this.saveItemOrder(id, position); + } }, - saveItemOrder() { - let checklistItemPositions = - { - checklist_item_positions: this.orderedChecklistItems.map( - (i) => [i.attributes.id, i.attributes.position] - ) - }; - + saveItemOrder(id, position) { $.ajax({ type: "POST", url: this.element.attributes.orderable.urls.reorder_url, - data: JSON.stringify(checklistItemPositions), + data: JSON.stringify({ attributes: { id, position } }), contentType: "application/json", dataType: "json", - error: (() => HelperModule.flashAlertMsg(this.i18n.t('errors.general'), 'danger')), - success: (() => this.update()) + error: (xhr) => this.setFlashErrors(xhr.responseJSON.errors), + success: () => this.update() }); }, handleMultilinePaste(data) { @@ -274,6 +274,14 @@ }; synchronousPost(0); + }, + setFlashErrors(errors) { + for(const key in errors){ + HelperModule.flashAlertMsg( + this.i18n.t(`activerecord.errors.models.checklist_item.attributes.${key}`), + 'danger' + ) + } } } } diff --git a/app/models/checklist_item.rb b/app/models/checklist_item.rb index 1f4de41a3..fd1a3c77d 100644 --- a/app/models/checklist_item.rb +++ b/app/models/checklist_item.rb @@ -5,10 +5,11 @@ class ChecklistItem < ApplicationRecord length: { maximum: Constants::TEXT_MAX_LENGTH } validates :checklist, presence: true validates :checked, inclusion: { in: [true, false] } - validates :position, uniqueness: { scope: :checklist }, unless: -> { position.nil? } + validates :position, uniqueness: { scope: :checklist } belongs_to :checklist, inverse_of: :checklist_items + acts_as_list scope: :checklist, top_of_list: 0, sequential_updates: true belongs_to :created_by, foreign_key: 'created_by_id', class_name: 'User', @@ -18,8 +19,6 @@ class ChecklistItem < ApplicationRecord class_name: 'User', optional: true - after_destroy :update_positions - # conditional touch excluding checked updates after_destroy :touch_checklist after_save :touch_checklist @@ -35,12 +34,4 @@ class ChecklistItem < ApplicationRecord checklist.touch # rubocop:enable Rails/SkipsModelValidations end - - def update_positions - transaction do - checklist.checklist_items.order(position: :asc).each_with_index do |checklist_item, i| - checklist_item.update!(position: i) - end - end - end end diff --git a/config/locales/en.yml b/config/locales/en.yml index a8480b761..8f0915210 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -232,6 +232,10 @@ en: attributes: output_id: creates_cycle: "mustn't create cycle" + checklist_item: + attributes: + text: Text is too long + position: "Position has already been taken by another item in the checklist" errors: general: "Something went wrong." general_text_too_long: 'Text is too long' diff --git a/db/migrate/20230728104825_add_index_to_checklist_items.rb b/db/migrate/20230728104825_add_index_to_checklist_items.rb new file mode 100644 index 000000000..510d9ad8c --- /dev/null +++ b/db/migrate/20230728104825_add_index_to_checklist_items.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class + AddIndexToChecklistItems < ActiveRecord::Migration[7.0] + def change + add_index :checklist_items, %i(checklist_id position), unique: true + end +end