From 8b190c4d3115e0e33edc219d1a17f84e5f97c1b0 Mon Sep 17 00:00:00 2001 From: Petrus Pierre Date: Fri, 19 Feb 2021 15:20:53 -0300 Subject: [PATCH] New position logic for drag and drop beta version (#716) * Add new position to story model * Create populate new position task * Create sort stories route * Added ordering stories specs * Added sort stories to redux * Implemented new position logic * Update and fix specs * Update changelog * Remove a console.log * Remove unnecessary PunditContext * Change new route path * Refactored SortStories reducer * Fixed beta stories controller spec * Refactores projectBoard beta model * Change calculatePositions arguments * Default value to calculatePosition arguments * Added logging to populate new position task * Fixed Story model spec * Fixed some CodeClimate issues * Fixed cypress specs --- CHANGELOG.md | 6 ++ app/assets/javascripts/actions/actionTypes.js | 1 + app/assets/javascripts/actions/story.js | 11 ++- .../components/projects/ProjectBoard.js | 11 ++- .../javascripts/models/beta/projectBoard.js | 73 +++++++++++-------- app/assets/javascripts/models/beta/story.js | 23 ++++-- app/assets/javascripts/reducers/stories.js | 12 ++- app/controllers/beta/stories_controller.rb | 19 +++++ app/models/story.rb | 2 +- app/policies/story_policy.rb | 4 + app/services/beta/sort_stories.rb | 39 ++++++++++ config/routes.rb | 5 ++ ...0210201140311_add_new_position_to_story.rb | 8 ++ db/schema.rb | 3 +- lib/tasks/populate_newposition.rake | 26 +++++++ spec/cypress/integration/dragAndDrop_spec.js | 14 ++-- spec/cypress/support/commands.js | 10 +++ spec/javascripts/actions/story_spec.js | 27 +++---- .../models/beta/project_board_spec.js | 32 ++++---- spec/javascripts/models/beta/story_spec.js | 54 +++++++------- spec/javascripts/selectors/backlog_spec.js | 16 ++-- spec/models/story_spec.rb | 2 +- spec/rails_helper.rb | 5 +- spec/requests/beta/stories_controller_spec.rb | 48 ++++++++++++ 24 files changed, 330 insertions(+), 121 deletions(-) create mode 100644 app/controllers/beta/stories_controller.rb create mode 100644 app/services/beta/sort_stories.rb create mode 100644 db/migrate/20210201140311_add_new_position_to_story.rb create mode 100644 lib/tasks/populate_newposition.rake create mode 100644 spec/requests/beta/stories_controller_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b525b97e1..4819da901 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ ### Fixed - Bug when change release date of story. +### Added +- New position column to Story + +### Changed +- Change drag and drop logic in beta version + ## [2.6.0] 2020-03-11 ### Added diff --git a/app/assets/javascripts/actions/actionTypes.js b/app/assets/javascripts/actions/actionTypes.js index 62663d3cb..5b5d75954 100644 --- a/app/assets/javascripts/actions/actionTypes.js +++ b/app/assets/javascripts/actions/actionTypes.js @@ -22,6 +22,7 @@ export default keyMirror({ RECEIVE_HISTORY_ERROR: null, CLOSE_HISTORY: null, UPDATE_STORY_SUCCESS: null, + SORT_STORIES_SUCCESS: null, STORY_FAILURE: null, SET_LOADING_STORY: null, ADD_TASK: null, diff --git a/app/assets/javascripts/actions/story.js b/app/assets/javascripts/actions/story.js index ff6f0b4ae..7f10cd751 100644 --- a/app/assets/javascripts/actions/story.js +++ b/app/assets/javascripts/actions/story.js @@ -58,6 +58,12 @@ export const updateStorySuccess = (story, from) => ({ from }); +export const sortStoriesSuccess = (stories, from) => ({ + type: actionTypes.SORT_STORIES_SUCCESS, + stories, + from +}); + export const optimisticallyUpdate = (story, from) => ({ type: actionTypes.OPTIMISTICALLY_UPDATE, story, @@ -183,11 +189,10 @@ export const dragDropStory = (storyId, projectId, newAttributes, from) => try { dispatch(optimisticallyUpdate(newStory, from)); - const updatedStory = await Story.update(newStory, projectId); + const updatedStories = await Story.updatePosition(newStory); await wait(300); - - return dispatch(updateStorySuccess(updatedStory, from)); + return dispatch(sortStoriesSuccess(updatedStories, from)); } catch (error) { dispatch(sendErrorNotification(error)) diff --git a/app/assets/javascripts/components/projects/ProjectBoard.js b/app/assets/javascripts/components/projects/ProjectBoard.js index c354c5430..e948a3eee 100644 --- a/app/assets/javascripts/components/projects/ProjectBoard.js +++ b/app/assets/javascripts/components/projects/ProjectBoard.js @@ -10,14 +10,14 @@ import { CHILLY_BIN, DONE, BACKLOG, EPIC } from "../../models/beta/column"; import PropTypes from "prop-types"; import { canCloseColumn, - getNewPosition, + getPositions, getNewSprints, getNewState, moveStory, getSprintColumn, dragStory } from '../../models/beta/projectBoard'; -import { historyStatus, columns } from 'libs/beta/constants'; +import { historyStatus, columns, storyTypes } from 'libs/beta/constants'; import StoryPropTypes from '../shapes/story'; import ProjectBoardPropTypes from '../shapes/projectBoard'; import Notifications from '../Notifications'; @@ -82,8 +82,9 @@ export const ProjectBoard = ({ if (isSameColumn && sourceIndex === destinationIndex) return; if (!dropColumn) return; + if (!isSameColumn && dragStory.storyType === storyTypes.FEATURE && !dragStory.estimate) return; - const newPosition = getNewPosition( + const [position, newPosition] = getPositions( destinationIndex, sourceIndex, destinationArray, @@ -91,6 +92,7 @@ export const ProjectBoard = ({ dragStory.state, ); + const newStories = moveStory( sourceArray, destinationArray, @@ -112,7 +114,8 @@ export const ProjectBoard = ({ const newState = getNewState(isSameColumn, dropColumn, dragStory.state); return dragDropStory(dragStory.id, dragStory.projectId, { - position: newPosition, + position, + newPosition, state: newState, }); }; diff --git a/app/assets/javascripts/models/beta/projectBoard.js b/app/assets/javascripts/models/beta/projectBoard.js index 708e321eb..5656fc55e 100644 --- a/app/assets/javascripts/models/beta/projectBoard.js +++ b/app/assets/javascripts/models/beta/projectBoard.js @@ -68,21 +68,28 @@ export const toggleColumn = (projectBoard, column, callback) => { } // Drag And drop utils -const calculatePosition = (aboveStory, bellowStory, storyState) => { - const aboveStoryState = aboveStory?.state; - const bellowStoryState = bellowStory?.state; - const aboveStoryPosition = aboveStory?.position; - const bellowStoryPosition = bellowStory?.position; - - if (!bellowStory) return Number(aboveStoryPosition) + 1; - if (!aboveStory) return Number(bellowStoryPosition) - 1; - if (aboveStoryState === storyState && bellowStoryState !== storyState) return Number(aboveStoryPosition) + 1; - if (bellowStoryState === storyState && aboveStoryState !== storyState) return Number(bellowStoryPosition) - 1; - - return (Number(bellowStoryPosition) + Number(aboveStoryPosition)) / 2; -}; +const calculatePositions = (aboveStory = {}, belowStory = {}, storyState = {}) => { + const aboveStoryState = aboveStory.state; + const belowStoryState = belowStory.state; + const aboveStoryPosition = Number(aboveStory.position); + const belowStoryPosition = Number(belowStory.position); + const aboveStoryNewPosition = aboveStory.newPosition; + const belowStoryNewPosition = belowStory.newPosition; + + const isFirstStory = !aboveStoryState; + const isLastStory = !belowStoryState; + const isLastStoryInSameState = aboveStoryState === storyState && belowStoryState !== storyState; + const isFirstStoryInSameState = belowStoryState === storyState && aboveStoryState !== storyState; + + // return [position, newPosition] + if (isFirstStory) return [belowStoryPosition - 1, 1]; + if (isFirstStoryInSameState) return [belowStoryPosition - 1, belowStoryNewPosition - 1]; + if (isLastStory || isLastStoryInSameState) return [aboveStoryPosition + 1, aboveStoryNewPosition + 1]; + + return [(belowStoryPosition + aboveStoryPosition) / 2, aboveStoryNewPosition + 1]; +} -export const getNewPosition = ( +export const getPositions = ( destinationIndex, sourceIndex, storiesArray, @@ -90,22 +97,14 @@ export const getNewPosition = ( storyState, ) => { if (isEmpty(storiesArray)) { - return 1; // if array is empty than set 1 to story position + return [1, 1]; } if (!isSameColumn || sourceIndex > destinationIndex) { - return calculatePosition( - storiesArray[destinationIndex - 1], - storiesArray[destinationIndex], - storyState, - ); + return calculatePositions(storiesArray[destinationIndex - 1], storiesArray[destinationIndex], storyState); } - return calculatePosition( - storiesArray[destinationIndex], - storiesArray[destinationIndex + 1], - storyState, - ); + return calculatePositions(storiesArray[destinationIndex], storiesArray[destinationIndex + 1], storyState); }; // reorder the array @@ -114,16 +113,28 @@ export const moveStory = ( destinationArray, sourceIndex, destinationIndex, + isSameColumn ) => { - const newSourceArray = [...sourceArray]; - const [removed] = newSourceArray.splice(sourceIndex, 1); - const newDestinationArray = [...destinationArray]; - const filteredArray = newDestinationArray.filter((el, index) => index !== sourceIndex); - filteredArray.splice(destinationIndex, 0, removed); + const removed = sourceArray[sourceIndex]; + + const newDestinationArray = isSameColumn + ? destinationArray.filter((_, index) => index !== sourceIndex) + : [...destinationArray]; - return [...filteredArray]; + newDestinationArray.splice(destinationIndex, 0, removed); + + return sortStories(newDestinationArray, destinationIndex); }; +export const sortStories = (destinationArray, destinationIndex) => { + return destinationArray.map((item, index) => { + if (index >= destinationIndex) { + return { ...item, newPosition: item.newPosition + 1 } + }; + return item; + }); +} + export const getNewSprints = (newStories, sprints, sprintIndex) => sprints.map((sprint, index) => index === sprintIndex ? { ...sprint, stories: newStories } : sprint, diff --git a/app/assets/javascripts/models/beta/story.js b/app/assets/javascripts/models/beta/story.js index 2ad35721a..efd46cd31 100644 --- a/app/assets/javascripts/models/beta/story.js +++ b/app/assets/javascripts/models/beta/story.js @@ -22,8 +22,8 @@ export const needConfirmation = story => story.state === status.RELEASE export const comparePosition = (a, b) => { - const positionA = parseFloat(a.position); - const positionB = parseFloat(b.position); + const positionA = a.newPosition; + const positionB = b.newPosition; return compareValues(positionA, positionB); }; @@ -108,20 +108,30 @@ export const findById = (stories, id) => { return stories.find(story => story.id === id) } +export const updatePosition = async (story) => { + const serializedStory = serialize(story); + + const { data } = await httpService.post(`/beta/stories/${story.id}/position`, { story: serializedStory }); + + const deserializedData = data.map((item) => (deserialize(item.story))); + + return deserializedData; +} + export const update = async (story, projectId, options) => { - const newStory = serialize(story); + const serializedStory = serialize(story); const { data } = await httpService - .put(`/projects/${projectId}/stories/${story.id}`, { story: newStory }); + .put(`/projects/${projectId}/stories/${story.id}`, { story: serializedStory }); return deserialize(data.story, options); }; export const post = async (story, projectId) => { - const newStory = serialize(story); + const serializedStory = serialize(story); const { data } = await httpService - .post(`/projects/${projectId}/stories`, { story: newStory }); + .post(`/projects/${projectId}/stories`, { story: serializedStory }); return deserialize(data.story); }; @@ -374,6 +384,7 @@ const emptyStory = { createdAt: '', updatedAt: '', position: '', + newPosition: null, labels: [], requestedByName: '', ownedByName: null, diff --git a/app/assets/javascripts/reducers/stories.js b/app/assets/javascripts/reducers/stories.js index 5e66e8b2f..b0616575a 100644 --- a/app/assets/javascripts/reducers/stories.js +++ b/app/assets/javascripts/reducers/stories.js @@ -74,6 +74,16 @@ const storiesReducer = (state = initialState, action) => { addNewAttributes(story, { ...action.story, needsToSave: false, loading: false }) )) }) + case actionTypes.SORT_STORIES_SUCCESS: + return allScopes(state, null, stories => { + return stories.map((story) => { + const editingStory = action.stories.find((incomingStory) => story.id === incomingStory.id) + + return editingStory + ? addNewAttributes(story, { ...editingStory, needsToSave: false, loading: false }) + : story + }) + }) case actionTypes.OPTIMISTICALLY_UPDATE: return allScopes(state, action.story.id, stories => { return stories.map( @@ -81,7 +91,7 @@ const storiesReducer = (state = initialState, action) => { const newStory = setLoadingValue(action.story, true); return addNewAttributes(story, { ...newStory, needsToSave: false }); } - )) + )) }) case actionTypes.STORY_FAILURE: return { diff --git a/app/controllers/beta/stories_controller.rb b/app/controllers/beta/stories_controller.rb new file mode 100644 index 000000000..9dee3725d --- /dev/null +++ b/app/controllers/beta/stories_controller.rb @@ -0,0 +1,19 @@ +class Beta::StoriesController < ApplicationController + before_action :set_project_and_story + def position + authorize Story + stories = Beta::SortStories.new(@story, allowed_params).call + render json: stories + end + + private + + def allowed_params + params.require(:story).permit(:position, :new_position, :id, :state, :project_id) + end + + def set_project_and_story + @project = policy_scope(Project).find(allowed_params[:project_id]) + @story = policy_scope(Story).find(allowed_params[:id]) + end +end diff --git a/app/models/story.rb b/app/models/story.rb index ae1aeaa73..4c1f70ed8 100644 --- a/app/models/story.rb +++ b/app/models/story.rb @@ -103,7 +103,7 @@ def from_csv_row(row) title release_date accepted_at created_at updated_at delivered_at description project_id story_type owned_by_id requested_by_id owned_by_name owned_by_initials requested_by_name estimate - state position id labels + state position id labels new_position ].freeze JSON_METHODS = %w[errors notes documents tasks].freeze diff --git a/app/policies/story_policy.rb b/app/policies/story_policy.rb index b1c60dbca..4794c8e85 100644 --- a/app/policies/story_policy.rb +++ b/app/policies/story_policy.rb @@ -27,6 +27,10 @@ def sort? update? end + def position? + update? + end + def backlog? update? end diff --git a/app/services/beta/sort_stories.rb b/app/services/beta/sort_stories.rb new file mode 100644 index 000000000..b4b0170a1 --- /dev/null +++ b/app/services/beta/sort_stories.rb @@ -0,0 +1,39 @@ +class Beta::SortStories + def initialize(story, sort_params) + @story = story + @new_position = sort_params[:new_position] + @new_state = sort_params[:state] + @position = sort_params[:position] + end + + def call + update_current_story + update_stories_positions + stories + end + + private + + def update_current_story + @story.update(new_position: @new_position, state: @new_state, position: @position) + end + + def stories_to_update + stories.where.not(id: @story.id) + end + + def update_stories_positions + stories_to_update.update_all('new_position = new_position + 1') + end + + def stories + case @new_state + when 'started', 'finished', 'delivered' + @story.project.stories.in_progress.where('new_position >= ?', @new_position) + when 'unstarted' + @story.project.stories.backlog.where('new_position >= ?', @new_position) + when 'unscheduled' + @story.project.stories.chilly_bin.where('new_position >= ?', @new_position) + end + end +end diff --git a/config/routes.rb b/config/routes.rb index d20789f79..2b1b5ef7a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -73,6 +73,11 @@ namespace :beta do resources :projects, only: :show resources :project_boards, only: :show + resources :stories, only: [] do + member do + post :position + end + end end resources :tag_groups diff --git a/db/migrate/20210201140311_add_new_position_to_story.rb b/db/migrate/20210201140311_add_new_position_to_story.rb new file mode 100644 index 000000000..9045dd2ed --- /dev/null +++ b/db/migrate/20210201140311_add_new_position_to_story.rb @@ -0,0 +1,8 @@ +class AddNewPositionToStory < ActiveRecord::Migration[5.2] + def change + add_column :stories, :new_position, :integer, null: true + end + def down + remove_column :stories, :new_position + end +end diff --git a/db/schema.rb b/db/schema.rb index d9ddd76b1..c6f5e05e4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_01_14_123043) do +ActiveRecord::Schema.define(version: 2021_02_01_140311) do # These are extensions that must be enabled in order to support this database enable_extension "hstore" @@ -186,6 +186,7 @@ t.date "release_date" t.datetime "delivered_at" t.string "branch" + t.integer "new_position" end create_table "tag_groups", id: :serial, force: :cascade do |t| diff --git a/lib/tasks/populate_newposition.rake b/lib/tasks/populate_newposition.rake new file mode 100644 index 000000000..af3fea182 --- /dev/null +++ b/lib/tasks/populate_newposition.rake @@ -0,0 +1,26 @@ +desc 'Populate new position column' +task populate_newposition: :environment do + puts "RUNNING TASK: populate_newposition\n\n" + + def update_column(stories) + new_position = 1 + stories.each_with_index do |story| + story.update_column(:new_position, new_position) + new_position += 1 + end + puts "✓ Updated #{stories.length} stories.\n\n" + end + + Project.find_each do |project| + puts 'Updating chilly_bin column...' + update_column(project.stories.chilly_bin.order(:position)) + puts 'Updating backlog column...' + update_column(project.stories.backlog.order(:position)) + puts 'Updating in_progress column...' + update_column(project.stories.in_progress.order(:position)) + puts 'Updating done column...' + update_column(project.stories.done.order(:position)) + end + + puts 'Task done.' +end diff --git a/spec/cypress/integration/dragAndDrop_spec.js b/spec/cypress/integration/dragAndDrop_spec.js index 4a4909d8a..6a30bd88b 100644 --- a/spec/cypress/integration/dragAndDrop_spec.js +++ b/spec/cypress/integration/dragAndDrop_spec.js @@ -10,9 +10,11 @@ describe("DragAndDrop", () => { beforeEach(() => { cy.app("clean"); //clean the db cy.app("load_seed"); // load the seeds + cy.exec("bundle exec rake populate_newposition"); cy.loginWith("foo@bar.com", "asdfasdf"); cy.aliasUpdateStory(); + cy.aliasUpdateStoryPosition(); }); describe("Drag and drop in same column", () => { @@ -30,7 +32,7 @@ describe("DragAndDrop", () => { // reorder cy.moveStory("@first", Keys.arrowDown, Keys.space); - cy.waitUpdateStory(200); + cy.waitUpdateStoryPosition(200); // wait loading story cy.wait(300); @@ -54,7 +56,7 @@ describe("DragAndDrop", () => { // reorder cy.moveStory("@second", Keys.arrowUp, Keys.space); - cy.waitUpdateStory(200); + cy.waitUpdateStoryPosition(200); // wait loading story cy.wait(300); @@ -83,7 +85,7 @@ describe("DragAndDrop", () => { .as("drag-story") .moveStory("@drag-story", Keys.arrowLeft, Keys.space); - cy.waitUpdateStory(200); + cy.waitUpdateStoryPosition(200); // check new order cy.getDraggablesFromColumn(backlog).should("not.contain", dragStory); @@ -112,7 +114,7 @@ describe("DragAndDrop", () => { .as("drag-element") .moveStory("@drag-element", Keys.arrowRight, Keys.space); - cy.waitUpdateStory(200); + cy.waitUpdateStoryPosition(200); // check new order cy.getDraggablesFromColumn(chillyBin).should("not.contain", dragStory); @@ -133,7 +135,7 @@ describe("DragAndDrop", () => { .as("drag-element") .moveStory("@drag-element", Keys.arrowRight, Keys.space); - cy.waitUpdateStory(200); + cy.waitUpdateStoryPosition(200); // check new order cy.getDraggablesFromColumn(chillyBin).should("not.contain", dragStory); @@ -154,8 +156,6 @@ describe("DragAndDrop", () => { .as("drag-element") .moveStory("@drag-element", Keys.arrowRight, Keys.space); - cy.waitUpdateStory(200); - // check new order cy.getDraggablesFromColumn(chillyBin).should("contain", dragStory); cy.getDraggablesFromColumn(backlog).should("not.contain", dragStory); diff --git a/spec/cypress/support/commands.js b/spec/cypress/support/commands.js index 61b73e963..a5e02e1c4 100644 --- a/spec/cypress/support/commands.js +++ b/spec/cypress/support/commands.js @@ -41,3 +41,13 @@ Cypress.Commands.add('waitUpdateStory', (httpCode = 200) => { cy.wait('@update-story') .should('have.property', 'status', httpCode); }) + +Cypress.Commands.add('aliasUpdateStoryPosition', () => { + cy.server(); + cy.route('POST', '/beta/stories/**').as('update-story-position'); +}) + +Cypress.Commands.add('waitUpdateStoryPosition', (httpCode = 200) => { + cy.wait('@update-story-position') + .should('have.property', 'status', httpCode); +}) diff --git a/spec/javascripts/actions/story_spec.js b/spec/javascripts/actions/story_spec.js index 75255f291..ae80fe146 100644 --- a/spec/javascripts/actions/story_spec.js +++ b/spec/javascripts/actions/story_spec.js @@ -265,29 +265,30 @@ describe('Story Actions', () => { describe('dragDropStory', () => { const story = storyFactory(); - const updatedStory = { ...story, position: 3.54 }; + const updatedStories = [{ ...story, newPosition: 4, id: 42 }, { ...story, newPosition: 6, id: 43 }] + const updatedStory = { ...story, newPosition: 4 }; const from = 1; - it('calls Story.updateStorySuccess with new position', async () => { + it('calls Story.sortStoriesSuccess with new position', async () => { const FakeStory = { findById: sinon.stub().returns(updatedStory), - update: sinon.stub().resolves(updatedStory), + updatePosition: sinon.stub().resolves(updatedStories), isNew: sinon.stub().returns(false), withScope: sinon.stub().returns([story]), addNewAttributes: sinon.stub().returns(story) }; - const fakeGetState = sinon.stub(); - fakeGetState.returns({ - stories: { all: [updatedStory] }, + const fakeGetState = sinon.stub().returns({ + stories: { all: updatedStories }, }); const fakeDispatch = sinon.stub().resolves({}); await Story.dragDropStory(story.id, story.projectId, { position: 3.54, + newPosition: 4, }, from)(fakeDispatch, fakeGetState, { Story: FakeStory }); - expect(fakeDispatch).toHaveBeenCalledWith(Story.updateStorySuccess(updatedStory, from)); + expect(fakeDispatch).toHaveBeenCalledWith(Story.sortStoriesSuccess(updatedStories, from)); }); @@ -296,14 +297,14 @@ describe('Story Actions', () => { const FakeStory = { findById: sinon.stub().returns(updatedStory), - update: sinon.stub().rejects(error), + updatePosition: sinon.stub().rejects(error), isNew: sinon.stub().returns(false), withScope: sinon.stub().returns([story]), addNewAttributes: sinon.stub().returns(story) }; const fakeGetState = sinon.stub().returns({ - stories: { all: [updatedStory] }, + stories: { all: updatedStories }, }); const fakeDispatch = sinon.stub().resolves({}); @@ -320,9 +321,9 @@ describe('Story Actions', () => { ); }); - it('do not dispatch updateStorySuccess', () => { + it('do not dispatch sortStoriesSuccess', () => { expect(fakeDispatch).not.toHaveBeenCalledWith( - Story.updateStorySuccess(updatedStory), + Story.sortStoriesSuccess(updatedStory), ); }); }); @@ -537,13 +538,13 @@ describe('Story Actions', () => { it('dispatch receiveStories with stories in epic scope', async () => { await Story.fetchEpic(label)(fakeDispatch, fakeGetState, { Story: fakeStory }); - + expect(fakeDispatch).toHaveBeenCalledWith(Story.receiveStories(stories, 'epic')); }); it('does not dispatch sendDefaultErrorNotification', async () => { await Story.fetchEpic(label)(fakeDispatch, fakeGetState, { Story: fakeStory }); - + expect(fakeDispatch).not.toHaveBeenCalledWith(sendDefaultErrorNotification()); }); }); diff --git a/spec/javascripts/models/beta/project_board_spec.js b/spec/javascripts/models/beta/project_board_spec.js index 289d0b822..adc9f3869 100644 --- a/spec/javascripts/models/beta/project_board_spec.js +++ b/spec/javascripts/models/beta/project_board_spec.js @@ -166,8 +166,8 @@ describe('ProjectBoard model', () => { describe('Drag and Drop', () => { - describe('getNewPosition', () => { - const array = [{ position: 24, state: 'finished' }, { position: 20, state: 'finished' }, { position: 16, state: 'unstarted' }, { position: 12, state: 'unstarted' }, { position: 8, state: 'started' }, { position: 4, state: 'started' }]; + describe('getPositions', () => { + const array = [{ newPosition: 24, position: 24, state: 'finished' }, { newPosition: 20, position: 20, state: 'finished' }, { newPosition: 16, position: 16, state: 'unstarted' }, { newPosition: 12, position: 12, state: 'unstarted' }, { newPosition: 8, position: 8, state: 'started' }, { newPosition: 4, position: 4, state: 'started' }]; describe('same column', () => { const isSameColumn = true; @@ -176,14 +176,14 @@ describe('ProjectBoard model', () => { const destinationIndex = 2; it('returns a new position', () => { expect( - ProjectBoard.getNewPosition( + ProjectBoard.getPositions( destinationIndex, sourceIndex, array, isSameColumn, 'unstarted', ), - ).toEqual(15); + ).toEqual([15,15]); }); }); @@ -192,14 +192,14 @@ describe('ProjectBoard model', () => { const destinationIndex = 1; it('returns a new position', () => { expect( - ProjectBoard.getNewPosition( + ProjectBoard.getPositions( destinationIndex, sourceIndex, array, isSameColumn, 'finished', ), - ).toEqual(21); + ).toEqual([21,21]); }); }); @@ -208,14 +208,14 @@ describe('ProjectBoard model', () => { const destinationIndex = 0; it('returns a new position', () => { expect( - ProjectBoard.getNewPosition( + ProjectBoard.getPositions( destinationIndex, sourceIndex, array, isSameColumn, 'finished', ), - ).toEqual(23); + ).toEqual([23,1]); }); }); @@ -224,14 +224,14 @@ describe('ProjectBoard model', () => { const destinationIndex = 5; it('returns a new position', () => { expect( - ProjectBoard.getNewPosition( + ProjectBoard.getPositions( destinationIndex, sourceIndex, array, isSameColumn, 'started', ), - ).toEqual(5); + ).toEqual([5,5]); }); }); }); @@ -243,14 +243,14 @@ describe('ProjectBoard model', () => { const destinationIndex = 2; it('returns a new position', () => { expect( - ProjectBoard.getNewPosition( + ProjectBoard.getPositions( destinationIndex, sourceIndex, array, isSameColumn, 'unstarted', ), - ).toEqual(15); + ).toEqual([15,15]); }); }); @@ -259,14 +259,14 @@ describe('ProjectBoard model', () => { const destinationIndex = 0; it('returns a new position', () => { expect( - ProjectBoard.getNewPosition( + ProjectBoard.getPositions( destinationIndex, sourceIndex, array, isSameColumn, 'finished', ), - ).toEqual(23); + ).toEqual([23,1]); }); }); @@ -275,14 +275,14 @@ describe('ProjectBoard model', () => { const destinationIndex = 6; it('returns a new position', () => { expect( - ProjectBoard.getNewPosition( + ProjectBoard.getPositions( destinationIndex, sourceIndex, array, isSameColumn, 'started', ), - ).toEqual(5); + ).toEqual([5,5]); }); }); }); diff --git a/spec/javascripts/models/beta/story_spec.js b/spec/javascripts/models/beta/story_spec.js index 0c0e08715..00c826876 100644 --- a/spec/javascripts/models/beta/story_spec.js +++ b/spec/javascripts/models/beta/story_spec.js @@ -7,11 +7,11 @@ describe('Story model', function () { describe('when position A is bigger then B', () => { it("return 1", () => { const storyA = { - position: "10.7" + newPosition: 10 }; const storyB = { - position: "4.3" + newPosition: 4 } expect(Story.comparePosition(storyA, storyB)).toEqual(1) @@ -21,11 +21,11 @@ describe('Story model', function () { describe('when position A is lower then B', () => { it("return -1", () => { const storyA = { - position: "3.4" + newPosition: 3 }; const storyB = { - position: "5.3" + newPosition: 5 } expect(Story.comparePosition(storyA, storyB)).toEqual(-1) @@ -35,11 +35,11 @@ describe('Story model', function () { describe('when position A is equal B', () => { it("return 0", () => { const storyA = { - position: "5.0" + newPosition: 5 }; const storyB = { - position: "5.0" + newPosition: 5 } expect(Story.comparePosition(storyA, storyB)).toEqual(0) @@ -257,7 +257,7 @@ describe('Story model', function () { const story = { _editing: { storyType: FEATURE, estimate: 1 } }; const newAttributes = { storyType: noFeatureType } let changedStory; - + beforeEach(() => { changedStory = Story.editStory(story, newAttributes); }) @@ -265,7 +265,7 @@ describe('Story model', function () { it('change story estimate to ""', () => { expect(changedStory._editing.estimate).toEqual(''); }) - + it(`change story type is ${noFeatureType}`, () => { expect(changedStory._editing.storyType).toEqual(noFeatureType); }) @@ -281,8 +281,8 @@ describe('Story model', function () { beforeEach(() => { changedStory = Story.editStory(story, newAttributes); }); - - it("keep estimate 1", () => { + + it("keep estimate 1", () => { expect(changedStory._editing.estimate).toEqual(1); }); @@ -303,7 +303,7 @@ describe('Story model', function () { changedStory = Story.editStory(story, newAttributes); }); - it(`change state to unscheduled `, () => { + it(`change state to unscheduled `, () => { expect(changedStory._editing.state).toEqual(UNSCHEDULED); }); @@ -320,7 +320,7 @@ describe('Story model', function () { const story = { _editing: { storyType: FEATURE, estimate: '', state: UNSCHEDULED } }; const newAttributes = { estimate: estimatedValue }; let changedStory; - + beforeEach(() => { changedStory = Story.editStory(story, newAttributes); }); @@ -341,12 +341,12 @@ describe('Story model', function () { describe(`when story type is ${noFeatureType}`, () => { const otherNotFeatureTypes = notFeatureTypes.filter(type => type !== noFeatureType); - otherNotFeatureTypes.forEach(otherNotFeatureType => { + otherNotFeatureTypes.forEach(otherNotFeatureType => { describe(`and new story type is ${otherNotFeatureType}`, () => { const story = { _editing: { storyType: noFeatureType, estimate: '' } }; const newAttributes = { storyType: otherNotFeatureType } let changedStory; - + beforeEach(() => { changedStory = Story.editStory(story, newAttributes); }); @@ -379,10 +379,10 @@ describe('Story model', function () { describe(`and story status is ${UNSCHEDULED}`, () => { describe(`and new story type is feature`, () => { - const story = { _editing: { - storyType: noFeatureType, - estimate: '', - state: UNSCHEDULED } + const story = { _editing: { + storyType: noFeatureType, + estimate: '', + state: UNSCHEDULED } }; const newAttributes = { storyType: FEATURE } let changedStory; @@ -391,7 +391,7 @@ describe('Story model', function () { changedStory = Story.editStory(story, newAttributes); }); - it('change estimate to ""', () => { + it('change estimate to ""', () => { expect(changedStory._editing.estimate).toEqual(''); }); @@ -407,11 +407,11 @@ describe('Story model', function () { describe(`when story status is ${UNSTARTED}`, () => { describe(`when new story type is feature`, () => { - const story = { _editing: { + const story = { _editing: { storyType: noFeatureType, estimate: 2, state: UNSTARTED - } + } }; const newAttributes = { storyType: FEATURE } let changedStory; @@ -444,7 +444,7 @@ describe('Story model', function () { changedStory = Story.editStory(story, newAttributes); }); - it("change estimate to ''", () => { + it("change estimate to ''", () => { expect(changedStory._editing.estimate).toEqual(''); }); @@ -1140,7 +1140,7 @@ describe('Story model', function () { describe('when highlighted is false', () => { it('returns falsy', () => { const story = { highlighted: false }; - + expect(Story.isHighlighted(story)).toBeFalsy(); }); }); @@ -1159,7 +1159,7 @@ describe('Story model', function () { expect(Story.isHighlighted(story)).toBeFalsy(); }); - }); + }); }); describe('isSearch', () => { @@ -1228,8 +1228,8 @@ describe('Story model', function () { describe('haveSearch', () => { describe('when have zero search stories', () => { - const stories = { - [storyScopes.SEARCH]: [] + const stories = { + [storyScopes.SEARCH]: [] }; it('returns falsy', () => { @@ -1341,7 +1341,7 @@ describe('Story model', function () { const story = { _editing: { id: 1, storyType: noFeatureType, estimate: '' } }; const newAttributes = { estimate: 2 }; const newStory = { ...story, ...newAttributes }; - + it('return an empty string', () => { expect(Story.estimateFor(story, newAttributes, newStory)).toEqual(''); }); diff --git a/spec/javascripts/selectors/backlog_spec.js b/spec/javascripts/selectors/backlog_spec.js index 181bd7077..f8d8388e8 100644 --- a/spec/javascripts/selectors/backlog_spec.js +++ b/spec/javascripts/selectors/backlog_spec.js @@ -75,7 +75,7 @@ describe('Backlog selector functions', () => { const stories = [newStory, oldStory]; const orderedStories = orderByState(stories); - + expect(orderedStories[0]).toEqual(oldStory); expect(orderedStories[1]).toEqual(newStory); }); @@ -97,7 +97,7 @@ describe('Backlog selector functions', () => { const stories = [newStory, oldStory]; const orderedStories = orderByState(stories); - + expect(orderedStories[0]).toEqual(oldStory); expect(orderedStories[1]).toEqual(newStory); }); @@ -119,7 +119,7 @@ describe('Backlog selector functions', () => { const stories = [newStory, oldStory]; const orderedStories = orderByState(stories); - + expect(orderedStories[0]).toEqual(oldStory); expect(orderedStories[1]).toEqual(newStory); }); @@ -127,24 +127,24 @@ describe('Backlog selector functions', () => { describe('All stories are rejected, finished or unstarted', () => { const storyStates = ['rejected', 'finished', 'unstarted']; - + it('sort by position', () => { storyStates.forEach(state => { const firstPosition = storyFactory({ id: 1, - position: '1', + newPosition: 1, state, }); const secondPosition = storyFactory({ id: 0, - position: '2', + newPosition: 2, state, }); - + const stories = [secondPosition, firstPosition]; const orderedStories = orderByState(stories); - + expect(orderedStories[0]).toEqual(firstPosition); expect(orderedStories[1]).toEqual(secondPosition); }); diff --git a/spec/models/story_spec.rb b/spec/models/story_spec.rb index ddb703b3a..8cead4073 100644 --- a/spec/models/story_spec.rb +++ b/spec/models/story_spec.rb @@ -469,7 +469,7 @@ title accepted_at created_at release_date updated_at delivered_at description project_id story_type owned_by_id requested_by_id requested_by_name owned_by_name owned_by_initials estimate - state position id errors labels notes tasks documents + state position id errors labels notes tasks documents new_position ].sort ) end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index ed7d29c92..5fa346e08 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -53,6 +53,7 @@ config.default_retry_count = 2 config.exceptions_to_retry = [Net::ReadTimeout] - config.include Devise::Test::ControllerHelpers, type: :controller - config.include IntegrationHelpers, type: :feature + config.include Devise::Test::ControllerHelpers, type: :controller + config.include IntegrationHelpers, type: :feature + config.include Devise::Test::IntegrationHelpers, type: :request end diff --git a/spec/requests/beta/stories_controller_spec.rb b/spec/requests/beta/stories_controller_spec.rb new file mode 100644 index 000000000..0612b33d7 --- /dev/null +++ b/spec/requests/beta/stories_controller_spec.rb @@ -0,0 +1,48 @@ +require 'rails_helper' + +RSpec.describe 'Ordering stories', type: :request do + let(:user) { create :user, :with_team } + let(:project) do + create(:project, name: 'Test Project', users: [user], teams: [user.teams.first]) + end + + context 'when moving story in the same state' do + let!(:bug) { create(:story, :active, project: project, requested_by: user, new_position: 1) } + let!(:chore) { create(:story, :active, project: project, requested_by: user, new_position: 2) } + let!(:feature) { create(:story, :active, project: project, requested_by: user, new_position: 3) } + + before do + sign_in user + post position_beta_story_path(id: chore.id), params: { story: { position: 4, new_position: 4, id: chore.id, state: 'started', project_id: project.id }, id: chore.id } + end + + it 'moves story to correct position' do + chore_new_position = chore.reload.new_position + bug_new_position = bug.reload.new_position + feature_new_position = feature.reload.new_position + expect(chore_new_position).to eq(4) + expect(bug_new_position).to eq(1) + expect(feature_new_position).to eq(3) + end + end + + context 'when moving story in different state' do + let!(:bug) { create(:story, state: 'unscheduled', project: project, requested_by: user, new_position: 1) } + let!(:chore) { create(:story, :active, project: project, requested_by: user, new_position: 1) } + let!(:feature) { create(:story, :active, project: project, requested_by: user, new_position: 2) } + + before do + sign_in user + post position_beta_story_path(id: bug.id), params: { story: { position: 0, new_position: 1, id: bug.id, state: 'started', project_id: project.id } } + end + + it 'moves story to correct position' do + chore_new_position = chore.reload.new_position + bug_new_position = bug.reload.new_position + feature_new_position = feature.reload.new_position + expect(chore_new_position).to eq(2) + expect(bug_new_position).to eq(1) + expect(feature_new_position).to eq(3) + end + end +end