Merge pull request #330 from smiclea/provider-pagination

Dynamic instances pagination based on provider

src/components/organisms/WizardInstances/WizardInstances.jsx

@@ -190,7 +190,7 @@ type Props = {
   instances: InstanceType[],
   selectedInstances: ?InstanceType[],
   currentPage: number,
-  chunkSize: number,
+  instancesPerPage: number,
   loading: boolean,
   chunksLoading: boolean,
   searching: boolean,
@@ -295,8 +295,8 @@ class WizardInstances extends React.Component<Props, State> {
     if (this.props.loading || this.props.searchNotFound || this.props.reloading || this.areNoInstances()) {
       return null
     }
-    let startIdx = (this.props.currentPage - 1) * this.props.chunkSize
-    let endIdx = startIdx + (this.props.chunkSize - 1)
+    let startIdx = (this.props.currentPage - 1) * this.props.instancesPerPage
+    let endIdx = startIdx + (this.props.instancesPerPage - 1)
     let filteredInstances = this.props.instances.filter((i, idx) => idx >= startIdx && idx <= endIdx)
 
     return (
@@ -359,7 +359,7 @@ class WizardInstances extends React.Component<Props, State> {
       return null
     }
 
-    let hasNextPage = this.props.currentPage * this.props.chunkSize < this.props.instances.length
+    let hasNextPage = this.props.currentPage * this.props.instancesPerPage < this.props.instances.length
     let areAllDisabled = this.props.searching
     let isPreviousDisabled = this.props.currentPage === 1 || areAllDisabled
     let isNextDisabled = !hasNextPage || areAllDisabled
@@ -374,7 +374,7 @@ class WizardInstances extends React.Component<Props, State> {
           <Arrow orientation="left" disabled={isPreviousDisabled} color={Palette.black} thick />
         </PagePrevious>
         <PageNumber data-test-id="wInstances-currentPage">
-          {this.props.currentPage} of {Math.ceil(this.props.instances.length / this.props.chunkSize)}
+          {this.props.currentPage} of {Math.ceil(this.props.instances.length / this.props.instancesPerPage)}
           {this.props.chunksLoading ? (
             <HorizontalLoading style={{ width: '100%', top: '3px' }} data-test-id="wInstances-loadingChunks" />
           ) : null}

src/components/organisms/WizardInstances/test.jsx

@@ -22,7 +22,7 @@ import WizardInstances from '.'
 
 const wrap = props => new TW(shallow(
   // $FlowIgnore
-  <WizardInstances chunkSize={6} {...props} />
+  <WizardInstances instancesPerPage={6} {...props} />
 ), 'wInstances')
 
 let instances = [
@@ -30,16 +30,14 @@ let instances = [
   { id: 'i-2', flavor_name: 'Flavor name', instance_name: 'Instance name 2', num_cpu: 3, memory_mb: 1024 },
   { id: 'i-3', flavor_name: 'Flavor name', instance_name: 'Instance name 3', num_cpu: 3, memory_mb: 1024 },
 ]
-let onChunkSizeUpdate = () => { }
-
 describe('WizardInstances Component', () => {
   it('has correct number of instances', () => {
-    let wrapper = wrap({ instances, currentPage: 1, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 1 })
     expect(wrapper.find('item-', true).length).toBe(instances.length)
   })
 
   it('has correct instances info', () => {
-    let wrapper = wrap({ instances, currentPage: 1, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 1 })
     instances.forEach(instance => {
       expect(wrapper.find(`item-${instance.id}`).findText('itemName')).toBe(instance.instance_name)
       expect(wrapper.find(`item-${instance.id}`).findText('itemDetails')).toBe(`${instance.num_cpu} vCPU | ${instance.memory_mb} MB RAM | ${instance.flavor_name}`)
@@ -54,7 +52,7 @@ describe('WizardInstances Component', () => {
         { ...instances[0] },
         { ...instances[2] },
       ],
-      onChunkSizeUpdate,
+      instancesPerPage: 3,
     })
     expect(wrapper.findText('selInfo')).toBe('2 instances selected')
     expect(wrapper.find('item-i-1').prop('selected')).toBe(true)
@@ -63,56 +61,56 @@ describe('WizardInstances Component', () => {
   })
 
   it('renders current page', () => {
-    let wrapper = wrap({ instances, currentPage: 2, chunkSize: 2, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 2, instancesPerPage: 2 })
     expect(wrapper.findText('currentPage')).toBe('2 of 2')
   })
 
   it('renders previous page disabled if page is 1', () => {
-    let wrapper = wrap({ instances, currentPage: 1, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 1 })
     expect(wrapper.find('prevPageButton').prop('disabled')).toBe(true)
   })
 
   it('renders previous page enabled if page is greater than 1', () => {
-    let wrapper = wrap({ instances, currentPage: 3, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 3 })
     expect(wrapper.find('prevPageButton').prop('disabled')).toBeFalsy()
     expect(wrapper.find('loadingStatus').length).toBe(0)
   })
 
   it('renders loading', () => {
-    let wrapper = wrap({ instances, currentPage: 1, loading: true, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 1, loading: true })
     expect(wrapper.find('loadingStatus').length).toBe(1)
   })
 
   it('renders searching', () => {
-    let wrapper = wrap({ instances, currentPage: 1, searching: true, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 1, searching: true })
     expect(wrapper.find('searchInput').prop('loading')).toBe(true)
   })
 
   it('renders search not found', () => {
-    let wrapper = wrap({ instances: [], currentPage: 1, searchNotFound: true, onChunkSizeUpdate })
+    let wrapper = wrap({ instances: [], currentPage: 1, searchNotFound: true })
     expect(wrapper.findText('notFoundText')).toBe('Your search returned no results')
     expect(wrapper.find('loadingChunks').length).toBe(0)
   })
 
   it('renders loading page', () => {
-    let wrapper = wrap({ instances, currentPage: 1, chunksLoading: true, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 1, chunksLoading: true })
     expect(wrapper.find('loadingChunks').length).toBe(1)
   })
 
   it('enabled next page', () => {
-    let wrapper = wrap({ instances, currentPage: 1, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 1 })
     expect(wrapper.find('nextPageButton').prop('disabled')).toBe(true)
-    wrapper = wrap({ instances, currentPage: 1, chunkSize: 2, onChunkSizeUpdate })
+    wrapper = wrap({ instances, currentPage: 1, instancesPerPage: 2 })
     expect(wrapper.find('nextPageButton').prop('disabled')).toBeFalsy()
   })
 
   it('dispatches next and previous page click, if enabled', () => {
     let onPageClick = sinon.spy()
-    let wrapper = wrap({ instances, currentPage: 1, onPageClick, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 1, onPageClick })
     wrapper.find('nextPageButton').click()
     wrapper.find('prevPageButton').click()
     expect(onPageClick.callCount).toBe(0)
-    wrapper = wrap({ instances, currentPage: 2, onPageClick, chunkSize: 1, onChunkSizeUpdate })
+    wrapper = wrap({ instances, currentPage: 2, onPageClick, instancesPerPage: 1 })
     wrapper.find('nextPageButton').click()
     wrapper.find('prevPageButton').click()
     expect(onPageClick.callCount).toBe(2)
@@ -120,7 +118,7 @@ describe('WizardInstances Component', () => {
 
   it('dispaches reload click', () => {
     let onReloadClick = sinon.spy()
-    let wrapper = wrap({ instances, currentPage: 1, onReloadClick, onChunkSizeUpdate })
+    let wrapper = wrap({ instances, currentPage: 1, onReloadClick })
     wrapper.find('reloadButton').click()
     expect(onReloadClick.calledOnce).toBe(true)
   })

src/components/organisms/WizardPageContent/WizardPageContent.jsx

@@ -315,7 +315,7 @@ class WizardPageContent extends React.Component<Props, State> {
         body = (
           <WizardInstances
             instances={this.props.instanceStore.instances}
-            chunkSize={this.props.instanceStore.chunkSize}
+            instancesPerPage={this.props.instanceStore.instancesPerPage}
             chunksLoading={this.props.instanceStore.chunksLoading}
             currentPage={this.props.instanceStore.currentPage}
             searchText={this.props.instanceStore.searchText}

src/components/pages/WizardPage/WizardPage.jsx

@@ -70,8 +70,9 @@ class WizardPage extends React.Component<Props, State> {
 
   contentRef: WizardPageContent
 
-  get instancesChunkSize() {
-    let { min, max } = wizardConfig.instancesPerPage
+  get instancesPerPage() {
+    const min = 3
+    const max = Infinity
     const instancesTableDiff = 505
     const instancesItemHeight = 67
     return Math.min(max, Math.max(min, Math.floor((window.innerHeight - instancesTableDiff) / instancesItemHeight)))
@@ -113,7 +114,7 @@ class WizardPage extends React.Component<Props, State> {
 
   @autobind
   handleResize() {
-    instanceStore.updateChunkSize(this.instancesChunkSize)
+    instanceStore.updateInstancesPerPage(this.instancesPerPage)
   }
 
   handleEnterKey() {
@@ -218,7 +219,7 @@ class WizardPage extends React.Component<Props, State> {
     endpointStore.getConnectionInfo(source).then(() => {
       if (source) {
         // Preload instances for 'vms' page
-        instanceStore.loadInstancesInChunks(source.id, this.instancesChunkSize)
+        instanceStore.loadInstancesInChunks(source, this.instancesPerPage)
       }
     }).catch(() => {
       this.handleSourceEndpointChange(null)
@@ -263,13 +264,13 @@ class WizardPage extends React.Component<Props, State> {
 
   handleInstancesSearchInputChange(searchText: string) {
     if (wizardStore.data.source) {
-      instanceStore.searchInstances(wizardStore.data.source.id, searchText)
+      instanceStore.searchInstances(wizardStore.data.source, searchText)
     }
   }
 
   handleInstancesReloadClick() {
     if (wizardStore.data.source) {
-      instanceStore.reloadInstances(wizardStore.data.source.id, this.instancesChunkSize)
+      instanceStore.reloadInstances(wizardStore.data.source, this.instancesPerPage)
     }
   }
 
@@ -284,10 +285,6 @@ class WizardPage extends React.Component<Props, State> {
     instanceStore.setPage(page)
   }
 
-  handleInstanceChunkSizeUpdate(chunkSize: number) {
-    instanceStore.updateChunkSize(chunkSize)
-  }
-
   handleDestOptionsChange(field: Field, value: any) {
     wizardStore.updateData({ networks: null })
     wizardStore.clearStorageMap()
@@ -366,7 +363,7 @@ class WizardPage extends React.Component<Props, State> {
           // Check if user has permission for this endpoint
           endpointStore.getConnectionInfo(source).then(() => {
             // Preload instances for 'vms' page
-            instanceStore.loadInstancesInChunks(source.id, this.instancesChunkSize)
+            instanceStore.loadInstancesInChunks(source, this.instancesPerPage)
           }).catch(() => {
             this.handleSourceEndpointChange(null)
           })
@@ -517,7 +514,6 @@ class WizardPage extends React.Component<Props, State> {
             onInstancesReloadClick={() => { this.handleInstancesReloadClick() }}
             onInstanceClick={instance => { this.handleInstanceClick(instance) }}
             onInstancePageClick={page => { this.handleInstancePageClick(page) }}
-            onInstanceChunkSizeUpdate={chunkSize => { this.handleInstanceChunkSizeUpdate(chunkSize) }}
             onDestOptionsChange={(field, value) => { this.handleDestOptionsChange(field, value) }}
             onSourceOptionsChange={(field, value) => { this.handleSourceOptionsChange(field, value) }}
             onNetworkChange={(sourceNic, targetNetwork) => { this.handleNetworkChange(sourceNic, targetNetwork) }}

src/config.js

@@ -112,9 +112,14 @@ export const wizardConfig = {
     { id: 'schedule', title: 'Schedule', breadcrumb: 'Schedule', excludeFrom: 'migration' },
     { id: 'summary', title: 'Summary', breadcrumb: 'Summary' },
   ],
-  instancesPerPage: { min: 3, max: Infinity },
 }
 
+// - Specifies the `limit` for each provider when listing all its VMs for pagination.
+// - If the provider is not in this list, the 'default' value will be used.
+// - If the `default` value is lower than the number of instances that fit into a page, the latter number will be used.
+// - `Infinity` value means no `limit` will be used, i.e. all VMs will be listed.
+export const instancesListBackgroundLoading = { default: 10, ovm: Infinity }
+
 // A list of providers for which `destination-options` API call(s) will be made in the Wizard
 // If the item is just a string with the provider name, only one API call will be made
 // If the item has `envRequiredFields`, an additional API call will be made once the specified fields are filled

src/sources/InstanceSource.js

@@ -28,16 +28,31 @@ class InstanceSource {
     searchText?: string
   ): Promise<Instance[]> {
     let url = `${servicesUrl.coriolis}/${Api.projectId}/endpoints/${endpointId}/instances`
-    url = `${url}?limit=${chunkSize}`
+    let queryParams: { [string]: string | number } = {}
 
-    if (lastInstanceId) {
-      url = `${url}&marker=${lastInstanceId}`
+    if (chunkSize !== Infinity) {
+      queryParams = {
+        limit: chunkSize,
+      }
+
+      if (lastInstanceId) {
+        queryParams = {
+          ...queryParams,
+          marker: lastInstanceId,
+        }
+      }
     }
 
     if (searchText) {
-      url = `${url}&name=${searchText}`
+      queryParams = {
+        ...queryParams,
+        name: searchText,
+      }
     }
 
+    let keys = Object.keys(queryParams)
+    url = `${url}${keys.length > 0 ? '?' : ''}${keys.map(p => `${p}=${queryParams[p]}`).join('&')}`
+
     return Api.send({ url, cancelId }).then(response => {
       return response.data.instances
     })

src/stores/InstanceStore.js

@@ -17,8 +17,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 import { observable, action, computed } from 'mobx'
 
 import type { Instance } from '../types/Instance'
+import type { Endpoint } from '../types/Endpoint'
 import InstanceSource from '../sources/InstanceSource'
 import ApiCaller from '../utils/ApiCaller'
+import { instancesListBackgroundLoading as chunkSize } from '../config'
 
 class InstanceLocalStorage {
   static saveInstancesToLocalStorage(endpointId: string, instances: Instance[]) {
@@ -83,7 +85,7 @@ class InstanceLocalStorage {
 
 class InstanceStore {
   @observable instancesLoading = false
-  @observable chunkSize = 6
+  @observable instancesPerPage = 6
   @observable currentPage = 1
   @observable searchChunksLoading = false
   @observable searchedInstances: Instance[] = []
@@ -115,8 +117,8 @@ class InstanceStore {
   lastEndpointId: string
   reqId: number
 
-  @action loadInstancesInChunks(endpointId: string, chunkSize?: number = 6, reload?: boolean) {
-    ApiCaller.cancelRequests(`${endpointId}-chunk`)
+  @action loadInstancesInChunks(endpoint: Endpoint, vmsPerPage?: number = 6, reload?: boolean) {
+    ApiCaller.cancelRequests(`${endpoint.id}-chunk`)
 
     this.backgroundInstances = []
     if (reload) {
@@ -125,11 +127,13 @@ class InstanceStore {
       this.instancesLoading = true
     }
     this.backgroundChunksLoading = true
-    this.lastEndpointId = endpointId
+    this.lastEndpointId = endpoint.id
+
+    let chunkCount = Math.max(chunkSize[endpoint.type] || chunkSize.default, vmsPerPage)
 
     let loadNextChunk = (lastEndpointId?: string) => {
-      let currentEndpointId = endpointId
-      InstanceSource.loadInstancesChunk(currentEndpointId, chunkSize, lastEndpointId, `${endpointId}-chunk`)
+      let currentEndpointId = endpoint.id
+      InstanceSource.loadInstancesChunk(currentEndpointId, chunkCount, lastEndpointId, `${endpoint.id}-chunk`)
         .then(instances => {
           if (currentEndpointId !== this.lastEndpointId) {
             return
@@ -141,7 +145,7 @@ class InstanceStore {
           }
           this.instancesLoading = false
 
-          if (instances.length < chunkSize) {
+          if (instances.length < chunkCount) {
             this.backgroundChunksLoading = false
             return
           }
@@ -177,8 +181,8 @@ class InstanceStore {
     })
   }
 
-  @action searchInstances(endpointId: string, searchText: string) {
-    ApiCaller.cancelRequests(`${endpointId}-chunk-search`)
+  @action searchInstances(endpoint: Endpoint, searchText: string) {
+    ApiCaller.cancelRequests(`${endpoint.id}-chunk-search`)
 
     this.searchText = searchText
     this.searchNotFound = false
@@ -199,14 +203,15 @@ class InstanceStore {
 
     this.searching = true
     this.searchChunksLoading = true
-    let chunkSize = this.chunkSize
+
+    let chunkCount = Math.max(chunkSize[endpoint.type] || chunkSize.default, this.instancesPerPage)
 
     let loadNextChunk = (lastEndpointId?: string) => {
       InstanceSource.loadInstancesChunk(
-        endpointId,
-        chunkSize,
+        endpoint.id,
+        chunkCount,
         lastEndpointId,
-        `${endpointId}-chunk-search`,
+        `${endpoint.id}-chunk-search`,
         searchText
       ).then(instances => {
         if (this.searching) {
@@ -217,7 +222,7 @@ class InstanceStore {
         this.searchedInstances = [...this.searchedInstances, ...instances]
         this.searching = false
         this.searchNotFound = Boolean(this.searchedInstances.length === 0)
-        if (instances.length < chunkSize) {
+        if (instances.length < chunkCount) {
           this.searchChunksLoading = false
         }
         return loadNextChunk(instances[instances.length - 1].id)
@@ -226,11 +231,11 @@ class InstanceStore {
     loadNextChunk()
   }
 
-  @action reloadInstances(endpointId: string, chunkSize?: number) {
+  @action reloadInstances(endpoint: Endpoint, chunkSize?: number) {
     this.searchNotFound = false
     this.searchText = ''
     this.currentPage = 1
-    this.loadInstancesInChunks(endpointId, chunkSize, true)
+    this.loadInstancesInChunks(endpoint, chunkSize, true)
   }
 
   @action cancelIntancesChunksLoading() {
@@ -245,9 +250,9 @@ class InstanceStore {
     this.currentPage = page
   }
 
-  @action updateChunkSize(chunkSize: number) {
+  @action updateInstancesPerPage(instancesPerPage: number) {
     this.currentPage = 1
-    this.chunkSize = chunkSize
+    this.instancesPerPage = instancesPerPage
   }
 
   @action loadInstancesDetails(endpointId: string, instancesInfo: Instance[], useLocalStorage?: boolean, quietError?: boolean): Promise<void> {