Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ktable): store offset of next page in nextOffset #2355

Merged
merged 15 commits into from
Aug 27, 2024
Merged

fix(ktable): store offset of next page in nextOffset #2355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a8ca54c
fix(ktable): send to page 1 when refreshed [KHCP-13030]
portikM Aug 21, 2024
cdd525c
fix(ktable): add comment [KHCP-13030]
portikM Aug 21, 2024
24824c3
fix(ktable): send to first page in consuming component [KHCP-13030]
portikM Aug 21, 2024
6c22db1
fix(ktable): watch offsets [KHCP-13030]
portikM Aug 22, 2024
eb755cb
fix: minor fix
portikM Aug 22, 2024
8004301
fix(ktable): watch page [KHCP-13030]
portikM Aug 22, 2024
27225ba
fix(ktable): watch itini data page [KHCP-13030]
portikM Aug 22, 2024
8b968d4
fix: minor fix
portikM Aug 22, 2024
820f6fa
fix(ktable): add console.logs [KHCP-13030]
portikM Aug 22, 2024
8109c02
fix(ktable): comment out the offender [KHCP-13030]
portikM Aug 22, 2024
4b670ad
fix(ktable): clean up [KHCP-13030]
portikM Aug 22, 2024
00b0f14
fix(ktable): store offset of next page in nextOffset
Leopoldthecoder Aug 23, 2024
c1a60ee
fix(ktable): send to first page when last item of last page deleted
Leopoldthecoder Aug 26, 2024
6e05744
test(ktable): add tests for refetch params
Leopoldthecoder Aug 26, 2024
72b088d
Merge branch 'main' into fix/store-next-offset
portikM Aug 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 127 additions & 0 deletions src/components/KTable/KTable.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ import KTable from '@/components/KTable/KTable.vue'
import { offsetPaginationHeaders, offsetPaginationFetcher } from '../../../mocks/KTableMockData'
import type { TableHeader } from '@/types'

interface FetchParams {
pageSize: number
page: number
query?: string
sortColumnKey?: string
sortColumnOrder?: 'asc' | 'desc'
offset?: string | null
}

