From 1bccbc03fa1e869cb76a739648e357f7c6389fd7 Mon Sep 17 00:00:00 2001 From: negue Date: Tue, 26 Jan 2021 00:40:18 +0100 Subject: [PATCH] Hotfix: moving pinned items (#12935) * increase checks for moving pinned items - fixes #10406 * allow to move official pinneditems * using common object instead of method import --- .../integration/user/POST-move-pinned-item.js | 95 ++++++++++++++++++- website/common/script/index.js | 2 + website/server/controllers/api-v3/user.js | 23 +++-- 3 files changed, 111 insertions(+), 9 deletions(-) diff --git a/test/api/v3/integration/user/POST-move-pinned-item.js b/test/api/v3/integration/user/POST-move-pinned-item.js index f7de9c65b2..e3792f2fa7 100644 --- a/test/api/v3/integration/user/POST-move-pinned-item.js +++ b/test/api/v3/integration/user/POST-move-pinned-item.js @@ -3,15 +3,15 @@ import { } from '../../../../helpers/api-integration/v3'; import getOfficialPinnedItems from '../../../../../website/common/script/libs/getOfficialPinnedItems'; +import content from '../../../../../website/common/script/content'; describe('POST /user/move-pinned-item/:path/move/to/:position', () => { let user; - let officialPinnedItems; let officialPinnedItemPaths; beforeEach(async () => { user = await generateUser(); - officialPinnedItems = getOfficialPinnedItems(user); + const officialPinnedItems = getOfficialPinnedItems(user); officialPinnedItemPaths = []; // officialPinnedItems are returned in { type: ..., path:... } format @@ -83,7 +83,7 @@ describe('POST /user/move-pinned-item/:path/move/to/:position', () => { expect(res).to.eql(expectedResponse); }); - it('adjusts the order of pinned items with order mismatch', async () => { + it('adjusts the order of pinned items with order mismatch - existing item in order', async () => { const testPinnedItems = [ { type: 'card', path: 'cardTypes.thankyou' }, { type: 'card', path: 'cardTypes.greeting' }, @@ -125,6 +125,95 @@ describe('POST /user/move-pinned-item/:path/move/to/:position', () => { expect(res).to.eql(expectedResponse); }); + it('adjusts the order of pinned items with order mismatch - not existing in order', async () => { + const testPinnedItems = [ + { type: 'card', path: 'cardTypes.thankyou' }, + { type: 'card', path: 'cardTypes.greeting' }, + { type: 'potion', path: 'potion' }, + { type: 'armoire', path: 'armoire' }, + ]; + + const testPinnedItemsOrder = [ + 'armoire', + 'potion', + ]; + + await user.update({ + pinnedItems: testPinnedItems, + pinnedItemsOrder: testPinnedItemsOrder, + }); + await user.sync(); + + await user.post('/user/move-pinned-item/cardTypes.greeting/move/to/2'); + await user.sync(); + + // The basic test + expect(user.pinnedItemsOrder[2]).to.equal('cardTypes.greeting'); + + // potion is now the last item because the 2 unacounted for cards show up + // at the beginning of the order + expect(user.pinnedItemsOrder[user.pinnedItemsOrder.length - 1]).to.equal('potion'); + }); + + it('adjusts the order of official pinned items with order mismatch - not existing in order', async () => { + const testPinnedItems = [ + { type: 'card', path: 'cardTypes.thankyou' }, + { type: 'card', path: 'cardTypes.greeting' }, + { type: 'potion', path: 'potion' }, + ]; + + const testPinnedItemsOrder = [ + 'potion', + ]; + + const { officialPinnedItems } = content; + + // add item to pinned + officialPinnedItems.push({ type: 'armoire', path: 'armoire' }); + + await user.update({ + pinnedItems: testPinnedItems, + pinnedItemsOrder: testPinnedItemsOrder, + }); + await user.sync(); + + await user.post('/user/move-pinned-item/armoire/move/to/2'); + await user.sync(); + + // The basic test + expect(user.pinnedItemsOrder[2]).to.equal('armoire'); + + // potion is now the last item because the 2 unacounted for cards show up + // at the beginning of the order + expect(user.pinnedItemsOrder[user.pinnedItemsOrder.length - 1]).to.equal('potion'); + }); + + it('adjusts the order of pinned items with order mismatch - not existing - out of length', async () => { + const testPinnedItems = [ + { type: 'card', path: 'cardTypes.thankyou' }, + { type: 'card', path: 'cardTypes.greeting' }, + { type: 'potion', path: 'potion' }, + { type: 'armoire', path: 'armoire' }, + ]; + + const testPinnedItemsOrder = [ + 'armoire', + 'potion', + ]; + + await user.update({ + pinnedItems: testPinnedItems, + pinnedItemsOrder: testPinnedItemsOrder, + }); + await user.sync(); + + await user.post('/user/move-pinned-item/cardTypes.greeting/move/to/33'); + await user.sync(); + + // since the target was out of bounce it added it to the last item + expect(user.pinnedItemsOrder[user.pinnedItemsOrder.length - 1]).to.equal('cardTypes.greeting'); + }); + it('cannot move pinned item that you do not have pinned', async () => { const testPinnedItems = [ { type: 'potion', path: 'potion' }, diff --git a/website/common/script/index.js b/website/common/script/index.js index 5de032345c..ac199818d2 100644 --- a/website/common/script/index.js +++ b/website/common/script/index.js @@ -89,6 +89,7 @@ import updateTask from './ops/updateTask'; // TODO under api.libs.statHelpers? import * as statHelpers from './statHelpers'; import { unEquipByType } from './ops/unequip'; +import getOfficialPinnedItems from './libs/getOfficialPinnedItems'; const api = {}; api.content = content; @@ -145,6 +146,7 @@ api.hasClass = hasClass; api.onboarding = onboarding; api.setDebuffPotionItems = setDebuffPotionItems; api.getDebuffPotionItems = getDebuffPotionItems; +api.getOfficialPinnedItems = getOfficialPinnedItems; api.fns = { autoAllocate, diff --git a/website/server/controllers/api-v3/user.js b/website/server/controllers/api-v3/user.js index 9ec6249de0..83edc48dbc 100644 --- a/website/server/controllers/api-v3/user.js +++ b/website/server/controllers/api-v3/user.js @@ -1716,7 +1716,7 @@ api.movePinnedItem = { const { user } = res.locals; const { path } = req.params; - const position = Number(req.params.position); + let position = Number(req.params.position); // If something has been added or removed from the inAppRewards, we need // to reset pinnedItemsOrder to have the correct length. Since inAppRewards @@ -1727,17 +1727,28 @@ api.movePinnedItem = { user.pinnedItemsOrder = currentPinnedItems.map(item => item.path); } + const officialItems = common.getOfficialPinnedItems(user); + + const itemExistInPinnedArray = user.pinnedItems.findIndex(item => item.path === path); + const itemExistInOfficialItems = officialItems.findIndex(item => item.path === path); + + if (itemExistInPinnedArray === -1 && itemExistInOfficialItems === -1) { + throw new BadRequest(res.t('wrongItemPath', { path }, req.language)); + } + // Adjust the order const currentIndex = user.pinnedItemsOrder.findIndex(item => item === path); const currentPinnedItemPath = user.pinnedItemsOrder[currentIndex]; - if (currentIndex === -1) { - throw new BadRequest(res.t('wrongItemPath', { path }, req.language)); + if (currentIndex !== -1) { + // Remove the one we will move + user.pinnedItemsOrder.splice(currentIndex, 1); + } else { + // usually the array would be already fixed by the inAppRewards call + // but it seems something didn't work out + position = Math.min(position, user.pinnedItemsOrder.length - 1); } - // Remove the one we will move - user.pinnedItemsOrder.splice(currentIndex, 1); - // reinsert the item in position (or just at the end) if (position === -1) { user.pinnedItemsOrder.push(currentPinnedItemPath);