Skip to content

Commit

Permalink
Remove duplicate snackbar for failed mutation & tidy Alert component
Browse files Browse the repository at this point in the history
MetRonnie committed Jan 19, 2023

Verified

This commit was signed with the committer’s verified signature. The key has expired.
pganssle Paul Ganssle
1 parent 03ab518 commit b9fe3a8
Showing 17 changed files with 84 additions and 113 deletions.
45 changes: 18 additions & 27 deletions src/components/core/Alert.vue
Original file line number Diff line number Diff line change
@@ -18,21 +18,20 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
<template>
<v-snackbar
v-if="alert"
v-model="snackbar"
v-model="alert"
:color="getColor(alert.color)"
:top="true"
top
timeout="-1"
:icon="alert.icon"
data-cy="alert-snack"
>

<template v-slot:action="{ attrs }">
<v-btn
color="black"
text
icon
v-bind="attrs"
@click="closeAlert"
data-cy="snack-close"
>
<v-icon>{{ svgPaths.close }}</v-icon>
<v-icon>{{ $options.icons.mdiClose }}</v-icon>
</v-btn>
</template>
{{ alert.text }}
@@ -43,43 +42,35 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
import { mdiClose } from '@mdi/js'
import { mapActions, mapState } from 'vuex'

// TODO: remove later when https://github.com/vuetifyjs/vuetify/issues/11021 is fixed
const colors = new Map([
['error', 'red'],
['success', 'green'],
['warning', 'amber']
])

export default {
name: 'Alert',

data () {
return {
// TODO: remove later when https://github.com/vuetifyjs/vuetify/issues/11021 is fixed
colors: new Map([
['error', 'red'],
['success', 'green'],
['warning', 'amber']
]),
svgPaths: {
close: mdiClose
},
snackbar: alert
}
},

computed: {
...mapState(['alert'])
},

methods: {
...mapActions(['setAlert']),
getColor (type) {
return this.colors.get(type) || ''
return colors.get(type) || ''
},
/**
* Dismisses the alert from the UI, also removing it from the Vuex store.
*
* @param {Function} toggleFunction - the original Vuetify toggle function
* @see https://vuetifyjs.com/en/api/v-alert/
*/
closeAlert () {
this.snackbar = false
this.setAlert(null)
}
},

icons: {
mdiClose
}
}
</script>
52 changes: 18 additions & 34 deletions src/components/cylc/Mutation.vue
Original file line number Diff line number Diff line change
@@ -116,17 +116,19 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
</v-tooltip>
</v-card-actions>
<v-snackbar
v-model="showSnackbar"
v-bind="snackbarProps"
data-cy="response-snackbar"
v-model="showWarning"
timeout="4e3"
color="amber accent-2"
light
data-cy="warning-snack"
>
{{ response.msg }}
{{ warningMsg }}
<template v-slot:action="{ attrs }">
<v-btn
@click="showSnackbar = false"
@click="showWarning = false"
icon
v-bind="attrs"
data-cy="snackbar-close"
data-cy="snack-close"
>
<v-icon>
{{ $options.icons.close }}
@@ -143,7 +145,8 @@ import EditRuntimeForm from '@/components/graphqlFormGenerator/EditRuntimeForm.v
import Markdown from '@/components/Markdown'
import {
getMutationShortDesc,
getMutationExtendedDesc
getMutationExtendedDesc,
mutationStatus
} from '@/utils/aotf'
import { mdiClose } from '@mdi/js'

