diff --git a/app/controllers/api/v1/accounts/articles_controller.rb b/app/controllers/api/v1/accounts/articles_controller.rb index 5e1609b64..4a8363fdd 100644 --- a/app/controllers/api/v1/accounts/articles_controller.rb +++ b/app/controllers/api/v1/accounts/articles_controller.rb @@ -40,8 +40,8 @@ class Api::V1::Accounts::ArticlesController < Api::V1::Accounts::BaseController end def reorder - Article.update_positions(portal: @portal, positions_hash: params[:positions_hash]) - head :ok + positions = Article.update_positions(portal: @portal, positions_hash: params[:positions_hash]) + render json: { positions: positions } end private diff --git a/app/javascript/dashboard/components-next/DraggableReorderList/DraggableReorderList.vue b/app/javascript/dashboard/components-next/DraggableReorderList/DraggableReorderList.vue new file mode 100644 index 000000000..95c05f6b6 --- /dev/null +++ b/app/javascript/dashboard/components-next/DraggableReorderList/DraggableReorderList.vue @@ -0,0 +1,378 @@ + + + diff --git a/app/javascript/dashboard/components-next/DraggableReorderList/specs/DraggableReorderList.spec.js b/app/javascript/dashboard/components-next/DraggableReorderList/specs/DraggableReorderList.spec.js new file mode 100644 index 000000000..a6800beb5 --- /dev/null +++ b/app/javascript/dashboard/components-next/DraggableReorderList/specs/DraggableReorderList.spec.js @@ -0,0 +1,222 @@ +import { mount } from '@vue/test-utils'; +import { h, nextTick } from 'vue'; +import DraggableReorderList from '../DraggableReorderList.vue'; + +// The component is pointer-driven, so we drive it through real pointer events on +// window while mocking the layout APIs jsdom does not implement: elementFromPoint +// (which card is under the cursor) and getBoundingClientRect (its geometry). +const elementAtPoint = { current: null }; + +const move = (clientX, clientY) => + window.dispatchEvent(new MouseEvent('pointermove', { clientX, clientY })); +const release = () => window.dispatchEvent(new MouseEvent('pointerup')); + +// Stack the rows 50px apart, each 40px tall, inside a 500px-wide list. +const stubGeometry = wrapper => { + wrapper.element.getBoundingClientRect = () => ({ + left: 0, + right: 500, + top: 0, + bottom: 600, + }); + wrapper.findAll('[data-drag-id]').forEach((li, index) => { + const top = index * 50; + li.element.getBoundingClientRect = () => ({ + top, + height: 40, + bottom: top + 40, + }); + }); +}; + +const mountList = (props = {}) => + mount(DraggableReorderList, { + props: { items: [], ...props }, + slots: { + item: scope => h('div', { class: 'card' }, scope.item.title), + ghost: scope => h('div', { class: 'ghost' }, scope.item.title), + }, + global: { stubs: { Icon: true, teleport: true } }, + }); + +describe('DraggableReorderList', () => { + let wrapper; + + beforeEach(() => { + elementAtPoint.current = null; + document.elementFromPoint = vi.fn(() => elementAtPoint.current); + }); + + afterEach(() => { + wrapper?.unmount(); + vi.useRealTimers(); + }); + + const startDragging = async id => { + stubGeometry(wrapper); + wrapper.find(`[data-drag-id="${id}"]`).element.dispatchEvent( + new MouseEvent('pointerdown', { + button: 0, + clientX: 250, + clientY: 20, + bubbles: true, + }) + ); + await nextTick(); + }; + + it('renders each item through the item slot', () => { + wrapper = mountList({ + items: [ + { id: 1, title: 'Alpha' }, + { id: 2, title: 'Beta' }, + ], + }); + + const cards = wrapper.findAll('.card'); + expect(cards).toHaveLength(2); + expect(cards[0].text()).toBe('Alpha'); + expect(wrapper.find('[data-drag-id="1"]').exists()).toBe(true); + expect(wrapper.find('[data-drag-id="2"]').exists()).toBe(true); + }); + + it('shows a grab affordance only when enabled', () => { + wrapper = mountList({ items: [{ id: 1, title: 'Alpha' }] }); + expect(wrapper.find('[data-drag-id="1"]').classes()).toContain( + 'cursor-grab' + ); + + wrapper.unmount(); + wrapper = mountList({ items: [{ id: 1, title: 'Alpha' }], disabled: true }); + expect(wrapper.find('[data-drag-id="1"]').classes()).not.toContain( + 'cursor-grab' + ); + }); + + it('does not start a drag when disabled', async () => { + wrapper = mountList({ + items: [ + { id: 1, title: 'Alpha' }, + { id: 2, title: 'Beta' }, + ], + disabled: true, + }); + await startDragging(1); + move(250, 200); + await nextTick(); + + expect(wrapper.emitted('dragging')).toBeUndefined(); + }); + + it('emits dragging true then false across a drag', async () => { + wrapper = mountList({ + items: [ + { id: 1, title: 'Alpha' }, + { id: 2, title: 'Beta' }, + ], + }); + await startDragging(1); + elementAtPoint.current = wrapper.find('[data-drag-id="2"]').element; + move(250, 60); + await nextTick(); + + expect(wrapper.emitted('dragging')[0]).toEqual([true]); + + release(); + await nextTick(); + expect(wrapper.emitted('dragging')[1]).toEqual([false]); + }); + + it('emits the midpoint position when dropped between two rows', async () => { + wrapper = mountList({ + items: [ + { id: 1, title: 'Alpha', position: 10 }, + { id: 2, title: 'Beta', position: 20 }, + { id: 3, title: 'Gamma', position: 30 }, + ], + }); + await startDragging(1); + + // Hover the lower half of Beta (top 50, height 40 → midpoint 70) so the gap + // sits before Gamma; dropping there lands halfway between Beta and Gamma. + elementAtPoint.current = wrapper.find('[data-drag-id="2"]').element; + move(250, 85); + await nextTick(); + release(); + await nextTick(); + + expect(wrapper.emitted('reorder')[0][0]).toEqual({ 1: 25 }); + }); + + it('does not reorder when the only row on a page is dropped in place', async () => { + // P1: dragging the lone article on a later page and releasing without + // crossing to another page must be a no-op, not move it to the top. + wrapper = mountList({ + items: [{ id: 5, title: 'Solo', position: 260 }], + currentPage: 2, + totalPages: 2, + }); + await startDragging(5); + move(250, 300); + await nextTick(); + release(); + await nextTick(); + + expect(wrapper.emitted('dragging')).toEqual([[true], [false]]); + expect(wrapper.emitted('reorder')).toBeUndefined(); + }); + + it('turns the page after dwelling on a pageable edge', async () => { + vi.useFakeTimers(); + wrapper = mountList({ + items: [ + { id: 1, title: 'Alpha', position: 10 }, + { id: 2, title: 'Beta', position: 20 }, + ], + currentPage: 1, + totalPages: 2, + }); + await startDragging(1); + + // Drag to the right edge over blank space (no card) and hold. + elementAtPoint.current = null; + move(490, 20); + await nextTick(); + vi.advanceTimersByTime(600); + + expect(wrapper.emitted('navigatePage')[0]).toEqual([2]); + }); + + it('can still turn pages after releasing during a pending flip', async () => { + // Releasing while a flip fetch is in flight must clear paging state, or every + // later drag would be stuck unable to navigate. + vi.useFakeTimers(); + wrapper = mountList({ + items: [ + { id: 1, title: 'Alpha', position: 10 }, + { id: 2, title: 'Beta', position: 20 }, + ], + currentPage: 1, + totalPages: 2, + }); + + // First drag: park at the edge to start a flip, then release before the new + // page arrives (items never change here). + await startDragging(1); + elementAtPoint.current = null; + move(490, 20); + await nextTick(); + vi.advanceTimersByTime(600); + release(); + await nextTick(); + + // Second drag must be able to flip again. + await startDragging(1); + elementAtPoint.current = null; + move(490, 20); + await nextTick(); + vi.advanceTimersByTime(600); + + expect(wrapper.emitted('navigatePage')).toEqual([[2], [2]]); + }); +}); diff --git a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticleList.vue b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticleList.vue index 75aeb86d1..9e886d1d3 100644 --- a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticleList.vue +++ b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticleList.vue @@ -1,6 +1,5 @@ { ); }); + it('adopts the backend re-spaced positions when the response returns them', async () => { + const serverPositions = { 1: 10, 2: 30, 3: 20 }; + axios.post.mockResolvedValue({ data: { positions: serverPositions } }); + + await actions.reorder( + { commit, state }, + { + portalSlug: 'test-portal', + categorySlug: 'test-category', + reorderedGroup: { 3: 25 }, + } + ); + + expect(commit).toHaveBeenCalledWith( + types.default.SET_ARTICLE_POSITIONS, + serverPositions + ); + }); + it('rolls back positions and throws when API call fails', async () => { axios.post.mockRejectedValue({ message: 'Network error' }); const reorderedGroup = { 1: 1, 2: 2 }; diff --git a/app/models/article.rb b/app/models/article.rb index a04ca05fe..9d1247e8b 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -137,15 +137,41 @@ class Article < ApplicationRecord end def self.update_positions(portal:, positions_hash:) - return if positions_hash.blank? + return {} if positions_hash.blank? + + moved_ids = positions_hash.keys.map(&:to_i) transaction do positions_hash.each do |article_id, new_position| portal.articles.find(article_id).update!(position: new_position) end + # Re-space touched categories to clean gaps and return the final positions + rebalance_positions(portal, moved_ids) end end + def self.rebalance_positions(portal, moved_ids) + category_ids = portal.articles.where(id: moved_ids).distinct.pluck(:category_id).compact + category_ids.each_with_object({}) do |category_id, positions| + resequence_category(portal, category_id, moved_ids, positions) + end + end + + def self.resequence_category(portal, category_id, moved_ids, positions) + ordered = portal.articles.where(category_id: category_id) + .sort_by { |article| [article.position || 0, moved_ids.include?(article.id) ? 1 : 0, article.id] } + return if ordered.length < 2 # a lone article can't collide, leave it as-is + + ordered.each_with_index do |article, index| + new_position = (index + 1) * 10 + positions[article.id] = new_position + next if article.position == new_position + + article.update_column(:position, new_position) # rubocop:disable Rails/SkipsModelValidations + end + end + private_class_method :rebalance_positions, :resequence_category + private def category_id_changed_action diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 04466ccd1..cdad2d9f4 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -207,4 +207,29 @@ RSpec.describe Article do expect(article.to_llm_text).to eq(expected_output) end end + + describe '.update_positions' do + let!(:article_a) { create(:article, portal: portal_1, category: category_1, author: user, position: 10) } + let!(:article_b) { create(:article, portal: portal_1, category: category_1, author: user, position: 11) } + let!(:article_c) { create(:article, portal: portal_1, category: category_1, author: user, position: 30) } + + it 're-spaces the category to clean gaps and places a collided move after its tie' do + # Dropping C into the tight 10/11 gap gives a floored midpoint of 10, colliding with A + positions = described_class.update_positions(portal: portal_1, positions_hash: { article_c.id => 10 }) + + expect(article_a.reload.position).to eq(10) + expect(article_c.reload.position).to eq(20) + expect(article_b.reload.position).to eq(30) + expect(positions).to eq(article_a.id => 10, article_c.id => 20, article_b.id => 30) + end + + it 'leaves a lone article untouched and returns nothing to sync' do + lone = create(:article, portal: portal_1, category: create(:category, portal_id: portal_1.id), author: user, position: 20) + + positions = described_class.update_positions(portal: portal_1, positions_hash: { lone.id => 20 }) + + expect(lone.reload.position).to eq(20) + expect(positions).to be_empty + end + end end