Skip to content

Commit

Permalink
Fix bug where subscriptions would not update on query change
Browse files Browse the repository at this point in the history
  • Loading branch information
MetRonnie committed Feb 23, 2023
1 parent 68d2ae0 commit f9d64eb
Show file tree
Hide file tree
Showing 15 changed files with 48 additions and 142 deletions.
9 changes: 9 additions & 0 deletions src/mixins/subscriptionComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,17 @@ export default {
],
beforeMount () {
this.$workflowService.subscribe(this)
this.$workflowService.startSubscriptions()
},
beforeDestroy () {
this.$workflowService.unsubscribe(this)
},
watch: {
query () {
// if the query changes, unsubscribe & re-subscribe
this.$workflowService.unsubscribe(this)
this.$workflowService.subscribe(this)
this.$workflowService.startSubscriptions()
}
}
}
40 changes: 0 additions & 40 deletions src/mixins/subscriptionView.js

This file was deleted.

4 changes: 1 addition & 3 deletions src/views/Dashboard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
import { mapState, mapGetters } from 'vuex'
import { mdiBook, mdiBookMultiple, mdiBookOpenVariant, mdiCog, mdiHubspot, mdiTable } from '@mdi/js'
import pageMixin from '@/mixins/index'
import subscriptionViewMixin from '@/mixins/subscriptionView'
import subscriptionComponentMixin from '@/mixins/subscriptionComponent'
import { createUrl } from '@/utils/urls'
import { WorkflowState, WorkflowStateOrder } from '@/model/WorkflowState.model'
Expand All @@ -179,8 +178,7 @@ export default {
name: 'Dashboard',
mixins: [
pageMixin,
subscriptionComponentMixin,
subscriptionViewMixin
subscriptionComponentMixin
],
metaInfo () {
return {
Expand Down
4 changes: 1 addition & 3 deletions src/views/Graph.vue
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ import gql from 'graphql-tag'
import { mapGetters } from 'vuex'
import pageMixin from '@/mixins/index'
import graphqlMixin from '@/mixins/graphql'
import subscriptionViewMixin from '@/mixins/subscriptionView'
import subscriptionComponentMixin from '@/mixins/subscriptionComponent'
import SubscriptionQuery from '@/model/SubscriptionQuery.model'
// import CylcTreeCallback from '@/services/treeCallback'
Expand Down Expand Up @@ -203,8 +202,7 @@ export default {
mixins: [
pageMixin,
graphqlMixin,
subscriptionComponentMixin,
subscriptionViewMixin
subscriptionComponentMixin
],
name: 'Graph',
components: {
Expand Down
2 changes: 0 additions & 2 deletions src/views/Guide.vue
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,9 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
import Task from '@/components/cylc/Task'
import Job from '@/components/cylc/Job'
import { TaskStateUserOrder } from '@/model/TaskState.model'
import subscriptionViewMixin from '@/mixins/subscriptionView'

export default {
name: 'Guide',
mixins: [subscriptionViewMixin],
components: {
task: Task,
job: Job
Expand Down
6 changes: 2 additions & 4 deletions src/views/Log.vue
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
import { mdiFileDocumentMultipleOutline } from '@mdi/js'
import pageMixin from '@/mixins/index'
import graphqlMixin from '@/mixins/graphql'
import subscriptionViewMixin from '@/mixins/subscriptionView'
import subscriptionMixin from '@/mixins/subscriptionComponent'
import subscriptionComponentMixin from '@/mixins/subscriptionComponent'
import LogComponent from '@/components/cylc/log/Log'
import SubscriptionQuery from '@/model/SubscriptionQuery.model'
import { LOGS_SUBSCRIPTION } from '@/graphql/queries'
Expand All @@ -116,8 +115,7 @@ export default {
mixins: [
pageMixin,
graphqlMixin,
subscriptionMixin,
subscriptionViewMixin
subscriptionComponentMixin
],
name: 'Log',
components: {
Expand Down
4 changes: 1 addition & 3 deletions src/views/Table.vue
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import { mapState, mapGetters } from 'vuex'
import { mdiTable } from '@mdi/js'
import pageMixin from '@/mixins/index'
import graphqlMixin from '@/mixins/graphql'
import subscriptionViewMixin from '@/mixins/subscriptionView'
import subscriptionComponentMixin from '@/mixins/subscriptionComponent'
import TableComponent from '@/components/cylc/table/Table.vue'
import SubscriptionQuery from '@/model/SubscriptionQuery.model'
Expand All @@ -43,8 +42,7 @@ export default {
mixins: [
pageMixin,
graphqlMixin,
subscriptionComponentMixin,
subscriptionViewMixin
subscriptionComponentMixin
],
name: 'Table',
components: {
Expand Down
4 changes: 1 addition & 3 deletions src/views/Tree.vue
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import { mapState, mapGetters } from 'vuex'
import { mdiFileTree } from '@mdi/js'
import pageMixin from '@/mixins/index'
import graphqlMixin from '@/mixins/graphql'
import subscriptionViewMixin from '@/mixins/subscriptionView'
import subscriptionComponentMixin from '@/mixins/subscriptionComponent'
import SubscriptionQuery from '@/model/SubscriptionQuery.model'
import TreeComponent from '@/components/cylc/tree/Tree'
Expand All @@ -48,8 +47,7 @@ export default {
mixins: [
pageMixin,
graphqlMixin,
subscriptionComponentMixin,
subscriptionViewMixin
subscriptionComponentMixin
],
name: 'Tree',
components: {
Expand Down
4 changes: 1 addition & 3 deletions src/views/UserProfile.vue
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,13 @@ import { decreaseFontSize, getCurrentFontSize, increaseFontSize, resetFontSize }
import { mdiCog, mdiFormatFontSizeDecrease, mdiFormatFontSizeIncrease } from '@mdi/js'
import Job from '@/components/cylc/Job'
import JobState from '@/model/JobState.model'
import subscriptionViewMixin from '@/mixins/subscriptionView'

// TODO: update where user preferences are stored after #335

export default {
name: 'UserProfile',
mixins: [
pageMixin,
subscriptionViewMixin
pageMixin
],
components: {
Job
Expand Down
4 changes: 1 addition & 3 deletions src/views/Workflows.vue
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,14 @@ import GScan from '@/components/cylc/gscan/GScan'
import { GSCAN_DELTAS_SUBSCRIPTION } from '@/graphql/queries'
import pageMixin from '@/mixins/index'
import subscriptionComponentMixin from '@/mixins/subscriptionComponent'
import subscriptionViewMixin from '@/mixins/subscriptionView'
import SubscriptionQuery from '@/model/SubscriptionQuery.model'

export default {
name: 'Workflows',

mixins: [
pageMixin,
subscriptionComponentMixin,
subscriptionViewMixin
subscriptionComponentMixin
],

components: {
Expand Down
4 changes: 2 additions & 2 deletions src/views/WorkflowsTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ import { mapState, mapGetters } from 'vuex'
import i18n from '@/i18n'
import { mdiTable } from '@mdi/js'
import pageMixin from '@/mixins/index'
import subscriptionViewMixin from '@/mixins/subscriptionView'
import subscriptionMixin from '@/mixins/subscription'
import SubscriptionQuery from '@/model/SubscriptionQuery.model'
import { WORKFLOWS_TABLE_DELTAS_SUBSCRIPTION } from '@/graphql/queries'
import WorkflowIcon from '@/components/cylc/gscan/WorkflowIcon'
Expand All @@ -81,7 +81,7 @@ export default {
name: 'WorkflowsTable',
mixins: [
pageMixin,
subscriptionViewMixin
subscriptionMixin
],
metaInfo () {
return {
Expand Down
11 changes: 2 additions & 9 deletions src/views/Workspace.vue
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import Vue from 'vue'
import { each, iter } from '@lumino/algorithm'
import pageMixin from '@/mixins'
import graphqlMixin from '@/mixins/graphql'
import subscriptionViewMixin from '@/mixins/subscriptionView'
import subscriptionMixin from '@/mixins/subscription'
import ViewState from '@/model/ViewState.model'
import { createWidgetId } from '@/components/cylc/workflow/index'
import Lumino from '@/components/cylc/workflow/Lumino'
Expand All @@ -70,7 +70,7 @@ export default {
mixins: [
pageMixin,
graphqlMixin,
subscriptionViewMixin
subscriptionMixin
],

components: {
Expand Down Expand Up @@ -179,13 +179,6 @@ export default {
createWidgetId(),
{ view, initialOptions }
)
this.$nextTick(() => {
// Views use navigation-guards to start the pending subscriptions. But we don't have
// this in this view. We must pretend we are doing the beforeRouteEnter here, and
// call the .startSubscriptions function, so that the WorkflowService will take care
// of loading the pending subscriptions (like the ones created by the new view).
this.$workflowService.startSubscriptions()
})
},
/**
* Remove all the widgets present in the UI.
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/components/cylc/drawer.vue.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import { expect } from 'chai'
import { createLocalVue, mount } from '@vue/test-utils'
import Drawer from '../../../../src/components/cylc/Drawer'
import Drawer from '@/components/cylc/Drawer'

import Vue from 'vue'
import Vuetify from 'vuetify'
Expand All @@ -37,6 +37,7 @@ const mountFunction = options => {
register () {},
unregister () {},
subscribe () {},
startSubscriptions () {},
introspection: Promise.resolve({
mutations: [
{ args: [] }
Expand Down
47 changes: 25 additions & 22 deletions tests/unit/mixins/subscriptionComponent.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,46 +16,49 @@
*/

import { expect } from 'chai'
import sinon from 'sinon'
import { createLocalVue, shallowMount } from '@vue/test-utils'
import subscriptionComponentMixin from '@/mixins/subscriptionComponent'
import WorkflowService from '@/services/workflow.service'

const localVue = createLocalVue()

describe('Subscription Component mixin', () => {
let workflowService
let workflowService, Component
beforeEach(() => {
workflowService = {
subscribe (componentOrView) {
componentOrView.subscribed = true
},
unsubscribe (componentOrView) {
componentOrView.subscribed = false
}
}
workflowService = sinon.createStubInstance(WorkflowService)
localVue.prototype.$workflowService = workflowService
})
it('should provide a hook for when the component is created', () => {
const Component = {

Component = {
mixins: [subscriptionComponentMixin],
data: () => ({
query: { foo: 1 }
}),
render () {
}
}
})

it('subscribes & unsubscribes when the component is mounted & destroyed', () => {
const component = shallowMount(Component, {
localVue
})
expect(component.vm.subscribed).to.equal(true)
expect(workflowService.subscribe.calledOnceWith(component.vm)).to.equal(true)
expect(workflowService.startSubscriptions.calledOnce).to.equal(true)
expect(workflowService.unsubscribe.called).to.equal(false)
component.vm.$destroy()
expect(workflowService.unsubscribe.calledOnceWith(component.vm)).to.equal(true)
})
it('should provide a hook for when the component is destroyed', () => {
const Component = {
mixins: [subscriptionComponentMixin],
render () {
}
}

it('un- & re-subcribes when the query changes', () => {
const component = shallowMount(Component, {
localVue
})
expect(component.vm.subscribed).to.equal(true)
component.vm.$destroy()
expect(component.vm.subscribed).to.equal(false)
component.vm.query = { foo: 2 }
component.vm.$nextTick(() => {
expect(workflowService.unsubscribe.calledOnceWith(component.vm)).to.equal(true)
expect(workflowService.subscribe.calledTwice).to.equal(true)
expect(workflowService.startSubscriptions.calledTwice).to.equal(true)
})
})
})
44 changes: 0 additions & 44 deletions tests/unit/mixins/subscriptionView.spec.js

This file was deleted.

0 comments on commit f9d64eb

Please sign in to comment.