@@ -186,10 +189,7 @@ export default {
data: () => ({
isValid: false,
submitting: false,
response: {
msg: null,
level: 'warn'
}
warningMsg: null
}),

computed: {
@@ -201,26 +201,13 @@ export default {
extendedDescription () {
return getMutationExtendedDesc(this.mutation.description)
},
showSnackbar: {
showWarning: {
get () {
return Boolean(this.response.msg)
return Boolean(this.warningMsg)
},
set (val) {
if (!val) this.response.msg = null
if (!val) this.warningMsg = null
}
},
snackbarProps () {
return this.response.level === 'error'
? {
timeout: -1,
color: 'red accent-2',
dark: true
}
: {
timeout: 4e3,
color: 'amber accent-2',
light: true
}
}
},

@@ -230,16 +217,13 @@ export default {
this.submitting = true
this.$refs.form.submit().then(response => {
this.submitting = false
if (response.status.name.includes('failed')) {
this.response.msg = response.message
this.response.level = 'error'
} else if (response.status.name === 'warn') {
this.response.msg = response.message
this.response.level = 'warn'
} else {
if (response.status === mutationStatus.SUCCEEDED) {
// Close the form on success
this.cancel()
} else if (response.status === mutationStatus.WARN) {
this.warningMsg = response.message
}
// else if error, an alert is generated by AOTF
})
}
},
6 changes: 2 additions & 4 deletions src/components/graphqlFormGenerator/EditRuntimeForm.vue
Original file line number Diff line number Diff line change
@@ -62,7 +62,7 @@ import { mdiHelpCircleOutline } from '@mdi/js'
import { cloneDeep, isArray, isEqual, snakeCase, startCase } from 'lodash'
import { VTextarea } from 'vuetify/lib/components'
import VuetifyConfig, { getComponentProps, RUNTIME_SETTING } from '@/components/graphqlFormGenerator/components/vuetify'
import { findByName, mutate } from '@/utils/aotf'
import { findByName, mutate, mutationStatus } from '@/utils/aotf'

export default {
name: 'EditRuntimeForm',
@@ -161,9 +161,7 @@ export default {
if (!settings.length) {
return {
message: 'No changes were made',
status: {
name: 'warn'
}
status: mutationStatus.WARN
}
}
const args = {
2 changes: 1 addition & 1 deletion src/graphql/index.js
Original file line number Diff line number Diff line change
@@ -97,7 +97,7 @@ export function createSubscriptionClient (wsUrl, options = {}, wsImpl = null) {
// would be nice to find a better error message using the error object
// subscriptionClient.onError((error) => {
// console.error(error)
// store.commit('SET_ALERT', new Alert(error, null, 'error'))
// store.commit('SET_ALERT', new Alert(error, 'error'))
// })
return subscriptionClient
}
2 changes: 1 addition & 1 deletion src/layouts/Default.vue
Original file line number Diff line number Diff line change
@@ -87,7 +87,7 @@ export default {

errorCaptured (error, vm, info) {
if (process.env.NODE_ENV !== 'production') {
store.dispatch('setAlert', new AlertModel(error, null, 'error'))
store.dispatch('setAlert', new AlertModel(error, 'error'))
}
}
}
3 changes: 1 addition & 2 deletions src/model/Alert.model.js
Original file line number Diff line number Diff line change
@@ -16,9 +16,8 @@
*/

export default class Alert {
constructor (text, icon, color) {
constructor (text, color) {
this.text = text
this.icon = icon
this.color = color
}
}
2 changes: 1 addition & 1 deletion src/model/Subscription.model.js
Original file line number Diff line number Diff line change
@@ -74,7 +74,7 @@ class Subscription {
} else {
Object.values(this.subscribers).forEach(function (subscriber) {
subscriber.viewState = viewState
subscriber.setAlert(new Alert(context.message, null, 'error'))
subscriber.setAlert(new Alert(context.message, 'error'))
if (_this.debug) {
// eslint-disable-next-line no-console
console.debug(`Subscription error: ${context.message}`, viewState, context)
2 changes: 1 addition & 1 deletion src/router/index.js
Original file line number Diff line number Diff line change
@@ -96,7 +96,7 @@ router.beforeEach((to, from, next) => {
next()
})
.catch(error => {
const alert = new Alert(error, null, 'error')
const alert = new Alert(error, 'error')
return store.dispatch('setAlert', alert)
})
} else {
4 changes: 2 additions & 2 deletions src/services/workflow.service.js
Original file line number Diff line number Diff line change
@@ -227,7 +227,7 @@ class WorkflowService {
if (callback.init) {
callback.init(store, errors)
for (const error of errors) {
store.commit('SET_ALERT', new Alert(error[0], null, 'error'), { root: true })
store.commit('SET_ALERT', new Alert(error[0], 'error'), { root: true })
// eslint-disable-next-line no-console
console.warn(...error)
subscription.handleViewState(ViewState.ERROR, error('Error presetting view state'))
@@ -340,7 +340,7 @@ class WorkflowService {
for (const error of errors) {
store.commit(
'SET_ALERT',
new Alert(error[0], null, 'error'),
new Alert(error[0], 'error'),
{ root: true }
)
// eslint-disable-next-line no-console
47 changes: 24 additions & 23 deletions src/utils/aotf.js
Original file line number Diff line number Diff line change
@@ -47,8 +47,7 @@ import {
mdiStop
} from '@mdi/js'

import AlertModel from '@/model/Alert.model'
import TaskState from '@/model/TaskState.model'
import Alert from '@/model/Alert.model'
import store from '@/store/index'
import { Tokens } from '@/utils/uid'

@@ -108,7 +107,7 @@ import { IntrospectionInputType } from 'graphql'

/**
* @typedef {Object} MutationResponse
* @property {TaskState} status
* @property {string} status
* @property {string} message
*/

@@ -280,11 +279,9 @@ export const alternateFields = {
* Maps onto task status.
*/
export const mutationStatus = Object.freeze({
[TaskState.WAITING]: TaskState.WAITING,
[TaskState.SUBMITTED]: TaskState.SUBMITTED,
[TaskState.SUCCEEDED]: TaskState.SUCCEEDED,
[TaskState.FAILED]: TaskState.FAILED,
[TaskState.SUBMIT_FAILED]: TaskState.SUBMIT_FAILED
FAILED: 'FAILED',
SUCCEEDED: 'SUCCEEDED',
WARN: 'WARN'
})

/**
@@ -831,6 +828,17 @@ export function getMutationArgsFromTokens (mutation, tokens) {
return argspec
}

/**
* @param {string} message
* @returns {MutationResponse}
*/
function _mutateSuccess (message) {
return {
status: mutationStatus.SUCCEEDED,
message
}
}

/**
* Handle an error in a called mutation.
*
@@ -848,15 +856,14 @@ async function _mutateError (mutationName, message, response) {
}

// open a user alert
await store.dispatch('setAlert', new AlertModel(
`command failed: ${mutationName} - ${message}`,
null,
'error')
await store.dispatch(
'setAlert',
new Alert(`Command failed: ${mutationName} - ${message}`, 'error')
)

// format a response
return {
status: TaskState.SUBMIT_FAILED,
status: mutationStatus.FAILED,
message
}
}
@@ -869,7 +876,7 @@ async function _mutateError (mutationName, message, response) {
* @param {ApolloClient} apolloClient
* @param {string=} cylcID
*
* @returns {Promise<MutationResponse>} {status, msg}
* @returns {(MutationResponse | Promise<MutationResponse>)} {status, msg}
*/
export async function mutate (mutation, variables, apolloClient, cylcID) {
const mutationStr = constructMutation(mutation)
@@ -898,24 +905,18 @@ export async function mutate (mutation, variables, apolloClient, cylcID) {
}

try {
const result = response.data[mutation.name].result
const { result } = response.data[mutation.name]
if (Array.isArray(result) && result.length === 2) {
// regular [commandSucceeded, message] format
if (result[0] === true) {
// success
return {
status: TaskState.SUBMITTED,
message: result[1]
}
return _mutateSuccess(result[1])
}
// failure (Cylc error, e.g. could not find workflow <x>)
return _mutateError(mutation.name, result[1], response)
}
// command in a different format (e.g. info command)
return {
status: TaskState.SUBMITTED,
message: result
}
return _mutateSuccess(result)
} catch (error) {
return _mutateError(mutation.name, 'invalid response', response)
}
10 changes: 5 additions & 5 deletions tests/e2e/specs/alert.cy.js
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ describe('Alert component', () => {
cy.get('.c-header').should('exist')
const errorMessage = 'Error displayed'
getStore().then(store => {
store.dispatch('setAlert', new Alert(errorMessage, null, 'error'))
store.dispatch('setAlert', new Alert(errorMessage, 'error'))
})
cy
.get('.v-snack')
@@ -38,7 +38,7 @@ describe('Alert component', () => {
cy.visit('/#/')
cy.get('.c-header').should('exist')
getStore().then(store => {
cy.wrap(store).invoke('commit', 'SET_ALERT', new Alert('An alert', null, 'success'))
cy.wrap(store).invoke('commit', 'SET_ALERT', new Alert('An alert', 'success'))
cy.get('.v-snack').then($alertElements => {
const backgroundColor = window.getComputedStyle($alertElements[0])['background-color']
expect(backgroundColor).to.not.equal(COLOR_BLACK)
@@ -51,7 +51,7 @@ describe('Alert component', () => {
cy.visit('/#/')
cy.get('.c-header').should('exist')
getStore().then(store => {
cy.wrap(store).invoke('commit', 'SET_ALERT', new Alert('An alert', null, 'warning'))
cy.wrap(store).invoke('commit', 'SET_ALERT', new Alert('An alert', 'warning'))
cy.get('.v-snack').then($alertElements => {
const backgroundColor = window.getComputedStyle($alertElements[0])['background-color']
expect(backgroundColor).to.not.equal(COLOR_BLACK)
@@ -64,7 +64,7 @@ describe('Alert component', () => {
cy.visit('/#/')
cy.get('.c-header').should('exist')
getStore().then(store => {
cy.wrap(store).invoke('commit', 'SET_ALERT', new Alert('An alert', null, 'error'))
cy.wrap(store).invoke('commit', 'SET_ALERT', new Alert('An alert', 'error'))
cy.get('.v-snack').then($alertElements => {
const backgroundColor = window.getComputedStyle($alertElements[0])['background-color']
expect(backgroundColor).to.not.equal(COLOR_BLACK)
@@ -78,7 +78,7 @@ describe('Alert component', () => {
cy.get('.c-header').should('exist')
const errorMessage = 'Error displayed'
getStore().then(store => {
store.dispatch('setAlert', new Alert(errorMessage, null, 'error'))
store.dispatch('setAlert', new Alert(errorMessage, 'error'))
})
cy
.get('.v-snack')
2 changes: 1 addition & 1 deletion tests/e2e/specs/editRuntimeForm.cy.js
Original file line number Diff line number Diff line change
@@ -159,7 +159,7 @@ describe('Edit Runtime form', () => {
.then(() => {
expect(receivedMutations.length).to.eq(0)
})
.get('[data-cy="response-snackbar"]')
.get('[data-cy="warning-snack"]')
.find('div[role="status"]')
.invoke('text')
.should('include', 'No changes were made')
6 changes: 3 additions & 3 deletions tests/e2e/specs/mutation.cy.js
Original file line number Diff line number Diff line change
@@ -150,13 +150,13 @@ describe('Mutations component', () => {
cy.get('[data-cy="submit"]')
.click()
// Error snackbar should appear
.get('[data-cy="response-snackbar"] > .v-snack__wrapper')
.get('[data-cy="alert-snack"] > .v-snack__wrapper')
.as('snackbar')
.should('be.visible')
.find('[data-cy="snackbar-close"]')
.find('[data-cy="snack-close"]')
.click()
.get('@snackbar')
.should('not.be.visible')
.should('not.exist')
// Form should stay open
.get('.c-mutation-dialog')
.should('be.visible')
2 changes: 1 addition & 1 deletion tests/unit/mixins/subscription.spec.js
Original file line number Diff line number Diff line change
@@ -84,7 +84,7 @@ describe('Subscription mixin', () => {
localVue,
store
})
const alert = new Alert('text', null, 'red')
const alert = new Alert('text', 'red')
component.vm.setAlert(alert)
expect(store.state.alert).to.equal(alert)
})
4 changes: 1 addition & 3 deletions tests/unit/model/alert.model.spec.js
Original file line number Diff line number Diff line change
@@ -29,11 +29,9 @@ describe('Alert model', () => {
describe('constructor', () => {
it('should be created', () => {
const text = 'my error'
const icon = 'error-icon'
const color = 'success'
const alert = new Alert(text, icon, color)
const alert = new Alert(text, color)
expect(alert.text).to.equal(text)
expect(alert.icon).to.equal(icon)
expect(alert.color).to.equal(color)
})
})
2 changes: 1 addition & 1 deletion tests/unit/services/user.service.spec.js
Original file line number Diff line number Diff line change
@@ -65,7 +65,7 @@ describe('UserService', () => {
sandbox.stub(axios, 'get').rejects(e)
return new UserService().getUserProfile()
.catch((error) => {
const alert = new Alert(error.response.statusText, null, 'error')
const alert = new Alert(error.response.statusText, 'error')
return store.dispatch('setAlert', alert)
})
.finally(() => {
6 changes: 3 additions & 3 deletions tests/unit/store/index.spec.js
Original file line number Diff line number Diff line change
@@ -50,13 +50,13 @@ describe('index', () => {
})
it('should set alert', () => {
const text = 'my-alert'
store.dispatch('setAlert', new Alert(text, '', ''))
store.dispatch('setAlert', new Alert(text, ''))
expect(store.state.alert.text).to.equal(text)
// repeating an alert with same text does not change anything
store.dispatch('setAlert', new Alert(text, '', ''))
store.dispatch('setAlert', new Alert(text, ''))
expect(store.state.alert.text).to.equal(text)
// but if the text is different, it will use the new value
store.dispatch('setAlert', new Alert('my-alert-2', '', ''))
store.dispatch('setAlert', new Alert('my-alert-2', ''))
expect(store.state.alert.text).to.equal('my-alert-2')
// and we can reset the state
store.dispatch('setAlert', null)

0 comments on commit b9fe3a8

Please sign in to comment.