Skip to content

Commit

Permalink
PR Feedback #1
Browse files Browse the repository at this point in the history
  • Loading branch information
grolu committed Dec 12, 2022
1 parent 5f71277 commit bbb1ffb
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 53 deletions.
7 changes: 2 additions & 5 deletions frontend/src/store/modules/shoots/getters.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export function getSortVal (rootGetters, item, sortBy) {

export default {
filteredItems (state) {
if (state.freezeSorting) {
if (state.focusMode) {
// When state is freezed, do not include new items
return map(state.uidsAtFreeze, freezedUID => {
const storeItem = find(state.filteredShoots, ['metadata.uid', freezedUID])
Expand Down Expand Up @@ -165,9 +165,6 @@ export default {
getShootListFilters (state) {
return state.shootListFilters
},
getFreezeSorting (state) {
return state.freezeSorting
},
onlyShootsWithIssues (state) {
return get(state.shootListFilters, 'onlyShootsWithIssues', true)
},
Expand Down Expand Up @@ -288,7 +285,7 @@ export default {
}
},
numberOfNewItemsSinceFreeze (state) {
if (!state.freezeSorting) {
if (!state.focusMode) {
return 0
}
return differenceWith(state.filteredShoots, state.uidsAtFreeze, (filteredShoot, uid) => {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/store/modules/shoots/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function putItem (state, newItem) {
Vue.set(state.shoots, keyForShoot(item.metadata), assign(item, newItem))
}
} else {
if (state.freezeSorting) {
if (state.focusMode) {
Vue.delete(state.freezedStaleShoots, newItem.metadata.uid)
}
newItem.info = undefined // register property to ensure reactivity
Expand All @@ -35,7 +35,7 @@ export function deleteItem (state, deletedItem) {
const item = findItem(state)(deletedItem.metadata)

if (item !== undefined) {
if (state.freezeSorting && state.uidsAtFreeze.includes(item.metadata.uid)) {
if (state.focusMode && state.uidsAtFreeze.includes(item.metadata.uid)) {
Vue.set(state.freezedStaleShoots, item.metadata.uid, { ...item, stale: true })
}
Vue.delete(state.shoots, keyForShoot(item.metadata))
Expand Down
14 changes: 7 additions & 7 deletions frontend/src/store/modules/shoots/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const state = {
shootListFilters: undefined,
newShootResource: undefined,
initialNewShootResource: undefined,
freezeSorting: false,
focusMode: false,
subscription: null,
subscriptionState: constants.CLOSED,
subscriptionError: null
Expand Down Expand Up @@ -368,16 +368,16 @@ const actions = {
commit('RESET_NEW_SHOOT_RESOURCE', shootResource)
return state.newShootResource
},
setFreezeSorting ({ commit }, value) {
commit('SET_FREEZE_SORTING', value)
setFocusMode ({ commit }, value) {
commit('SET_FOCUS_MODE', value)
if (value) {
const uids = map(state.filteredShoots, 'metadata.uid')
commit('SET_UIDS_AT_FREEZE', uids)
} else {
commit('CLEAR_FREEZED_STALE_SHOOTS')
commit('SET_UIDS_AT_FREEZE', undefined)
}
return state.freezeSorting
return state.focusMode
}
}

Expand Down Expand Up @@ -456,7 +456,7 @@ const mutations = {
}
}

if (state.freezeSorting) {
if (state.focusMode) {
const oldKeys = Object.keys(state.shoots)
const newKeys = Object.keys(shoots)
const removedShootKeys = difference(oldKeys, newKeys)
Expand Down Expand Up @@ -542,8 +542,8 @@ const mutations = {
state.newShootResource = shootResource
state.initialNewShootResource = cloneDeep(shootResource)
},
SET_FREEZE_SORTING (state, value) {
state.freezeSorting = value
SET_FOCUS_MODE (state, value) {
state.focusMode = value
},
SET_UIDS_AT_FREEZE (state, value) {
state.uidsAtFreeze = value
Expand Down
58 changes: 28 additions & 30 deletions frontend/src/views/ShootList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ SPDX-License-Identifier: Apache-2.0
overlap
>
<v-switch
v-model="freezeSorting"
v-model="focusModeInternal"
v-if="!projectScope && isAdmin"
class="mr-3"
color="primary lighten-3"
Expand Down Expand Up @@ -195,7 +195,7 @@ export default {
this.$localStorage.setObject('projects/shoot-list/options', { sortBy, sortDesc, itemsPerPage })
}
},
freezeSorting (value) {
focusModeInternal (value) {
if (value) {
const { sortBy, sortDesc } = this.options
const sortedItems = this.sortItems(this.items, sortBy, sortDesc)
Expand All @@ -213,9 +213,9 @@ export default {
...mapActions([
'subscribeShoots'
]),
...mapActions({
setFreezeSorting: 'shoots/setFreezeSorting'
}),
...mapActions('shoots', [
'setFocusMode'
]),
async showDialog (args) {
switch (args.action) {
case 'access':
Expand Down Expand Up @@ -290,7 +290,7 @@ export default {
this.shootSearch = value
}, 500),
customSort (items, sortByArr, sortDescArr) {
if (this.freezeSorting) {
if (this.focusModeInternal) {
// If freezed, the list is static - order is defined by the cached array
return map(this.sortedUIDsAtFreeze, freezedUID => {
return find(items, ['metadata.uid', freezedUID])
Expand Down Expand Up @@ -326,13 +326,12 @@ export default {
...mapGetters('shoots', [
'sortItems',
'searchItems',
'getFreezeSorting',
'numberOfNewItemsSinceFreeze'
]),
...mapState([
'cfg',
'namespace',
'shoots/freezeSorting'
'shoots/focusMode'
]),
defaultTableOptions () {
return {
Expand All @@ -351,12 +350,12 @@ export default {
}
}
},
freezeSorting: {
focusModeInternal: {
get () {
return this.getFreezeSorting
return this.focusMode
},
set (value) {
this.setFreezeSorting(value)
this.setFocusMode(value)
}
},
currentName () {
Expand Down Expand Up @@ -545,7 +544,7 @@ export default {
allHeaders () {
let allHeaders = [...this.standardHeaders, ...this.customHeaders]
allHeaders = sortBy(allHeaders, ['weight', 'text'])
if (this.freezeSorting) {
if (this.focusModeInternal) {
return map(allHeaders, header => {
return {
...header,
Expand All @@ -569,42 +568,42 @@ export default {
value: 'onlyShootsWithIssues',
selected: this.onlyShootsWithIssues,
hidden: this.projectScope,
disabled: this.disableChangeFilters
disabled: this.changeFiltersDisabled
},
{
text: 'Hide progressing clusters',
value: 'progressing',
selected: this.isFilterActive('progressing'),
hidden: this.projectScope || !this.isAdmin || this.hideFiltersForOnlyShootsWithIssues,
disabled: this.disableChangeFilters
hidden: this.projectScope || !this.isAdmin || this.showAllShoots,
disabled: this.changeFiltersDisabled
},
{
text: 'Hide no operator action required issues',
value: 'noOperatorAction',
selected: this.isFilterActive('noOperatorAction'),
hidden: this.projectScope || !this.isAdmin || this.hideFiltersForOnlyShootsWithIssues,
hidden: this.projectScope || !this.isAdmin || this.showAllShoots,
helpTooltip: [
'Hide clusters that do not require action by an operator',
'- Clusters with user issues',
'- Clusters with temporary issues that will be retried automatically',
'- Clusters with annotation dashboard.gardener.cloud/ignore-issues'
],
disabled: this.disableChangeFilters
disabled: this.changeFiltersDisabled
},
{
text: 'Hide clusters with deactivated reconciliation',
value: 'deactivatedReconciliation',
selected: this.isFilterActive('deactivatedReconciliation'),
hidden: this.projectScope || !this.isAdmin || this.hideFiltersForOnlyShootsWithIssues,
disabled: this.disableChangeFilters
hidden: this.projectScope || !this.isAdmin || this.showAllShoots,
disabled: this.changeFiltersDisabled
},
{
text: 'Hide clusters with configured ticket labels',
value: 'hideTicketsWithLabel',
selected: this.isFilterActive('hideTicketsWithLabel'),
hidden: this.projectScope || !this.isAdmin || !this.gitHubRepoUrl || !this.hideClustersWithLabels.length || this.hideFiltersForOnlyShootsWithIssues,
hidden: this.projectScope || !this.isAdmin || !this.gitHubRepoUrl || !this.hideClustersWithLabels.length || this.showAllShoots,
helpTooltip: this.hideTicketsWithLabelTooltip,
disabled: this.disableChangeFilters
disabled: this.changeFiltersDisabled
}
]
},
Expand All @@ -629,17 +628,16 @@ export default {
items () {
return this.cachedItems || this.mappedItems
},
disableChangeFilters () {
return this.freezeSorting
changeFiltersDisabled () {
return this.focusModeInternal
},
hideFiltersForOnlyShootsWithIssues () {
showAllShoots () {
return !this.showOnlyShootsWithIssues
},
filterTooltip () {
if (this.freezeSorting) {
return 'Filters cannot be changed when focus mode is active'
}
return undefined
return this.freezeSorting
? 'Filters cannot be changed when focus mode is active'
: ''
},
headlineSubtitle () {
const subtitle = []
Expand Down Expand Up @@ -676,13 +674,13 @@ export default {
beforeRouteUpdate (to, from, next) {
this.shootSearch = null
this.updateTableSettings()
this.freezeSorting = false
this.focusModeInternal = false
next()
},
beforeRouteLeave (to, from, next) {
this.cachedItems = this.mappedItems.slice(0)
this.shootSearch = null
this.freezeSorting = false
this.focusModeInternal = false
next()
}
}
Expand Down
18 changes: 9 additions & 9 deletions frontend/tests/unit/store.shoots.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('store.shoots', () => {
}
let state
let sortItems
let setFreezeSorting
let setFocusMode

beforeEach(() => {
shootItems = [
Expand Down Expand Up @@ -134,12 +134,12 @@ describe('store.shoots', () => {
state = {
shoots: fromPairs(shootItemKeyValuePairs),
filteredShoots: shootItems,
freezeSorting: false,
focusMode: false,
freezedStaleShoots: []
}
assign(shootModule.state, state)

setFreezeSorting = (value) => shootModule.actions.setFreezeSorting({ commit: (f, v) => shootModule.mutations[f](shootModule.state, v) }, value)
setFocusMode = (value) => shootModule.actions.setFocusMode({ commit: (f, v) => shootModule.mutations[f](shootModule.state, v) }, value)
sortItems = getters.sortItems(shootModule.state, undefined, undefined, rootGetters)
})

Expand Down Expand Up @@ -240,7 +240,7 @@ describe('store.shoots', () => {
expect(shootModule.state.filteredShoots.length).toBe(2)
expect(shootModule.getters.filteredItems(shootModule.state).length).toBe(2)
expect(shootModule.getters.filteredItems(shootModule.state)[0].stale).toBe(undefined)
setFreezeSorting(true)
setFocusMode(true)

expect(shootModule.getters.filteredItems(shootModule.state)[0].stale).toBe(undefined)
deleteItem(shootModule.state, shootModule.state.filteredShoots[0])
Expand All @@ -252,7 +252,7 @@ describe('store.shoots', () => {
})

it('should not add new shoots to list when shoot list is freezed', () => {
setFreezeSorting(true)
setFocusMode(true)
const newShoot = {
metadata: {
name: 'shoot4',
Expand All @@ -267,16 +267,16 @@ describe('store.shoots', () => {

expect(getters.filteredItems(shootModule.state).length).toBe(3)

setFreezeSorting(false)
setFocusMode(false)
expect(getters.filteredItems(shootModule.state).length).toBe(4)

setFreezeSorting(true)
setFocusMode(true)
expect(getters.filteredItems(shootModule.state).length).toBe(4)
})

it('should add and remove freezedStaleShoots', () => {
const shoot = Object.values(shootModule.state.shoots)[1]
setFreezeSorting(true)
setFocusMode(true)

deleteItem(shootModule.state, shoot)
expect(shootModule.state.freezedStaleShoots[shoot.metadata.uid]).not.toBeUndefined()
Expand All @@ -289,7 +289,7 @@ describe('store.shoots', () => {
describe('mutations', () => {
it('should receive items and update freezedStaleShoots', () => {
const shoots = Object.values(shootModule.state.shoots)
setFreezeSorting(true)
setFocusMode(true)
const itemToDelete = shoots[0]
deleteItem(shootModule.state, itemToDelete)

Expand Down

0 comments on commit bbb1ffb

Please sign in to comment.