From 08f1e2b2732096459ac336332284aaa976cee552 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Mon, 25 May 2020 17:02:29 +0200 Subject: [PATCH] Client: remove unnecessary API calls + members fixes (#12179) * wip * refactor world state * allow resource to be reloaded when the server is updated * fix #9242 * fix event listeners * remove un-needed code * add tests for asyncResourceFactory reloadOnAppVersionChange * fix double cron notifications and party members showing up in the header after a party invitation is accepted * remove console.log * do not send vm info to loggly due to circular dependency + fix typo * fix #12181 * do not load invites multiple times in members modal * add hover to challenge member count * groups: load members only on demand * minor ui fixes * choose class: fix vue duplicate key warning * minor ui fixes * challanges: load members on demand * add loading spinner * change loading mechanism * fix loading gryphon issues * reduce code duplication --- .eslintignore | 1 + test/client/.babelrc | 16 --- test/client/unit/index.js | 21 ---- test/client/unit/karma.conf.js | 40 -------- website/client/src/app.vue | 99 +++++++++---------- .../components/achievements/chooseClass.vue | 4 +- .../src/components/achievements/newStuff.vue | 2 +- .../components/challenges/challengeDetail.vue | 36 +++++-- .../src/components/chat/chatMessages.vue | 2 +- .../src/components/chat/copyAsTodoModal.vue | 2 +- .../src/components/chat/reportFlagModal.vue | 4 +- .../client/src/components/groups/group.vue | 71 +++++++------ .../src/components/groups/membersModal.vue | 37 +++---- .../src/components/groups/startQuestModal.vue | 2 +- .../client/src/components/header/index.vue | 5 +- .../components/header/notifications/base.vue | 8 +- .../header/notifications/worldBoss.vue | 22 +++-- .../inventory/stable/hatchedPetDialog.vue | 2 +- .../inventory/stable/mountRaisedModal.vue | 2 +- .../members/memberSearchDropdown.vue | 11 +++ .../src/components/messages/messageList.vue | 2 - .../client/src/components/notifications.vue | 6 +- .../src/components/payments/amazonModal.vue | 2 +- .../payments/cancelModalConfirm.vue | 2 +- .../src/components/payments/canceledModal.vue | 2 +- .../src/components/payments/successModal.vue | 2 +- .../src/components/shops/market/index.vue | 12 +-- .../src/components/shops/market/sellModal.vue | 2 +- .../src/components/shops/quests/index.vue | 11 +-- .../src/components/shops/seasonal/index.vue | 14 ++- .../components/shops/timeTravelers/index.vue | 2 +- .../src/components/tasks/brokenTaskModal.vue | 2 +- .../client/src/components/tasks/column.vue | 2 +- .../client/src/components/tasks/taskModal.vue | 2 +- .../src/components/ui/customMenuDropdown.vue | 2 +- .../src/components/ui/loadingGryphon.vue | 45 +++++++++ .../src/components/userMenu/profileModal.vue | 2 +- website/client/src/libs/asyncResource.js | 23 ++++- website/client/src/libs/logging.js | 1 - website/client/src/pages/private-messages.vue | 3 +- website/client/src/store/actions/chat.js | 10 +- website/client/src/store/actions/guilds.js | 3 +- website/client/src/store/actions/index.js | 2 +- .../client/src/store/actions/notifications.js | 2 + .../client/src/store/actions/world-state.js | 7 -- .../client/src/store/actions/worldState.js | 14 +++ website/client/src/store/getters/index.js | 2 + .../client/src/store/getters/worldState.js | 23 +++++ website/client/src/store/index.js | 14 ++- .../tests/unit/libs/asyncResource.spec.js | 50 ++++++++++ 50 files changed, 394 insertions(+), 259 deletions(-) delete mode 100644 test/client/.babelrc delete mode 100644 test/client/unit/index.js delete mode 100644 test/client/unit/karma.conf.js create mode 100644 website/client/src/components/ui/loadingGryphon.vue delete mode 100644 website/client/src/store/actions/world-state.js create mode 100644 website/client/src/store/actions/worldState.js create mode 100644 website/client/src/store/getters/worldState.js diff --git a/.eslintignore b/.eslintignore index 2569aef9fe..2c1e5c7a43 100644 --- a/.eslintignore +++ b/.eslintignore @@ -10,6 +10,7 @@ dist/ dist-client/ apidoc_build/ content_cache/ +i18n_cache/ node_modules/ # Old migrations, disabled diff --git a/test/client/.babelrc b/test/client/.babelrc deleted file mode 100644 index 5fd9045a15..0000000000 --- a/test/client/.babelrc +++ /dev/null @@ -1,16 +0,0 @@ -{ - "env": { - "test": { - plugins: [ - ["istanbul"], - ], - }, - }, - "presets": [ - ["es2015", { modules: false }], - ], - "plugins": [ - "transform-object-rest-spread", - ], - "comments": false, -} \ No newline at end of file diff --git a/test/client/unit/index.js b/test/client/unit/index.js deleted file mode 100644 index 9a3f4b6f9a..0000000000 --- a/test/client/unit/index.js +++ /dev/null @@ -1,21 +0,0 @@ -/* eslint-disable */ -// TODO verify if it's needed, added because Axios require Promise in the global scope -// and babel-runtime doesn't affect external libraries -require('babel-polyfill'); - -// Automatically setup SinonJS' sandbox for each test -beforeEach(() => { - global.sandbox = sinon.createSandbox(); -}); - -afterEach(() => { - global.sandbox.restore(); -}); - -// require all test files -const testsContext = require.context('./specs', true, /\.js$/); -testsContext.keys().forEach(testsContext); - -// require all .vue and .js files except main.js for coverage. -const srcContext = require.context('../../../website/client', true, /^\.\/(?=(?!main(\.js)?$))(?=(.*\.(vue|js)$))/); -srcContext.keys().forEach(srcContext); diff --git a/test/client/unit/karma.conf.js b/test/client/unit/karma.conf.js deleted file mode 100644 index b12c5aac67..0000000000 --- a/test/client/unit/karma.conf.js +++ /dev/null @@ -1,40 +0,0 @@ -/* eslint-disable */ -// This is a karma config file. For more details see -// http://karma-runner.github.io/0.13/config/configuration-file.html -// we are also using it with karma-webpack -// https://github.com/webpack/karma-webpack - -// Necessary for babel to respect the env version of .babelrc which is necessary -// Because inject-loader does not work with ["es2015", { modules: false }] that we use -// in order to let webpack2 handle the imports -process.env.CHROME_BIN = require('puppeteer').executablePath(); -// eslint-disable-line no-process-env -process.env.BABEL_ENV = process.env.NODE_ENV; // eslint-disable-line no-process-env -const webpackConfig = require('../../../webpack/webpack.test.conf'); - -module.exports = function (config) { - config.set({ - // to run in additional browsers: - // 1. install corresponding karma launcher - // http://karma-runner.github.io/0.13/config/browsers.html - // 2. add it to the `browsers` array below. - browsers: ['ChromeHeadless'], - frameworks: ['mocha', 'sinon-stub-promise', 'sinon-chai', 'chai-as-promised', 'chai'], - reporters: ['spec', 'coverage'], - files: ['./index.js'], - preprocessors: { - './index.js': ['webpack', 'sourcemap'], - }, - webpack: webpackConfig, - webpackMiddleware: { - noInfo: true, - }, - coverageReporter: { - dir: '../../../coverage/client-unit', - reporters: [ - { type: 'lcov', subdir: '.' }, - { type: 'text-summary' }, - ], - }, - }); -}; diff --git a/website/client/src/app.vue b/website/client/src/app.vue index 0f9fb26081..6b5f7db865 100644 --- a/website/client/src/app.vue +++ b/website/client/src/app.vue @@ -297,7 +297,7 @@ export default { }; }, computed: { - ...mapState(['isUserLoggedIn', 'browserTimezoneOffset', 'isUserLoaded']), + ...mapState(['isUserLoggedIn', 'browserTimezoneOffset', 'isUserLoaded', 'notificationsRemoved']), ...mapState({ user: 'user.data' }), isStaticPage () { return this.$route.meta.requiresLogin === false; @@ -361,13 +361,55 @@ export default { showSpinner: false, }); - // Set up Error interceptors - axios.interceptors.response.use(response => { - if (this.user && response.data && response.data.notifications) { - this.$set(this.user, 'notifications', response.data.notifications); + axios.interceptors.response.use(response => { // Set up Response interceptors + // Verify that the user was not updated from another browser/app/client + // If it was, sync + const { url } = response.config; + const { method } = response.config; + + const isApiCall = url.indexOf('api/v4') !== -1; + const userV = response.data && response.data.userV; + const isCron = url.indexOf('/api/v4/cron') === 0 && method === 'post'; + + if (this.isUserLoaded && isApiCall && userV) { + const oldUserV = this.user._v; + this.user._v = userV; + + // Do not sync again if already syncing + const isUserSync = url.indexOf('/api/v4/user') === 0 && method === 'get'; + const isTasksSync = url.indexOf('/api/v4/tasks/user') === 0 && method === 'get'; + // exclude chat seen requests because with real time chat they would be too many + const isChatSeen = url.indexOf('/chat/seen') !== -1 && method === 'post'; + // exclude POST /api/v4/cron because the user is synced automatically after cron runs + + // Something has changed on the user object that was not tracked here, sync the user + if (userV - oldUserV > 1 && !isCron && !isChatSeen && !isUserSync && !isTasksSync) { + Promise.all([ + this.$store.dispatch('user:fetch', { forceLoad: true }), + this.$store.dispatch('tasks:fetchUserTasks', { forceLoad: true }), + ]); + } } + + // Store the app version from the server + const serverAppVersion = response.data && response.data.appVersion; + + if (serverAppVersion && this.$store.state.serverAppVersion !== response.data.appVersion) { + this.$store.state.serverAppVersion = serverAppVersion; + } + + // Store the notifications, filtering those that have already been read + // See store/index.js on why this is necessary + if (this.user && response.data && response.data.notifications) { + const filteredNotifications = response.data.notifications.filter(serverNotification => { + if (this.notificationsRemoved.includes(serverNotification.id)) return false; + return true; + }); + this.$set(this.user, 'notifications', filteredNotifications); + } + return response; - }, error => { + }, error => { // Set up Error interceptors if (error.response.status >= 400) { const isBanned = this.checkForBannedUser(error); if (isBanned === true) return null; // eslint-disable-line consistent-return @@ -425,51 +467,6 @@ export default { return Promise.reject(error); }); - axios.interceptors.response.use(response => { - // Verify that the user was not updated from another browser/app/client - // If it was, sync - const { url } = response.config; - const { method } = response.config; - - const isApiCall = url.indexOf('api/v4') !== -1; - const userV = response.data && response.data.userV; - const isCron = url.indexOf('/api/v4/cron') === 0 && method === 'post'; - - if (this.isUserLoaded && isApiCall && userV) { - const oldUserV = this.user._v; - this.user._v = userV; - - // Do not sync again if already syncing - const isUserSync = url.indexOf('/api/v4/user') === 0 && method === 'get'; - const isTasksSync = url.indexOf('/api/v4/tasks/user') === 0 && method === 'get'; - // exclude chat seen requests because with real time chat they would be too many - const isChatSeen = url.indexOf('/chat/seen') !== -1 && method === 'post'; - // exclude POST /api/v4/cron because the user is synced automatically after cron runs - - // Something has changed on the user object that was not tracked here, sync the user - if (userV - oldUserV > 1 && !isCron && !isChatSeen && !isUserSync && !isTasksSync) { - Promise.all([ - this.$store.dispatch('user:fetch', { forceLoad: true }), - this.$store.dispatch('tasks:fetchUserTasks', { forceLoad: true }), - ]); - } - } - - // Verify the client is updated - // const serverAppVersion = response.data.appVersion; - // let serverAppVersionState = this.$store.state.serverAppVersion; - // if (isApiCall && !serverAppVersionState) { - // this.$store.state.serverAppVersion = serverAppVersion; - // } else if (isApiCall && serverAppVersionState !== serverAppVersion) { - // if (document.activeElement.tagName !== 'INPUT' - // || confirm(this.$t('habiticaHasUpdated'))) { - // location.reload(true); - // } - // } - - return response; - }); - // Setup listener for title this.$store.watch(state => state.title, title => { document.title = title; diff --git a/website/client/src/components/achievements/chooseClass.vue b/website/client/src/components/achievements/chooseClass.vue index 913abb1b1d..5597f4328f 100644 --- a/website/client/src/components/achievements/chooseClass.vue +++ b/website/client/src/components/achievements/chooseClass.vue @@ -16,7 +16,7 @@
@@ -60,7 +60,7 @@
@@ -279,6 +280,10 @@ font-size: 20px; vertical-align: bottom; + &.member-count:hover { + cursor: pointer; + } + .svg-icon { width: 30px; display: inline-block; @@ -364,6 +369,7 @@ export default { }), challenge: {}, members: [], + membersLoaded: false, tasksByType: { habit: [], daily: [], @@ -427,8 +433,6 @@ export default { this.$router.push('/challenges/findChallenges'); return; } - this.members = await this - .loadMembers({ challengeId: this.searchId, includeAllPublicFields: true }); const tasks = await this.$store.dispatch('tasks:getChallengeTasks', { challengeId: this.searchId }); this.tasksByType = { habit: [], @@ -454,7 +458,22 @@ export default { } return this.$store.dispatch('members:getChallengeMembers', payload); }, + initialMembersLoad () { + this.$store.state.memberModalOptions.loading = true; + if (!this.membersLoaded) { + this.membersLoaded = true; + this.loadMembers({ + challengeId: this.searchId, + includeAllPublicFields: true, + }).then(m => { + this.members.push(...m); + this.$store.state.memberModalOptions.loading = false; + }); + } else { + this.$store.state.memberModalOptions.loading = false; + } + }, editTask (task) { this.taskFormPurpose = 'edit'; this.editingTask = cloneDeep(task); @@ -489,6 +508,8 @@ export default { this.tasksByType[task.type].splice(index, 1); }, showMemberModal () { + this.initialMembersLoad(); + this.$root.$emit('habitica:show-member-modal', { challengeId: this.challenge._id, groupId: 'challenge', // @TODO: change these terrible settings @@ -501,8 +522,8 @@ export default { async joinChallenge () { this.user.challenges.push(this.searchId); this.challenge = await this.$store.dispatch('challenges:joinChallenge', { challengeId: this.searchId }); - this.members = await this - .loadMembers({ challengeId: this.searchId, includeAllPublicFields: true }); + this.membersLoaded = false; + this.members = []; await this.$store.dispatch('tasks:fetchUserTasks', { forceLoad: true }); }, @@ -511,10 +532,11 @@ export default { }, async updateChallenge () { this.challenge = await this.$store.dispatch('challenges:getChallenge', { challengeId: this.searchId }); - this.members = await this - .loadMembers({ challengeId: this.searchId, includeAllPublicFields: true }); + this.membersLoaded = false; + this.members = []; }, closeChallenge () { + this.initialMembersLoad(); this.$root.$emit('bv::show::modal', 'close-challenge-modal'); }, edit () { diff --git a/website/client/src/components/chat/chatMessages.vue b/website/client/src/components/chat/chatMessages.vue index a1fe82ef72..5b7ee3b8e0 100644 --- a/website/client/src/components/chat/chatMessages.vue +++ b/website/client/src/components/chat/chatMessages.vue @@ -223,7 +223,7 @@ export default { created () { window.addEventListener('scroll', this.handleScroll); }, - destroyed () { + beforeDestroy () { window.removeEventListener('scroll', this.handleScroll); }, methods: { diff --git a/website/client/src/components/chat/copyAsTodoModal.vue b/website/client/src/components/chat/copyAsTodoModal.vue index 42a7f2d1af..89ba5e1699 100644 --- a/website/client/src/components/chat/copyAsTodoModal.vue +++ b/website/client/src/components/chat/copyAsTodoModal.vue @@ -81,7 +81,7 @@ export default { this.$root.$emit('bv::show::modal', 'copyAsTodo'); }); }, - destroyed () { + beforeDestroy () { this.$root.$off('habitica::copy-as-todo'); }, methods: { diff --git a/website/client/src/components/chat/reportFlagModal.vue b/website/client/src/components/chat/reportFlagModal.vue index e59c769e4d..eac5ea738b 100644 --- a/website/client/src/components/chat/reportFlagModal.vue +++ b/website/client/src/components/chat/reportFlagModal.vue @@ -122,10 +122,10 @@ export default { }; }, }, - created () { + mounted () { this.$root.$on('habitica::report-chat', this.handleReport); }, - destroyed () { + beforeDestroy () { this.$root.$off('habitica::report-chat', this.handleReport); }, methods: { diff --git a/website/client/src/components/groups/group.vue b/website/client/src/components/groups/group.vue index 503b120629..6f58e1f4b1 100644 --- a/website/client/src/components/groups/group.vue +++ b/website/client/src/components/groups/group.vue @@ -385,7 +385,7 @@ import extend from 'lodash/extend'; import groupUtilities from '@/mixins/groupsUtilities'; import styleHelper from '@/mixins/styleHelper'; -import { mapState } from '@/libs/store'; +import { mapState, mapGetters } from '@/libs/store'; import * as Analytics from '@/libs/analytics'; import startQuestModal from './startQuestModal'; import questDetailsModal from './questDetailsModal'; @@ -447,6 +447,7 @@ export default { bronzeGuildBadgeIcon, }), members: [], + membersLoaded: false, selectedQuest: {}, chat: { submitDisable: false, @@ -455,7 +456,12 @@ export default { }; }, computed: { - ...mapState({ user: 'user.data' }), + ...mapState({ + user: 'user.data', + }), + ...mapGetters({ + partyMembers: 'party:members', + }), partyStore () { return this.$store.state.party; }, @@ -487,10 +493,15 @@ export default { } }, }, - mounted () { + async mounted () { if (this.isParty) this.searchId = 'party'; if (!this.searchId) this.searchId = this.groupId; - this.load(); + await this.fetchGuild(); + + this.$root.$on('updatedGroup', this.onGroupUpdate); + }, + beforeDestroy () { + this.$root.$off('updatedGroup', this.onGroupUpdate); }, beforeRouteUpdate (to, from, next) { this.$set(this, 'searchId', to.params.groupId); @@ -501,19 +512,9 @@ export default { acceptCommunityGuidelines () { this.$store.dispatch('user:set', { 'flags.communityGuidelinesAccepted': true }); }, - async load () { - if (this.isParty) { - this.searchId = 'party'; - // @TODO: Set up from old client. Decide what we need and what we don't - // Check Desktop notifs - // Load invites - } - await this.fetchGuild(); - - this.$root.$on('updatedGroup', group => { - const updatedGroup = extend(this.group, group); - this.$set(this.group, updatedGroup); - }); + onGroupUpdate (group) { + const updatedGroup = extend(this.group, group); + this.$set(this.group, updatedGroup); }, /** @@ -531,6 +532,26 @@ export default { return this.$store.dispatch('members:getGroupMembers', payload); }, showMemberModal () { + this.$store.state.memberModalOptions.loading = true; + + if (this.isParty) { + this.membersLoaded = true; + this.members = this.partyMembers; + this.$store.state.memberModalOptions.loading = false; + } else if (!this.membersLoaded) { + this.membersLoaded = true; + + this.loadMembers({ + groupId: this.group._id, + includeAllPublicFields: true, + }).then(m => { + this.members.push(...m); + this.$store.state.memberModalOptions.loading = false; + }); + } else { + this.$store.state.memberModalOptions.loading = false; + } + this.$root.$emit('habitica:show-member-modal', { groupId: this.group._id, group: this.group, @@ -565,19 +586,13 @@ export default { const groupId = this.searchId === 'party' ? this.user.party._id : this.searchId; if (this.hasUnreadMessages(groupId)) { - // Delay by 1sec to make sure it returns after - // other requests that don't have the notification marked as read - setTimeout(() => { - this.$store.dispatch('chat:markChatSeen', { groupId }); - this.$delete(this.user.newMessages, groupId); - }, 1000); + const notification = this.user + .notifications.find(n => n.type === 'NEW_CHAT_MESSAGE' && n.data.group.id === groupId); + const notificationId = notification && notification.id; + this.$store.dispatch('chat:markChatSeen', { groupId, notificationId }); } - - this.members = await this.loadMembers({ - groupId: this.group._id, - includeAllPublicFields: true, - }); }, + // returns the notification id or false hasUnreadMessages (groupId) { if (this.user.newMessages[groupId]) return true; diff --git a/website/client/src/components/groups/membersModal.vue b/website/client/src/components/groups/membersModal.vue index f02e9494c0..e6be50bf55 100644 --- a/website/client/src/components/groups/membersModal.vue +++ b/website/client/src/components/groups/membersModal.vue @@ -99,14 +99,21 @@
-
+ +
- +
@@ -201,7 +208,7 @@ class="row gradient" >
-
+
this.$store.dispatch('members:getGroupMembers', p), }); }, setPartyMembersWidth ($event) { diff --git a/website/client/src/components/header/notifications/base.vue b/website/client/src/components/header/notifications/base.vue index 40af37b2ee..e1e79896c7 100644 --- a/website/client/src/components/header/notifications/base.vue +++ b/website/client/src/components/header/notifications/base.vue @@ -182,10 +182,10 @@ export default { remove () { if (this.notification.type === 'NEW_CHAT_MESSAGE') { const groupId = this.notification.data.group.id; - this.$store.dispatch('chat:markChatSeen', { groupId }); - if (this.user.newMessages[groupId]) { - this.$delete(this.user.newMessages, groupId); - } + this.$store.dispatch('chat:markChatSeen', { + groupId, + notificationId: this.notification.id, + }); } else { this.readNotification({ notificationId: this.notification.id }); } diff --git a/website/client/src/components/header/notifications/worldBoss.vue b/website/client/src/components/header/notifications/worldBoss.vue index 48e3876173..f3be771fbd 100644 --- a/website/client/src/components/header/notifications/worldBoss.vue +++ b/website/client/src/components/header/notifications/worldBoss.vue @@ -1,6 +1,6 @@