Skip to content

Commit

Permalink
Made the projects/namespaces list page paginated
Browse files Browse the repository at this point in the history
  • Loading branch information
codyrancher committed Dec 31, 2024
1 parent 3d1b497 commit 82c7602
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 35 deletions.
61 changes: 30 additions & 31 deletions shell/components/ExplorerProjectsNamespaces.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<script>
import { mapGetters } from 'vuex';
import ResourceTable, { defaultTableSortGenerationFn } from '@shell/components/ResourceTable';
import { STATE, AGE, NAME, NS_SNAPSHOT_QUOTA } from '@shell/config/table-headers';
import { uniq } from '@shell/utils/array';
import { MANAGEMENT, NAMESPACE, VIRTUAL_TYPES, HCI } from '@shell/config/types';
Expand All @@ -16,14 +15,15 @@ import { NAMESPACE_FILTER_ALL_ORPHANS } from '@shell/utils/namespace-filter';
import ResourceFetch from '@shell/mixins/resource-fetch';
import DOMPurify from 'dompurify';
import { HARVESTER_NAME as HARVESTER } from '@shell/config/features';
import PaginatedResourceTable from '@shell/components/PaginatedResourceTable.vue';
export default {
name: 'ListProjectNamespace',
components: {
ExtensionPanel,
Masthead,
MoveModal,
ResourceTable,
PaginatedResourceTable,
ButtonMultiAction,
},
mixins: [ResourceFetch],
Expand All @@ -46,18 +46,7 @@ export default {
this.harvesterResourceQuotaSchema = this.$store.getters[`${ inStore }/schemaFor`](HCI.RESOURCE_QUOTA);
this.schema = this.$store.getters[`${ inStore }/schemaFor`](NAMESPACE);
this.projectSchema = this.$store.getters[`management/schemaFor`](MANAGEMENT.PROJECT);
if ( !this.schema ) {
// clusterReady: When switching routes, it will cause clusterReady to change, causing itself to repeat rendering。
// this.$store.dispatch('loadingError', `Type ${ NAMESPACE } not found`);
return;
}
await this.$fetchType(NAMESPACE);

This comment has been minimized.

Copy link
@codyrancher

codyrancher Dec 31, 2024

Author Contributor

This should fetching should now be handled by the PaginatedResourceTable

this.projects = await this.$store.dispatch('management/findAll', { type: MANAGEMENT.PROJECT, opt: { force: true } });
},
data() {
return {
loadResources: [NAMESPACE],
Expand Down Expand Up @@ -276,6 +265,21 @@ export default {
}
},
methods: {
filterRowsApi(existing) {
// We should add sorting by the projectId so we can group our namespaces by projects without the namespaces interleaving.
// I can't do that right now because the server side sorting is broken on my instance
return { ...existing, sort: undefined };

This comment has been minimized.

Copy link
@codyrancher

codyrancher Dec 31, 2024

Author Contributor

If I leave any kind of sorting in the paginated requests right now I get a backend error.

Image

We also need to add the sorting by projectId otherwise we end up with duplicate project groups when paginating.
Image
Image

This comment has been minimized.

Copy link
@richard-cox

richard-cox Jan 2, 2025

Member

The group by stuff works on a similar pattern to the current way. The product can define resource config that overides the default 'if resource is namespaced show namespace group option'.

For SSP then the configuration needs to be updated to only point at properties that exist natively in the resource (rather than model). For namespaces this would be metadata.annotations["field.cattle.io/projectId"] (this would need to be included in the next request to index fields).

There's only one example of this for SSP (given there's not a lot of custom group lists) and that's for pods list and group by node -

configureType(POD, {
listGroups: [
...STEVE_LIST_GROUPS,
// Allow Pods to be grouped by node
{
icon: 'icon-cluster',
value: 'role',
field: 'spec.nodeName',
hideColumn: NODE_COL.name,
groupLabelKey: 'groupByNode',
tooltipKey: 'resourceTable.groupBy.node'
}
],
listGroupsWillOverride: true,
});

That somehow needs to work alongside the SSP-disabled world (ExplorerProjectNamespace's ResourceTable provides a custom name for the group by component, namespace model provides a groupByLabel which determines which project group it appears in).

Think this could work with something like the pods in nodes type config, but using the metadata.annotations["field.cattle.io/projectId"] value. However groups would be sorted on the project id rather than project name. Let's discuss before/at PR review time

},
async fetchSecondaryResources() {
if ( !this.schema ) {
return;
}
// await this.$fetchType(NAMESPACE);
this.projects = await this.$store.dispatch('management/findAll', { type: MANAGEMENT.PROJECT, opt: { force: true } });

This comment has been minimized.

Copy link
@richard-cox

richard-cox Jan 2, 2025

Member

we might want to only fetch project applicable to the page (there should be a fetchPageSEcondaryResources hook for that).

however... i'm not sure how many projects we're expected to scale up to

},
/**
* Get PSA HTML to be displayed in the tooltips
*/
Expand Down Expand Up @@ -368,18 +372,9 @@ export default {
return project?.description;
},
clearSelection() {
afterNamespaceMoved() {
this.$refs.table.clearSelection();
},
sortGenerationFn() {

This comment has been minimized.

Copy link
@codyrancher

codyrancher Dec 31, 2024

Author Contributor

This doesn't appear relevant in the paginated world.

// The sort generation function creates a unique value and is used to create a key including sort details.
// The unique key determines if the list is redrawn or a cached version is shown.
// Because we ensure the 'not in a project' group is there via a row, and timing issues, the unqiue key doesn't change
// after a namespace is removed... so the list won't update... so we need to inject a string to ensure the key is fresh
const base = defaultTableSortGenerationFn(this.schema, this.$store);
return base + (this.showMockNotInProjectGroup ? '-mock' : '');
this.$refs.table.refreshTableData();

This comment has been minimized.

Copy link
@codyrancher

codyrancher Dec 31, 2024

Author Contributor

I believe this can be removed once we add the new subscription mechanism.

Right now there appears to be errors that occur when we move a namespace from one project to another project and we end up with a empty rows until we refresh the data.

Image

I haven't dug into it yet.

},
}
Expand Down Expand Up @@ -418,16 +413,20 @@ export default {
:type="extensionType"
:location="extensionLocation"
/>
<ResourceTable
<PaginatedResourceTable
ref="table"
v-bind="{...$attrs, class: null }"
class="table project-namespaces-table"
:schema="schema"
:headers="headers"
:rows="filteredRows"
:pagination-headers="headers"
context="projectnamespaces"
:api-filter="filterRowsApi"
:namespaced="false"
:groupable="true"
:sort-generation-fn="sortGenerationFn"
:loading="loading"
manualRefreshButtonSize="sm"
:fetchSecondaryResources="fetchSecondaryResources"
:fetchPageSecondaryResources="fetchSecondaryResources"
group-tooltip="resourceTable.groupBy.project"
key-field="_key"
>
Expand Down Expand Up @@ -524,8 +523,8 @@ export default {
</td>
</tr>
</template>
</ResourceTable>
<MoveModal @moving="clearSelection" />
</PaginatedResourceTable>
<MoveModal @moving="afterNamespaceMoved" />
</div>
</template>
<style lang="scss" scoped>
Expand Down
20 changes: 20 additions & 0 deletions shell/components/PaginatedResourceTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ export default defineComponent({
default: null, // Automatic from schema
},
/**

This comment has been minimized.

Copy link
@codyrancher

codyrancher Dec 31, 2024

Author Contributor

All changes in this file are to forward/expose features that project/namespaces used with ResourceTable.

* This just forwards the prop to the resource table. It allows you to pass a string's key which will show the referenced string as the group by tooltip
*/
groupTooltip: {
type: String,
default: null, // Automatic from schema
},
/**
* Information may be required from resources other than the primary one shown per row
*
Expand Down Expand Up @@ -98,6 +106,16 @@ export default defineComponent({
return customHeaders || this.$store.getters['type-map/headersFor'](this.schema, this.canPaginate);
}
},
methods: {
refreshTableData() {
this.$refs.table.refreshTableData();
},
clearSelection() {
this.$refs.table.clearSelection();
}
}
});
Expand All @@ -106,12 +124,14 @@ export default defineComponent({
<template>
<div>
<ResourceTable
ref="table"
v-bind="$attrs"
:schema="schema"
:rows="rows"
:alt-loading="canPaginate"
:loading="loading"
:groupable="groupable"
:group-tooltip="groupTooltip"

:headers="safeHeaders"
:namespaced="namespaced"
Expand Down
4 changes: 4 additions & 0 deletions shell/components/ResourceTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,10 @@ export default {
},
methods: {
refreshTableData() {
this.$refs.table.refreshTableData();
},
keyAction(action) {
const table = this.$refs.table;
Expand Down
2 changes: 1 addition & 1 deletion shell/config/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export const DEFAULT_PERF_SETTING: PerfSettings = {
resources: {
enableAll: false,
enableSome: {
enabled: ['configmap', 'secret', 'pod', 'node'],
enabled: ['configmap', 'secret', 'pod', 'node', 'namespace'],
generic: true,
}
}
Expand Down
4 changes: 2 additions & 2 deletions shell/mixins/resource-fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,11 @@ export default {
const opt = {
hasManualRefresh: this.hasManualRefresh,
pagination: { ...this.pagination },
force: this.paginating !== null // Fix for manual refresh (before ripped out).
force: true
};

if (this.apiFilter) {
opt.paginating = this.apiFilter(opt.pagination);
opt.pagination = this.apiFilter(opt.pagination);

This comment has been minimized.

Copy link
@codyrancher

codyrancher Dec 31, 2024

Author Contributor

I believe this was the intended code. Correct me if I'm wrong.

This comment has been minimized.

Copy link
@richard-cox

richard-cox Jan 2, 2025

Member

Would need to dive into the code, but there was something odd around this when manual refresh was enabled and ssp disable. I'll give it a prod at review time

}

this['paginating'] = true;
Expand Down
6 changes: 5 additions & 1 deletion shell/utils/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,5 +104,9 @@ export const getPerformanceSetting = (rootGetters: Record<string, (arg0: string,
// Start with the default and overwrite the values from the setting - ensures we have defaults for newly added options
const safeDefaults = Object.assign({}, DEFAULT_PERF_SETTING);

return Object.assign(safeDefaults, perfSetting || {});
const r = Object.assign(safeDefaults, perfSetting || {});

r.serverPagination.stores.cluster.resources.enableSome.enabled.push('namespace');

This comment has been minimized.

Copy link
@codyrancher

codyrancher Dec 31, 2024

Author Contributor

I coulnd't request the namespace resource without making this addition to the our settings resource.

When viewing the api of the resource it looked like I couldn't modify it. I'm guessing we need a backend change to support other resources.

This comment has been minimized.

Copy link
@richard-cox

richard-cox Jan 2, 2025

Member

This is a process that needs addressing.

For enable SSP for a specific type it needs to be enabled via setting.

  1. Needs adding to the default settings in const DEFAULT_PERF_SETTING
  2. system SETTING needs resetting to contain the new default (in the second PR there's something to help this before a better approach is worked out). Basically requires deleting and then re-applying using dev UI

return r;
};

0 comments on commit 82c7602

Please sign in to comment.