Skip to content

Commit

Permalink
fix: kubectl api-resources has duplicate rows and odd pagination beha…
Browse files Browse the repository at this point in the history
…vior

Fixes #4626
  • Loading branch information
myan9 authored and starpit committed May 18, 2020
1 parent eac3705 commit e740270
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 4 deletions.
4 changes: 4 additions & 0 deletions packages/test/src/api/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ export const TABLE_HEADER_CELL = (cellKey: string) => `thead tr th[data-key="${c
export const TABLE_CELL = (rowKey: string, cellKey: string) => `tbody [data-name="${rowKey}"] [data-key="${cellKey}"]`
export const TABLE_SHOW_AS_GRID = (N: number) => `${OUTPUT_N(N)} .kui--toolbar-button-as-grid`
export const TABLE_SHOW_AS_LIST = (N: number) => `${OUTPUT_N(N)} .kui--toolbar-button-as-list`
export const TABLE_PAGINATION_FORWARD = (N: number) =>
`${OUTPUT_N(N)} .kui--data-table-toolbar-pagination button.bx--pagination__button--forward`
export const TABLE_PAGINATION_BACKWARD = (N: number) =>
`${OUTPUT_N(N)} .kui--data-table-toolbar-pagination button.bx--pagination__button--backward`

const _TABLE_AS_GRID = '.kui--data-table-as-grid'
export const TABLE_AS_GRID = (N: number) => `${OUTPUT_N(N)} ${_TABLE_AS_GRID}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,12 @@ export default class LivePaginatedTable extends PaginatedTable<LiveProps, LiveSt
const existingRows = this.state.rows
const nRowsBefore = existingRows.length

const foundIndex = existingRows.findIndex(_ => _.id === newKuiRow.rowKey || _.NAME === newKuiRow.name)
const foundIndex = existingRows.findIndex(
_ => (_.rowKey && _.rowKey === newKuiRow.rowKey) || _.NAME === newKuiRow.name
// the _.rowKey existence check here is important
// because we didn't ask rowKey to be a required field
// if both of the rowKey are undefined, we will get a wrong foundIndex
)

const insertionIndex = foundIndex === -1 ? nRowsBefore : foundIndex

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export default class Toolbar extends React.PureComponent<Props> {
onMouseDown={evt => evt.preventDefault()}
disabled={isFirstPage}
className={
'bx--pagination__button bx--pagination__button--forward' +
'bx--pagination__button bx--pagination__button--backward' +
(isFirstPage ? ' bx--pagination__button--no-index' : '')
}
aria-label="Previous page"
Expand All @@ -118,7 +118,7 @@ export default class Toolbar extends React.PureComponent<Props> {
onMouseDown={evt => evt.preventDefault()}
disabled={isLastPage}
className={
'bx--pagination__button bx--pagination__backward--forward' +
'bx--pagination__button bx--pagination__button--forward' +
(isLastPage ? ' bx--pagination__button--no-index' : '')
}
aria-label="Next page"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { DataTableHeader, DataTableRow } from 'carbon-components-react'

export interface NamedDataTableRow extends DataTableRow {
NAME: string
rowKey: string
}

/** attempt to infer header model from body model */
Expand Down Expand Up @@ -59,7 +60,7 @@ export function kuiRow2carbonRow(headers: DataTableHeader[]) {
return (row: KuiRow, ridx: number): NamedDataTableRow => {
const isSelected = row.rowCSS ? row.rowCSS.includes('selected-row') : false

const rowData = { id: row.rowKey || `${row.name}-${ridx}`, isSelected, NAME: '' }
const rowData = { id: ridx.toString(), rowKey: row.rowKey, isSelected, NAME: '' }
rowData[headers[0].key] = row.name

if (!row.key) {
Expand Down
1 change: 1 addition & 0 deletions plugins/plugin-kubectl/src/controller/kubectl/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ class StatusWatcher implements Abortable, Watcher {

this.initialBody = this.resourcesToWaitFor.map(ref => {
const { group = '', version = '', kind, name, namespace } = ref

return {
name,
onclick: `${this.command} get ${fqnOfRef(ref)} -o yaml`,
Expand Down
50 changes: 50 additions & 0 deletions plugins/plugin-kubectl/src/test/k8s2/api-resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,56 @@ describe('kubectl api-resources', function(this: Common.ISuite) {
})
.catch(Common.oops(this, true)))

it('should get a list of api resources with pagination', () =>
CLI.command('kubectl api-resources', this.app)
.then(async res => {
console.error('api-resource table')
await ReplExpect.okWithCustom({ selector: Selectors.BY_NAME('bindings') })(res)
const actualTitle = await this.app.client.getText(Selectors.TABLE_TITLE(res.count))
assert.strictEqual(actualTitle, 'api-resources')

console.error('api-resource pagination forward')
await this.app.client.waitForExist(Selectors.TABLE_PAGINATION_FORWARD(res.count))
await this.app.client.click(Selectors.TABLE_PAGINATION_FORWARD(res.count))

console.error('api-resource 10 rows per page')
const testNumOfRows1 = await this.app.client.execute(tableSelector => {
const numRowsOfTable = document.querySelectorAll(tableSelector).length
return numRowsOfTable === 10
}, Selectors.LIST_RESULTS_N(res.count))

assert.strictEqual(testNumOfRows1.value, true)

console.error('api-resource two deployment rows with different apigroup')
const testNumOfDeploymentsRows = await this.app.client.execute(deploymentRowsSelector => {
// should see two deployments rows (NOTE: this behavior depends on different version of kubectl)
const deploymentRows = document.querySelectorAll(deploymentRowsSelector)
const numDeploymentRows = deploymentRows.length
const apiGroup1 = deploymentRows[0].textContent
const apiGroup2 = deploymentRows[1].textContent

return numDeploymentRows === 2 && apiGroup1 === 'apps' && apiGroup2 === 'extensions'
}, `${Selectors.OUTPUT_N(res.count)} ${Selectors.TABLE_CELL('deployments', 'APIGROUP')}`)

assert.strictEqual(testNumOfDeploymentsRows.value, true)

console.error('api-resource pagination backward')
await this.app.client.waitForExist(Selectors.TABLE_PAGINATION_BACKWARD(res.count))
await this.app.client.click(Selectors.TABLE_PAGINATION_BACKWARD(res.count))

console.error('api-resource table after backward')
await ReplExpect.okWithCustom({ selector: Selectors.BY_NAME('bindings') })(res)

console.error('api-resource 10 rows per page after backward')
const testNumOfRows2 = await this.app.client.execute(tableSelector => {
const numRowsOfTable = document.querySelectorAll(tableSelector).length
return numRowsOfTable === 10
}, Selectors.LIST_RESULTS_N(res.count))

assert.strictEqual(testNumOfRows2.value, true)
})
.catch(Common.oops(this, true)))

it('should get a list of api resources', () =>
CLI.command('kubectl api-resources --namespaced=true', this.app)
.then(ReplExpect.okWithCustom({ selector: Selectors.BY_NAME('bindings') }))
Expand Down

0 comments on commit e740270

Please sign in to comment.