From fa044ffb44c4e4ffaebe91bae23b284d0ee6a021 Mon Sep 17 00:00:00 2001 From: Kip Raske Date: Fri, 13 Apr 2018 08:22:06 -0500 Subject: [PATCH] Feature/sortable reward area (#9930) * Client POC We need to wrap each draggable region it its own div or else the "draggable" element will conflict with each other. This screws up the styling but that is totally fixable * Ah that ref was being used after all, changing back * Scaffold out a new callback for when we drag these things Next is going to be the hard part: I need to save the sort order for these to the database. I don't even know if there is a schema but hey this is the best place to start * Firefox caching is the problem: don't actually need the wrapper div So I guess I should try this in chrome and see how it works then come back to firefox and figure out what the heck is going on * Scaffolding out our API call to save the sort order The endpoint doesn't exist yet so we will need to add that * Ok we are now calling our API endpoint to reorder these things Of course it doesn't exist yet so you get a 404 when you try, but that is ok * Defining api endpoint, a work in progress In particular I really had ought to use _id for these too, it appears that the primary way we detect order doesn't even use "key" at all. * Switching to using the pinned item UUID This has much better results, but of course the server and client logic don't match now. Will have to keep working on my splice to make sure that they are the same * I thought this would fix our server/client mismatch but it is not it Something is really wrong with my logic somewhere, maybe I need to update the db step? * Moving this logic to the "user" rather than "tasks" and key off path Path is unique and is less finiky than dealing with string comparisons with ids. Unfortunately everything is still not working... I suppose user.update() doesn't care about the position? * This client code caused quite a lot of problems if you dragged fast We don't really need it it seems, so off it goes * Updating markup and CSS so it actually looks good. Everything is working horray!! I did just notice the following bug: the popover text sometimes makes it very annoying to drag because you can't drop over it@ * Cleaning up my comments in the API section user.js I had a lot of TODOS that are mostly done now * Fixing a spacing code standards thing * Turns out we never use type, so we should remove this from the API call * Adding pinnedItemsOrder into the user schema And disabling my call in the frontend before I do any more damage * Halfway to using pinnedItemsOrder This isn't working yet but it is not going to break it horribly like it was before. * Hooking up inAppRewards to always produce sorted information It is suspicially working right now even though I have not added the seasonal stuff logic yet... * Updating the comments in user.js in movedPinnedItem It turns out that my bandaid fix to just get the ball rolling perfectly does what I need it to do when we have a length discrepancy. So we are getting much closer to the final product, just need lots of testing * Cleaning up code standards kinds of things * Yay, this fixes the popover issue I hope this is the right "vue" way to do things, because I tried a bunch of other things that definately were not the right way to do it. And this appears to work too * ** Partial Work ** Starting tests on api call for draggable items Doesn't work, doesn't compile so don't include in PR! * Test failing still... This is worth a save. The api call grabs the seasonal items too, so we can't get away from using the common functions and calls here to get the actual list of items * Okay have the first test passing Need to clean up my linter problems though * Planning out the next two tests and fixing my format problems * 2nd Test case written, this time with the "more" odd case * Making sure that we didn't mess with pinned items * Huh... this test doesn't give me the expected result Drat, I guess I found a bug * Throw an error when we put garbage in our api call. Well, before we got user.pinnedItemsOrder filled with a bunch of "null" entries which is not ideal. it still worked, but isn't this confusing enough already? * Cleaning up the multitude of linting problems thanks gulp :) * Writing tests for inAppRewards.js, but something is wrong * Fixing my linting errors in inAppRewards tests These tests still do not run though, so they may fail and I would not know * Applying Negue's fixes to inAppRewards.js test It never occured to me that we shouldn't try to reach the database while in the common tests. Well, we shouldn't do that, we should use the common.helpers instead. Thanks! --- .../integration/user/POST-move-pinned-item.js | 148 ++++++++++++++++++ test/common/libs/inAppRewards.js | 84 ++++++++++ website/client/components/tasks/column.vue | 32 +++- website/client/store/actions/user.js | 5 + website/common/script/libs/inAppRewards.js | 30 +++- website/server/controllers/api-v3/user.js | 64 ++++++++ website/server/models/user/schema.js | 2 + 7 files changed, 361 insertions(+), 4 deletions(-) create mode 100644 test/api/v3/integration/user/POST-move-pinned-item.js create mode 100644 test/common/libs/inAppRewards.js diff --git a/test/api/v3/integration/user/POST-move-pinned-item.js b/test/api/v3/integration/user/POST-move-pinned-item.js new file mode 100644 index 0000000000..010e09d722 --- /dev/null +++ b/test/api/v3/integration/user/POST-move-pinned-item.js @@ -0,0 +1,148 @@ +import { + generateUser, +} from '../../../../helpers/api-integration/v3'; + +import getOfficialPinnedItems from '../../../../../website/common/script/libs/getOfficialPinnedItems.js'; + +describe('POST /user/move-pinned-item/:path/move/to/:position', () => { + let user; + let officialPinnedItems; + let officialPinnedItemPaths; + + beforeEach(async () => { + user = await generateUser(); + officialPinnedItems = getOfficialPinnedItems(user); + + officialPinnedItemPaths = []; + // officialPinnedItems are returned in { type: ..., path:... } format but we just need the paths for testPinnedItemsOrder + if (officialPinnedItems.length > 0) { + officialPinnedItemPaths = officialPinnedItems.map(item => item.path); + } + }); + + it('adjusts the order of pinned items with no order mismatch', async () => { + let testPinnedItems = [ + { type: 'armoire', path: 'armoire' }, + { type: 'potion', path: 'potion' }, + { type: 'marketGear', path: 'gear.flat.weapon_warrior_1' }, + { type: 'marketGear', path: 'gear.flat.head_warrior_1' }, + { type: 'marketGear', path: 'gear.flat.armor_warrior_1' }, + { type: 'hatchingPotions', path: 'hatchingPotions.Golden' }, + { type: 'marketGear', path: 'gear.flat.shield_warrior_1' }, + { type: 'card', path: 'cardTypes.greeting' }, + { type: 'potion', path: 'hatchingPotions.Golden' }, + { type: 'card', path: 'cardTypes.thankyou' }, + { type: 'food', path: 'food.Saddle' }, + ]; + + let testPinnedItemsOrder = [ + 'hatchingPotions.Golden', + 'cardTypes.greeting', + 'armoire', + 'gear.flat.weapon_warrior_1', + 'gear.flat.head_warrior_1', + 'cardTypes.thankyou', + 'gear.flat.armor_warrior_1', + 'food.Saddle', + 'gear.flat.shield_warrior_1', + 'potion', + ]; + + // For this test put seasonal items at the end so they stay out of the way + testPinnedItemsOrder = testPinnedItemsOrder.concat(officialPinnedItemPaths); + + await user.update({ + pinnedItems: testPinnedItems, + pinnedItemsOrder: testPinnedItemsOrder, + }); + + let res = await user.post('/user/move-pinned-item/armoire/move/to/5'); + await user.sync(); + + expect(user.pinnedItemsOrder[5]).to.equal('armoire'); + expect(user.pinnedItemsOrder[2]).to.equal('gear.flat.weapon_warrior_1'); + + // We have done nothing to change pinnedItems! + expect(user.pinnedItems).to.deep.equal(testPinnedItems); + + let expectedResponse = [ + 'hatchingPotions.Golden', + 'cardTypes.greeting', + 'gear.flat.weapon_warrior_1', + 'gear.flat.head_warrior_1', + 'cardTypes.thankyou', + 'armoire', + 'gear.flat.armor_warrior_1', + 'food.Saddle', + 'gear.flat.shield_warrior_1', + 'potion', + ]; + expectedResponse = expectedResponse.concat(officialPinnedItemPaths); + + expect(res).to.eql(expectedResponse); + }); + + it('adjusts the order of pinned items with order mismatch', async () => { + let testPinnedItems = [ + { type: 'card', path: 'cardTypes.thankyou' }, + { type: 'card', path: 'cardTypes.greeting' }, + { type: 'potion', path: 'potion' }, + { type: 'armoire', path: 'armoire' }, + ]; + + let testPinnedItemsOrder = [ + 'armoire', + 'potion', + ]; + + await user.update({ + pinnedItems: testPinnedItems, + pinnedItemsOrder: testPinnedItemsOrder, + }); + + let res = await user.post('/user/move-pinned-item/armoire/move/to/1'); + await user.sync(); + + // The basic test + expect(user.pinnedItemsOrder[1]).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'); + + let expectedResponse = [ + 'cardTypes.thankyou', + 'cardTypes.greeting', + 'potion', + ]; + // inAppRewards is used here and will by default put these seasonal items in the front like this: + expectedResponse = officialPinnedItemPaths.concat(expectedResponse); + // now put "armoire" in where we moved it: + expectedResponse.splice(1, 0, 'armoire'); + + expect(res).to.eql(expectedResponse); + }); + + it('cannot move pinned item that you do not have pinned', async () => { + let testPinnedItems = [ + { type: 'potion', path: 'potion' }, + { type: 'armoire', path: 'armoire' }, + ]; + + let testPinnedItemsOrder = [ + 'armoire', + 'potion', + ]; + + await user.update({ + pinnedItems: testPinnedItems, + pinnedItemsOrder: testPinnedItemsOrder, + }); + + try { + await user.post('/user/move-pinned-item/cardTypes.thankyou/move/to/1'); + } catch (err) { + expect(err).to.exist; + } + }); +}); diff --git a/test/common/libs/inAppRewards.js b/test/common/libs/inAppRewards.js new file mode 100644 index 0000000000..d7634c72b0 --- /dev/null +++ b/test/common/libs/inAppRewards.js @@ -0,0 +1,84 @@ +import { + generateUser, +} from '../../helpers/common.helper'; +import getOfficialPinnedItems from '../../../website/common/script/libs/getOfficialPinnedItems.js'; +import inAppRewards from '../../../website/common/script/libs/inAppRewards'; + +describe('inAppRewards', () => { + let user; + let officialPinnedItems; + let officialPinnedItemPaths; + let testPinnedItems; + let testPinnedItemsOrder; + + beforeEach(() => { + user = generateUser(); + officialPinnedItems = getOfficialPinnedItems(user); + + officialPinnedItemPaths = []; + // officialPinnedItems are returned in { type: ..., path:... } format but we just need the paths for testPinnedItemsOrder + if (officialPinnedItems.length > 0) { + officialPinnedItemPaths = officialPinnedItems.map(item => item.path); + } + + testPinnedItems = [ + { type: 'armoire', path: 'armoire' }, + { type: 'potion', path: 'potion' }, + { type: 'marketGear', path: 'gear.flat.weapon_warrior_1' }, + { type: 'marketGear', path: 'gear.flat.head_warrior_1' }, + { type: 'marketGear', path: 'gear.flat.armor_warrior_1' }, + { type: 'hatchingPotions', path: 'hatchingPotions.Golden' }, + { type: 'marketGear', path: 'gear.flat.shield_warrior_1' }, + { type: 'card', path: 'cardTypes.greeting' }, + { type: 'potion', path: 'hatchingPotions.Golden' }, + { type: 'card', path: 'cardTypes.thankyou' }, + { type: 'food', path: 'food.Saddle' }, + ]; + + testPinnedItemsOrder = [ + 'hatchingPotions.Golden', + 'cardTypes.greeting', + 'armoire', + 'gear.flat.weapon_warrior_1', + 'gear.flat.head_warrior_1', + 'cardTypes.thankyou', + 'gear.flat.armor_warrior_1', + 'food.Saddle', + 'gear.flat.shield_warrior_1', + 'potion', + ]; + + // For this test put seasonal items at the end so they stay out of the way + testPinnedItemsOrder = testPinnedItemsOrder.concat(officialPinnedItemPaths); + }); + + it('returns the pinned items in the correct order', () => { + user.pinnedItems = testPinnedItems; + user.pinnedItemsOrder = testPinnedItemsOrder; + + let result = inAppRewards(user); + + expect(result[2].path).to.eql('armoire'); + expect(result[9].path).to.eql('potion'); + }); + + it('does not return seasonal items which have been unpinned', () => { + if (officialPinnedItems.length === 0) { + return; // if no seasonal items, this test is not applicable + } + + let testUnpinnedItem = officialPinnedItems[0]; + let testUnpinnedPath = testUnpinnedItem.path; + let testUnpinnedItems = [ + { type: testUnpinnedItem.type, path: testUnpinnedPath}, + ]; + + user.pinnedItems = testPinnedItems; + user.pinnedItemsOrder = testPinnedItemsOrder; + user.unpinnedItems = testUnpinnedItems; + + let result = inAppRewards(user); + let itemPaths = result.map(item => item.path); + expect(itemPaths).to.not.include(testUnpinnedPath); + }); +}); diff --git a/website/client/components/tasks/column.vue b/website/client/components/tasks/column.vue index 6c198fc3d3..4f59bda022 100644 --- a/website/client/components/tasks/column.vue +++ b/website/client/components/tasks/column.vue @@ -37,8 +37,9 @@ .small-text {{$t(`${type}sDesc`)}} draggable.sortable-tasks( ref="tasksList", - @update='sorted', + @update='taskSorted', :options='{disabled: activeFilter.label === "scheduled"}', + class="sortable-tasks" ) task( v-for="task in taskList", @@ -49,12 +50,19 @@ :group='group', ) template(v-if="hasRewardsList") - .reward-items + draggable( + ref="rewardsList", + @update="rewardSorted", + @start="rewardDragStart", + @end="rewardDragEnd", + class="reward-items", + ) shopItem( v-for="reward in inAppRewards", :item="reward", :key="reward.key", :highlightBorder="reward.isSuggested", + :showPopover="showPopovers" @click="openBuyDialog(reward)", :popoverPosition="'left'" ) @@ -319,6 +327,7 @@ export default { quickAddText: '', quickAddFocused: false, quickAddRows: 1, + showPopovers: true, selectedItemToBuy: {}, }; @@ -450,7 +459,7 @@ export default { loadCompletedTodos: 'tasks:fetchCompletedTodos', createTask: 'tasks:create', }), - async sorted (data) { + async taskSorted (data) { const filteredList = this.taskList; const taskToMove = filteredList[data.oldIndex]; const taskIdToMove = taskToMove._id; @@ -494,6 +503,23 @@ export default { }); this.user.tasksOrder[`${this.type}s`] = newOrder; }, + async rewardSorted (data) { + const rewardsList = this.inAppRewards; + const rewardToMove = rewardsList[data.oldIndex]; + + let newOrder = await this.$store.dispatch('user:movePinnedItem', { + path: rewardToMove.path, + position: data.newIndex, + }); + this.user.pinnedItemsOrder = newOrder; + }, + rewardDragStart () { + // We need to stop popovers from interfering with our dragging + this.showPopovers = false; + }, + rewardDragEnd () { + this.showPopovers = true; + }, quickAdd (ev) { // Add a new line if Shift+Enter Pressed if (ev.shiftKey) { diff --git a/website/client/store/actions/user.js b/website/client/store/actions/user.js index 809407a63e..145ff2a216 100644 --- a/website/client/store/actions/user.js +++ b/website/client/store/actions/user.js @@ -111,6 +111,11 @@ export function togglePinnedItem (store, params) { return addedItem; } +export async function movePinnedItem (store, params) { + let response = await axios.post(`/api/v3/user/move-pinned-item/${params.path}/move/to/${params.position}`); + return response.data.data; +} + export function castSpell (store, params) { let spellUrl = `/api/v3/user/class/cast/${params.key}`; diff --git a/website/common/script/libs/inAppRewards.js b/website/common/script/libs/inAppRewards.js index 98e641c918..939925af52 100644 --- a/website/common/script/libs/inAppRewards.js +++ b/website/common/script/libs/inAppRewards.js @@ -1,9 +1,36 @@ import getItemInfo from './getItemInfo'; import shops from './shops'; import getOfficialPinnedItems from './getOfficialPinnedItems'; +import compactArray from 'lodash/compact'; import getItemByPathAndType from './getItemByPathAndType'; +/** + * Orders the pinned items so we always get our inAppRewards in the order + * which the user has saved + * + * @param user is the user + * @param items is the combined list of pinned items to sort + * @return items of ordered inAppRewards + */ +function sortInAppRewards (user, items) { + let pinnedItemsOrder = user.pinnedItemsOrder; + let orderedItems = []; + let unorderedItems = []; // what we want to add later + + items.forEach((item, index) => { + let i = pinnedItemsOrder[index] === item.path ? index : pinnedItemsOrder.indexOf(item.path); + if (i === -1) { + unorderedItems.push(item); + } else { + orderedItems[i] = item; + } + }); + orderedItems = compactArray(orderedItems); + orderedItems = unorderedItems.concat(orderedItems); + return orderedItems; +} + module.exports = function getPinnedItems (user) { let officialPinnedItems = getOfficialPinnedItems(user); @@ -22,5 +49,6 @@ module.exports = function getPinnedItems (user) { shops.checkMarketGearLocked(user, items); - return items; + let orderedItems = sortInAppRewards(user, items); + return orderedItems; }; diff --git a/website/server/controllers/api-v3/user.js b/website/server/controllers/api-v3/user.js index bf5009de87..389e27f708 100644 --- a/website/server/controllers/api-v3/user.js +++ b/website/server/controllers/api-v3/user.js @@ -1769,4 +1769,68 @@ api.togglePinnedItem = { }, }; +/** + * @api {post} /api/v3/user/move-pinned-item/:type/:path/move/to/:position Move a pinned item in the rewards column to a new position after being sorted + * @apiName MovePinnedItem + * @apiGroup User + * + * @apiParam (Path) {String} path The unique item path used for pinning + * @apiParam (Path) {Number} position Where to move the task. 0 = top of the list. -1 = bottom of the list. (-1 means push to bottom). First position is 0 + * + * @apiSuccess {Array} data The new pinned items order. + * + * @apiSuccessExample {json} + * {"success":true,"data":{"path":"quests.mayhemMistiflying3","type":"quests","_id": "5a32d357232feb3bc94c2bdf"},"notifications":[]} + * + * @apiUse TaskNotFound + */ +api.movePinnedItem = { + method: 'POST', + url: '/user/move-pinned-item/:path/move/to/:position', + middlewares: [authWithHeaders()], + async handler (req, res) { + req.checkParams('path', res.t('taskIdRequired')).notEmpty(); + req.checkParams('position', res.t('positionRequired')).notEmpty().isNumeric(); + + let validationErrors = req.validationErrors(); + if (validationErrors) throw validationErrors; + + let user = res.locals.user; + let path = req.params.path; + 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 + // Uses the current pinnedItemsOrder to return these in the right order, + // the new reset array will be in the right order before we do the swap + let currentPinnedItems = common.inAppRewards(user); + if (user.pinnedItemsOrder.length !== currentPinnedItems.length) { + user.pinnedItemsOrder = currentPinnedItems.map(item => item.path); + } + + // Adjust the order + let currentIndex = user.pinnedItemsOrder.findIndex(item => item === path); + let currentPinnedItemPath = user.pinnedItemsOrder[currentIndex]; + + if (currentIndex === -1) { + throw new BadRequest(res.t('wrongItemPath', req.language)); + } + + // 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); + } else { + user.pinnedItemsOrder.splice(position, 0, currentPinnedItemPath); + } + + await user.save(); + let userJson = user.toJSON(); + + res.respond(200, userJson.pinnedItemsOrder); + }, +}; + module.exports = api; diff --git a/website/server/models/user/schema.js b/website/server/models/user/schema.js index 35c61bea8b..98e4f7331d 100644 --- a/website/server/models/user/schema.js +++ b/website/server/models/user/schema.js @@ -588,6 +588,8 @@ let schema = new Schema({ path: {type: String}, type: {type: String}, }], + // Ordered array of shown pinned items, necessary for sorting because seasonal items are not stored in pinnedItems + pinnedItemsOrder: [{type: String}], // Items the user manually unpinned from the ones suggested by Habitica unpinnedItems: [{ _id: false,