const largeDataSet = [
{
name: 'Basic Auth',
Expand Down Expand Up @@ -666,6 +675,124 @@ describe('KTable', () => {

cy.getTestId('table-pagination').should('be.visible')
})

it('refetch with paginationOffset: true', () => {
const data: Array<{ name: string }> = []
for (let i = 0; i < 12; i++) {
data.push({ name: 'row' + i })
}
const fns = {
fetcher: (params: FetchParams) => {
const { pageSize, page, offset } = params
const start = offset ? Number(offset) : 0
return {
data: data.slice(start, start + pageSize),
pagination: {
offset: `${start + pageSize}`,
page,
},
}
},
}
cy.spy(fns, 'fetcher').as('fetcher')

mount(KTable, {
propsData: {
fetcher: fns.fetcher,
initialFetcherParams: { pageSize: 10 },
loading: false,
headers: options.headers,
paginationPageSizes: [10],
paginationOffset: true,
hidePaginationWhenOptional: true,
fetcherCacheKey: '0',
},
})

// page 1
cy.getTestId('table-pagination').should('be.visible')
cy.get('.table tbody').find('tr').should('have.length', 10)
cy.get('.table tbody').should('contain.text', 'row0')
cy.get('@fetcher')
.should('have.callCount', 1) // ensure fetcher is NOT called twice on load
.should('have.been.calledWith', { pageSize: 10, page: 1, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' })
.then(() => cy.wrap(Cypress.vueWrapper.setProps({ fetcherCacheKey: '1' }))) // manually trigger refetch
.get('@fetcher')
.should('have.callCount', 2)
.its('lastCall')
.should('have.been.calledWith', { pageSize: 10, page: 1, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' })

// page 2
cy.getTestId('next-button').click()
cy.get('.table tbody').find('tr').should('have.length', 2)
cy.get('.table tbody').should('contain.text', 'row10')
cy.get('@fetcher')
.should('have.callCount', 3)
.its('lastCall')
.should('have.been.calledWith', { pageSize: 10, page: 2, offset: '10', query: '', sortColumnKey: '', sortColumnOrder: 'desc' })
.then(() => cy.wrap(Cypress.vueWrapper.setProps({ fetcherCacheKey: '2' }))) // manually trigger refetch
.get('@fetcher')
.should('have.callCount', 4)
.its('lastCall')
.should('have.been.calledWith', { pageSize: 10, page: 2, offset: '10', query: '', sortColumnKey: '', sortColumnOrder: 'desc' })
})

it('refetch with paginationOffset: false', () => {
const data: Array<{ name: string }> = []
for (let i = 0; i < 12; i++) {
data.push({ name: 'row' + i })
}
const fns = {
fetcher: (params: FetchParams) => {
const { pageSize, page } = params
return {
data: data.slice((page - 1) * pageSize, page * pageSize),
total: data.length,
}
},
}
cy.spy(fns, 'fetcher').as('fetcher')

mount(KTable, {
propsData: {
fetcher: fns.fetcher,
initialFetcherParams: { pageSize: 10 },
loading: false,
headers: options.headers,
paginationPageSizes: [10],
paginationOffset: false,
hidePaginationWhenOptional: true,
fetcherCacheKey: '0',
},
})

// page 1
cy.getTestId('table-pagination').should('be.visible')
cy.get('.table tbody').find('tr').should('have.length', 10)
cy.get('.table tbody').should('contain.text', 'row0')
cy.get('@fetcher')
.should('have.callCount', 1) // ensure fetcher is NOT called twice on load
.should('have.been.calledWith', { pageSize: 10, page: 1, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' })
.then(() => cy.wrap(Cypress.vueWrapper.setProps({ fetcherCacheKey: '1' }))) // manually trigger refetch
.get('@fetcher')
.should('have.callCount', 2)
.its('lastCall')
.should('have.been.calledWith', { pageSize: 10, page: 1, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' })

// page 2
cy.getTestId('next-button').click()
cy.get('.table tbody').find('tr').should('have.length', 2)
cy.get('.table tbody').should('contain.text', 'row10')
cy.get('@fetcher')
.should('have.callCount', 3)
.its('lastCall')
.should('have.been.calledWith', { pageSize: 10, page: 2, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' })
.then(() => cy.wrap(Cypress.vueWrapper.setProps({ fetcherCacheKey: '2' }))) // manually trigger refetch
.get('@fetcher')
.should('have.callCount', 4)
.its('lastCall')
.should('have.been.calledWith', { pageSize: 10, page: 2, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' })
})
})

describe('misc', () => {
Expand Down
28 changes: 15 additions & 13 deletions src/components/KTable/KTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@
:initial-page-size="pageSize"
:neighbors="paginationNeighbors"
:offset="paginationOffset"
:offset-next-button-disabled="!offset || !hasNextPage"
:offset-next-button-disabled="!nextOffset || !hasNextPage"
:offset-previous-button-disabled="!previousOffset"
:page-sizes="paginationPageSizes"
:total-count="total"
Expand Down Expand Up @@ -552,7 +552,6 @@ const offsets: Ref<Array<any>> = ref([])
const hasNextPage = ref(true)
const isClickable = ref(false)
const hasInitialized = ref(false)
const nextPageClicked = ref(false)
const hasToolbarSlot = computed((): boolean => !!slots.toolbar || hasColumnVisibilityMenu.value)
const tableWrapperStyles = computed((): Record<string, string> => ({
maxHeight: getSizeFromString(props.maxHeight),
Expand Down Expand Up @@ -820,15 +819,9 @@ const fetchData = async () => {

if (props.paginationOffset) {
if (!res.pagination?.offset) {
offset.value = null

// reset to first page if no pagiantion data is returned unless the "next page" button was clicked
// this will ensure buttons display the correct state for cases like search
if (!nextPageClicked.value) {
page.value = 1
}
nextOffset.value = null
} else {
offset.value = res.pagination.offset
nextOffset.value = res.pagination.offset

if (!offsets.value[page.value]) {
offsets.value.push(res.pagination.offset)
Expand All @@ -838,7 +831,15 @@ const fetchData = async () => {
hasNextPage.value = (res.pagination && 'hasNextPage' in res.pagination) ? res.pagination.hasNextPage : true
}

nextPageClicked.value = false
// if the data is empty and the page is greater than 1,
// e.g. user deletes the last item on the last page,
// reset the page to 1
if (data.value.length === 0 && page.value > 1) {
page.value = 1
offsets.value = [null]
offset.value = null
}

isInitialFetch.value = false

return res
Expand Down Expand Up @@ -882,6 +883,7 @@ watch(() => props.headers, (newVal: TableHeader[]) => {
}, { deep: true })

const previousOffset = computed((): string | null => offsets.value[page.value - 1])
const nextOffset = ref<string | null>(null)

// once `initData()` finishes, setting tableFetcherCacheKey to non-falsey value triggers fetch of data
const tableFetcherCacheKey = computed((): string => {
Expand Down Expand Up @@ -1010,7 +1012,7 @@ const emitTablePreferences = (): void => {

const getNextOffsetHandler = (): void => {
page.value++
nextPageClicked.value = true
offset.value = nextOffset.value
}

const getPrevOffsetHandler = (): void => {
Expand All @@ -1026,7 +1028,7 @@ const getPrevOffsetHandler = (): void => {
const shouldShowPagination = computed((): boolean => {
return !!(props.fetcher && !props.disablePagination &&
!(!props.paginationOffset && props.hidePaginationWhenOptional && total.value <= props.paginationPageSizes[0]) &&
!(props.paginationOffset && props.hidePaginationWhenOptional && !previousOffset.value && !offset.value && data.value.length < props.paginationPageSizes[0]))
!(props.paginationOffset && props.hidePaginationWhenOptional && !previousOffset.value && !nextOffset.value && data.value.length < props.paginationPageSizes[0]))
})

const getTestIdString = (message: string): string => {
Expand Down
Loading