From ca4bde55ba6d653b4b6e5c4b7f761d97de14d701 Mon Sep 17 00:00:00 2001 From: Bryan Davis Date: Mon, 18 Jan 2021 16:20:49 -0700 Subject: [PATCH] ui: Add a global notification component plugins/notify is a new locally developed vue + vuetify + vuex plugin implementing a global notification service. * displays notifications sent to the user. * Vue.$notify.{ success, info, warning, error } allow easy addition of messages from anywhere with access to the Vue object (often via `this` in a .vue component). * The 'notify' vuex module can be called directly to send notices from a vuex context. * Notifications stay on-screen until manually dismissed by default. * An optional timeout can be provided to automatically dismiss success and info notifications. This plugin is installed and the Notifications component has been added to the root App. Notifications are displayed using v-alert vutify components in a container attached to the bottom of the user's viewport. This commit also converts the AuditLogs view and its associated vuex store to use the new system rather than it's own error tracking and display code. Follow up commits should convert other views. Bug: T272185 Change-Id: I30e3aeb9bd7450d393ca48d7b511393f988a2cc8 --- .gitignore | 1 + package.json | 5 +- vue.config.js | 33 +++--- vue/src/App.vue | 3 +- vue/src/assets/locales/i18n/en.json | 2 + vue/src/assets/locales/i18n/qqq.json | 2 + vue/src/assets/styles/index.css | 25 +++++ vue/src/main.js | 3 + vue/src/plugins/notify/component.vue | 47 ++++++++ vue/src/plugins/notify/index.js | 89 +++++++++++++++ vue/src/plugins/notify/index.spec.js | 80 ++++++++++++++ vue/src/plugins/notify/vuex.js | 160 +++++++++++++++++++++++++++ vue/src/plugins/notify/vuex.spec.js | 126 +++++++++++++++++++++ vue/src/store/auditlogs.js | 12 +- vue/src/views/AuditLogs.vue | 15 +-- 15 files changed, 565 insertions(+), 38 deletions(-) create mode 100644 vue/src/plugins/notify/component.vue create mode 100644 vue/src/plugins/notify/index.js create mode 100644 vue/src/plugins/notify/index.spec.js create mode 100644 vue/src/plugins/notify/vuex.js create mode 100644 vue/src/plugins/notify/vuex.spec.js diff --git a/.gitignore b/.gitignore index f0413244..6e4b76d6 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ /docs/htmlcov /node_modules/ /vue/dist/ +/vue/dist-tests/ diff --git a/package.json b/package.json index a0dee377..0e22fa7d 100644 --- a/package.json +++ b/package.json @@ -9,13 +9,13 @@ "lint:banana": "banana-checker vue/src/assets/locales/i18n/", "lint:css-rtl": "bash -c \"if grep -rnHIE --color '\\b[mp][lr]-(n?[0-9][0-6]?|auto)\\b' vue/src; then echo 'ERROR: Use S and E helper classes, not L and R. (T269056)'; echo; exit 1; fi\"", "lint:eslint": "eslint .", - "lint:stylelint": "stylelint -f verbose \"**/*.{css,scss,sass,vue}\"", + "lint:stylelint": "stylelint -f verbose '**/*.{css,scss,sass,vue}'", "lint:vue": "vue-cli-service lint", "schemas:generate": "jsonschema-tools materialize-all", "serve:vue": "vue-cli-service serve", "test": "npm run test:jsonschema && npm run test:vue", "test:jsonschema": "mocha tests/jsonschema", - "test:vue": "nyc vue-cli-service test:unit --colors vue/src/**/*.spec.js" + "test:vue": "NODE_ENV=test nyc vue-cli-service test:unit --colors 'vue/src/**/*.spec.js'" }, "dependencies": { "@wikimedia/language-data": "^1.0.0", @@ -141,6 +141,7 @@ "rules": { "camelcase": "off", "no-undef": "off", + "no-underscore-dangle": [ "error", { "allowAfterThis": true } ], "vue/singleline-html-element-content-newline": "off", "vuetify/grid-unknown-attributes": "error", "vuetify/no-deprecated-classes": "error", diff --git a/vue.config.js b/vue.config.js index 51d0f192..a4b60767 100644 --- a/vue.config.js +++ b/vue.config.js @@ -3,6 +3,7 @@ const path = require( 'path' ); const BundleTracker = require( 'webpack-bundle-tracker' ); const isProduction = process.env.NODE_ENV === 'production'; +const isTest = process.env.NODE_ENV === 'test'; const PORT = process.env.PORT || 8001; const pages = { @@ -14,7 +15,11 @@ const pages = { } }; -const buildDir = 'vue/dist'; +// Keep bundles built for running test separate from bundles for the dev/prod +// server. Test bundles skip building somethings and do not split into the +// same chunks as dev/prod bundles. Why? Good question. Webpack gets angry is +// the best answer I have at the moment. :/ +const buildDir = isTest ? 'vue/dist-tests' : 'vue/dist'; module.exports = { pages: pages, @@ -35,20 +40,22 @@ module.exports = { devtool: isProduction ? false : 'cheap-source-map' }, chainWebpack: ( config ) => { - // Separate vendored js into its own bundle - config.optimization.splitChunks( - { - cacheGroups: { - vendor: { - test: /[\\/]node_modules[\\/]/, - name: 'chunk-vendors', - chunks: 'all', - priority: 1, - enforce: true + if ( !isTest ) { + // Separate vendored js into its own bundle + config.optimization.splitChunks( + { + cacheGroups: { + vendor: { + test: /[\\/]node_modules[\\/]/, + name: 'chunk-vendors', + chunks: 'all', + priority: 1, + enforce: true + } } } - } - ); + ); + } // We are not serving pages directly, so remove plugins that generate // stubs for pages and including js/css. diff --git a/vue/src/App.vue b/vue/src/App.vue index b6b58cd0..11f0f5c2 100644 --- a/vue/src/App.vue +++ b/vue/src/App.vue @@ -76,13 +76,12 @@ - + - diff --git a/vue/src/assets/locales/i18n/en.json b/vue/src/assets/locales/i18n/en.json index 7198bb7f..7da78846 100644 --- a/vue/src/assets/locales/i18n/en.json +++ b/vue/src/assets/locales/i18n/en.json @@ -18,6 +18,7 @@ "auditlog-summary": "$1 a $2", "auditlogs": "Audit logs", "auditlogs-pagetitle": "This page lists all log entries", + "auditlogs-apierror": "Error fetching page $1: $2", "authors": "Author(s)", "bugtracker": "Bug Tracker", "browsetool": "Browse tool", @@ -40,6 +41,7 @@ "locale-select": "Select language", "login": "Login", "logout": "Logout", + "message-close": "Close message", "moreinfo": "More information", "mostrecentcrawledurls": "Most recent crawled URL(s)", "my-account": "My Account", diff --git a/vue/src/assets/locales/i18n/qqq.json b/vue/src/assets/locales/i18n/qqq.json index e14c3f99..e4b06535 100644 --- a/vue/src/assets/locales/i18n/qqq.json +++ b/vue/src/assets/locales/i18n/qqq.json @@ -21,6 +21,7 @@ "auditlog-summary": "Audit log summary label. Parameters:\n* $1 - action taken\n* $2 - object action was taken on", "auditlogs": "Navigation menu label, links to Audit Logs page.", "auditlogs-pagetitle": "Page title.", + "auditlogs-apierror": "Error message shown when fetching auditlog information from the backend server fails. Parameters:\n* $1 - numeric page number\n* $2 - error message returned by API call (possibly not localized)", "authors": "Tool information label.", "bugtracker": "Link label, links to bug tracker for a tool.", "browsetool": "Button label, links to an external tool.", @@ -44,6 +45,7 @@ "login": "Button label.\n{{Identical:Login}}", "logout": "Button label.\n{{Identical|Logout}}", "moreinfo": "Section title.", + "message-close": "Label for button that closes a notiication message.", "mostrecentcrawledurls": "Table information label.", "my-account": "Alt text for icon.", "newtoolsfound": "Crawler run statistics label. Parameters:\n* $1 - number of tools found", diff --git a/vue/src/assets/styles/index.css b/vue/src/assets/styles/index.css index 2dcb8b1f..ace3ea10 100644 --- a/vue/src/assets/styles/index.css +++ b/vue/src/assets/styles/index.css @@ -56,3 +56,28 @@ div.home-tool-card { top: 0; right: 0; } + +#notifications { + /* z-index just below the level of our nav menu */ + z-index: 5; + position: fixed; + bottom: 0%; +} + +@media only screen and ( min-width: 960px ) { + .v-application--is-ltr #notifications { + margin-left: 28px; + left: 50%; + transform: translateX( -50% ); + } + + .v-application--is-rtl #notifications { + margin-right: 28px; + right: 50%; + transform: translateX( 50% ); + } +} + +#notifications .v-alert { + border: thin solid currentColor !important; +} diff --git a/vue/src/main.js b/vue/src/main.js index 9e15946f..003220d2 100644 --- a/vue/src/main.js +++ b/vue/src/main.js @@ -4,9 +4,12 @@ import router from './router'; import store from './store'; import vuetify from './plugins/vuetify'; import i18n from './plugins/i18n'; +import notify from './plugins/notify'; Vue.config.productionTip = false; +Vue.use( notify, { store: store } ); + new Vue( { vuetify, router, diff --git a/vue/src/plugins/notify/component.vue b/vue/src/plugins/notify/component.vue new file mode 100644 index 00000000..0cb549ab --- /dev/null +++ b/vue/src/plugins/notify/component.vue @@ -0,0 +1,47 @@ + + + diff --git a/vue/src/plugins/notify/index.js b/vue/src/plugins/notify/index.js new file mode 100644 index 00000000..54ba77f1 --- /dev/null +++ b/vue/src/plugins/notify/index.js @@ -0,0 +1,89 @@ +import module from './vuex'; +import Notifications from './component'; + +export let $store; + +export function setStore( store ) { + $store = store; +} + +export const methods = { + /** + * Signal a successful action. + * + * @param {string} message - Content + * @param {number} [timeout=0] - Time in milliseconds to display + * @return {number} Message id + */ + success( message, timeout = 0 ) { + return $store.dispatch( 'notify/success', { message, timeout } ); + }, + + /** + * Present information to the user. + * + * @param {string} message - Content + * @param {number} [timeout=0] - Time in milliseconds to display + * @return {number} Message id + */ + info( message, timeout = 0 ) { + return $store.dispatch( 'notify/info', { message, timeout } ); + }, + + /** + * Raise a warning. + * + * @param {string} msg - Warning + * @return {number} Message id + */ + warning( msg ) { + return $store.dispatch( 'notify/warning', msg ); + }, + + /** + * Signal an error. + * + * @param {string} msg - Error message + * @return {number} Message id + */ + error( msg ) { + return $store.dispatch( 'notify/error', msg ); + }, + + /** + * Remove a notification. + * + * @param {number} messageId - Message id + * @return {undefined} + */ + clear( messageId ) { + return $store.dispatch( 'notify/clearMessage', messageId ); + } +}; + +/** + * Install this plugin. + * + * @param {Object} Vue - Vue instance installing plugin + * @param {Object} options - Plugin options + * @param {Object} options.store - Vuex store + */ +export function install( Vue, options ) { + if ( install.installed ) { + return; + } + if ( !options.store ) { + throw new Error( 'Required options.store Vuex instance missing.' ); + } + + install.installed = true; + setStore( options.store ); + + options.store.registerModule( 'notify', module ); + Vue.component( 'Notifications', Notifications ); + Vue.prototype.$notify = methods; +} + +export default { + install +}; diff --git a/vue/src/plugins/notify/index.spec.js b/vue/src/plugins/notify/index.spec.js new file mode 100644 index 00000000..83218e52 --- /dev/null +++ b/vue/src/plugins/notify/index.spec.js @@ -0,0 +1,80 @@ +'use strict'; +import chai from 'chai'; +import sinon from 'sinon'; + +chai.use( require( 'sinon-chai' ) ); +const expect = chai.expect; +/* eslint-disable no-unused-expressions */ + +import { setStore, methods, install } from './index'; + +let $store; + +describe( 'notify/index', () => { + beforeEach( () => { + $store = { dispatch: sinon.spy() }; + setStore( $store ); + } ); + + describe( 'methods', () => { + it( 'success', () => { + methods.success( 'success message' ); + + expect( $store.dispatch ).to.have.been.calledOnce; + expect( $store.dispatch ).to.have.been.calledWithExactly( + 'notify/success', { message: 'success message', timeout: 0 } + ); + } ); + + it( 'info', () => { + methods.info( 'info message' ); + + expect( $store.dispatch ).to.have.been.calledOnce; + expect( $store.dispatch ).to.have.been.calledWithExactly( + 'notify/info', { message: 'info message', timeout: 0 } + ); + } ); + + it( 'warning', () => { + methods.warning( 'warning message' ); + + expect( $store.dispatch ).to.have.been.calledOnce; + expect( $store.dispatch ).to.have.been.calledWithExactly( + 'notify/warning', 'warning message' + ); + } ); + + it( 'error', () => { + methods.error( 'error message' ); + + expect( $store.dispatch ).to.have.been.calledOnce; + expect( $store.dispatch ).to.have.been.calledWithExactly( + 'notify/error', 'error message' + ); + } ); + + it( 'clear', () => { + methods.clear( 31337 ); + + expect( $store.dispatch ).to.have.been.calledOnce; + expect( $store.dispatch ).to.have.been.calledWithExactly( + 'notify/clearMessage', 31337 + ); + } ); + } ); + + describe( 'install', () => { + it( 'happy path', () => { + const vue = { component: sinon.spy(), prototype: sinon.spy() }; + const store = { registerModule: sinon.spy() }; + + install( vue, { store: store } ); + + expect( install.installed ).to.be.a( 'boolean' ).that.is.true; + expect( store.registerModule ).to.have.been.calledOnce; + expect( vue.component ).to.have.been.calledOnce; + expect( vue.prototype.$notify ).to.be.an( 'object' ); + } ); + } ); + +} ); diff --git a/vue/src/plugins/notify/vuex.js b/vue/src/plugins/notify/vuex.js new file mode 100644 index 00000000..871e4d76 --- /dev/null +++ b/vue/src/plugins/notify/vuex.js @@ -0,0 +1,160 @@ +let globalIdCounter = Date.now(); + +export const getters = {}; + +export const actions = { + /** + * Post a message. + * + * @param {Object} context - Vuex context + * @param {Object} payload + * @param {string|null} payload.message - content + * @param {string|null} payload.type - success, info, warning, error + * @param {boolean|null} payload.prominent - Draw more attention + * @param {number|null} payload.timeout - Dismiss after N milliseconds + * @return {number} Message id + */ + message( context, payload ) { + const mid = globalIdCounter++; + if ( payload.timeout ) { + payload.timeout = window.setTimeout( () => { + context.commit( 'onClearMessage', mid ); + }, Number( payload.timeout ) ); + } + context.commit( 'onMessage', { + id: mid, + message: payload.message, + type: payload.type, + prominent: payload.prominent, + timeoutID: payload.timeout || null + } ); + return mid; + }, + + /** + * Announce a successful action. + * + * @param {Object} context - Vuex context + * @param {Object} payload + * @param {string} payload.message - Content + * @param {number|null} payload.timeout - Time in milliseconds to display + * @return {number} Message id + */ + success( context, payload ) { + return context.dispatch( 'message', { + message: payload.message, + timeout: payload.timeout || null, + type: 'success' + } ); + }, + + /** + * Present information to the user. + * + * @param {Object} context - Vuex context + * @param {Object} payload + * @param {string} payload.message - Content + * @param {number|null} payload.timeout - Time in milliseconds to display + * @return {number} Message id + */ + info( context, payload ) { + return context.dispatch( 'message', { + message: payload.message, + timeout: payload.timeout || null, + type: 'info' + } ); + }, + + /** + * Raise a warning. + * + * @param {Object} context - Vuex context + * @param {string} msg - Warning + * @return {number} Message id + */ + warning( context, msg ) { + return context.dispatch( 'message', { + message: msg, + type: 'warning', + prominent: true + } ); + }, + + /** + * Signal an error. + * + * @param {Object} context - Vuex context + * @param {string} msg - Error message + * @return {number} Message id + */ + error( context, msg ) { + return context.dispatch( 'message', { + message: msg, + type: 'error', + prominent: true + } ); + }, + + /** + * Remove a message. + * + * @param {Object} context - Vuex context + * @param {number} messageId - Message id + */ + clearMessage( context, messageId ) { + context.commit( 'onClearMessage', messageId ); + } +}; + +export const mutations = { + /** + * Persist a message. + * + * @param {Object} state - Vuex state tree. + * @param {Object} payload - mutation payload. + * @param {number} payload.id - message id + * @param {string|null} payload.message - content + * @param {string|null} payload.type - success, info, warning, error + * @param {boolean|null} payload.prominent - Draw more attention + * @param {number|null} payload.timeoutID - Active timer id + */ + onMessage( state, payload ) { + if ( payload && payload.id ) { + state.messages.push( { + id: payload.id, + message: payload.message, + type: payload.type, + prominent: payload.prominent, + timeoutID: payload.timeoutID || false + } ); + } + }, + + /** + * Remove a message. + * + * @param {Object} state - Vuex state tree. + * @param {number} messageId - Message id + */ + onClearMessage( state, messageId ) { + if ( state.messages && state.messages.length ) { + state.messages = state.messages.filter( ( item ) => { + if ( item.id === messageId && item.timeoutID ) { + window.clearTimeout( item.timeoutID ); + } + return item.id !== messageId; + } ); + } + } +}; + +export default { + namespaced: true, + state: { + messages: [] + }, + getters: getters, + actions: actions, + mutations: mutations, + strict: process.env.NODE_ENV !== 'production' +}; diff --git a/vue/src/plugins/notify/vuex.spec.js b/vue/src/plugins/notify/vuex.spec.js new file mode 100644 index 00000000..d6498ae4 --- /dev/null +++ b/vue/src/plugins/notify/vuex.spec.js @@ -0,0 +1,126 @@ +'use strict'; +import chai from 'chai'; +import sinon from 'sinon'; + +chai.use( require( 'sinon-chai' ) ); +const expect = chai.expect; +/* eslint-disable no-unused-expressions */ + +import { actions, mutations } from './vuex'; + +describe( 'notify/vuex', () => { + describe( 'actions', () => { + it( 'message', () => { + const commit = sinon.spy(); + + const payload = { + message: 'test-message', + type: 'info', + prominent: false + }; + + const mid = actions.message( { commit }, payload ); + + expect( commit ).to.have.been.calledOnce; + expect( commit ).to.have.been.calledWithExactly( + 'onMessage', + // eslint-disable-next-line es/no-object-assign + Object.assign( payload, { id: mid, timeoutID: null } ) + ); + } ); + + it( 'message with timeout', () => { + const commit = sinon.spy(); + const expectTimeout = 12345; + const setTimeout = sinon.stub( window, 'setTimeout' ) + .returns( expectTimeout ); + + const payload = { + message: 'test-message', + type: 'info', + prominent: false, + timeout: 31337 + }; + + const mid = actions.message( { commit }, payload ); + + expect( setTimeout ).to.have.been.calledOnce; + expect( setTimeout ).to.have.been.calledWithExactly( + sinon.match.func, + 31337 + ); + expect( setTimeout ).to.have.been.calledBefore( commit ); + expect( commit ).to.have.been.calledOnce; + expect( commit ).to.have.been.calledWithExactly( + 'onMessage', { + id: mid, + message: payload.message, + type: payload.type, + prominent: payload.prominent, + timeoutID: expectTimeout + } + ); + } ); + + it( 'clearMessage', () => { + const commit = sinon.spy(); + + actions.clearMessage( { commit }, 31337 ); + + expect( commit ).to.have.been.calledOnce; + expect( commit ).to.have.been.calledWithExactly( + 'onClearMessage', 31337 + ); + } ); + } ); + + describe( 'mutations', () => { + it( 'onMessage', () => { + const state = { messages: [] }; + + const payload = { + id: 10100111001, + message: 'test-message', + type: 'success', + prominent: false, + timeoutID: false + }; + mutations.onMessage( state, payload ); + + expect( state.messages ).to.deep.equal( [ payload ] ); + } ); + + it( 'onClearMessage', () => { + const state = { messages: [ { id: 31337 } ] }; + + mutations.onClearMessage( state, 31337 ); + expect( state.messages ).to.be.an( 'array' ).that.is.empty; + } ); + + it( 'onClearMessage with unknown id', () => { + const state = { messages: [] }; + + mutations.onClearMessage( state, 1337 ); + expect( state.messages ).to.be.an( 'array' ).that.is.empty; + } ); + + it( 'onClearMessage with timeout', () => { + const clearTimeout = sinon.stub( window, 'clearTimeout' ); + + const state = { messages: [ + { id: 1, timeoutID: false }, + { id: 2, timeoutID: 1337 }, + { id: 3, timeoutID: false } + ] }; + + mutations.onClearMessage( state, 2 ); + + expect( clearTimeout ).to.have.been.calledOnce; + expect( clearTimeout ).to.have.been.calledWithExactly( 1337 ); + expect( state.messages ).to.be.an( 'array' ) + .that.has.lengthOf( 2 ) + .but.not.deep.include( { id: 2, timeoutID: 1337 } ); + } ); + } ); + +} ); diff --git a/vue/src/store/auditlogs.js b/vue/src/store/auditlogs.js index ff0553ee..18e2db88 100644 --- a/vue/src/store/auditlogs.js +++ b/vue/src/store/auditlogs.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import Vuex from 'vuex'; import SwaggerClient from 'swagger-client'; +import i18n from '@/plugins/i18n'; Vue.use( Vuex ); @@ -8,17 +9,12 @@ export default { namespaced: true, state: { auditLogs: [], - numLogs: 0, - apiErrorMsg: '' + numLogs: 0 }, mutations: { AUDIT_LOGS( state, logs ) { state.auditLogs = logs.results; state.numLogs = logs.count; - state.apiErrorMsg = ''; - }, - ERROR( state, error ) { - state.apiErrorMsg = error; } }, actions: { @@ -45,7 +41,9 @@ export default { const resp = await context.dispatch( 'makeApiCall', url ); if ( resp.error ) { - context.commit( 'ERROR', resp.error ); + this._vm.$notify.error( + i18n.t( 'auditlogs-apierror', [ page, resp.error ] ) + ); return; } diff --git a/vue/src/views/AuditLogs.vue b/vue/src/views/AuditLogs.vue index 08044d30..ad9418b5 100644 --- a/vue/src/views/AuditLogs.vue +++ b/vue/src/views/AuditLogs.vue @@ -14,19 +14,6 @@ - - - - {{ $t( 'apierror' ) }} {{ apiErrorMsg }} - - - -