Skip to content

dbeaver/pro#7548 fixes "load more" button for main nodes#4408

Open
sergeyteleshev wants to merge 1 commit into
develfrom
7548-te-load-more-is-displayed-on-test1-te-instead-full-connectionscriptdataset-lists
Open

dbeaver/pro#7548 fixes "load more" button for main nodes#4408
sergeyteleshev wants to merge 1 commit into
develfrom
7548-te-load-more-is-displayed-on-test1-te-instead-full-connectionscriptdataset-lists

Conversation

@sergeyteleshev

@sergeyteleshev sergeyteleshev commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

closes https://github.com/dbeaver/pro/issues/7548

The pagination mechanism works perfectly fine. The main issue is that we have what I would call "main" nodes and their children.

Examples of "main" nodes include: node://, node://rm, node://dbvfs, node://projectId, etc. Basically, these are all nodes that contain the trees of child nodes displayed in the tree view.

The pagination mechanism, with a limit configurable in the settings, applies to all of these nodes.

The problem occurs during the initial page load. The application loads the "main" nodes using the configured pagination limit. For example, if the limit is set to 10 items per page and there are 100 projects in the TE product, only the first 10 projects are loaded on startup. A "Load more" button is then displayed.

However, the currently selected project may be outside the loaded range (for example, project #82 out of 100). In that case, the user has to click "Load more" repeatedly until the page containing the selected project is loaded.

This mechanism works well for non-main nodes (i.e., child nodes). However, I propose loading all "main" nodes during initialization so that the application is aware of all available root-level items from the start. We can still use "Load more" for loading additional information related to those "main" nodes.

In summary, this PR introduces eager loading of all "main" nodes when tree loading is triggered (for any tree: connections, scripts, datasets, cloud storage, etc.). All other nodes continue to be handled exactly as they were before this PR.

Additionally, I added several optimizations to the page-loading process to reduce the number of network requests. The overall traffic remains the same, and I haven't noticed any artifacts or unexpected behavior during testing.

P.S. I will also add comments to the PR where certain parts may be unclear, to better explain the reasoning behind my decisions

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric Results
Complexity 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

() => CachedMapAllKey,
);
this.projectInfoResource.onDataOutdated.addHandler(() => this.markTreeOutdated(resourceKeyList(this.keys)));
this.projectInfoResource.onDataOutdated.addHandler(data => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we preloaded all projects. If we change project we don't really want to update all project tree nodes info. We just want project we switched to, or projects we have changed. data is an array of projects

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise there are abused network with requests about all projects info


private async loadNodeChildren(parentPath: string, offset: number, limit: number): Promise<NavNodeChildrenQuery> {
async loadAllChildren(nodeId: string): Promise<void> {
if (!this.appAuthService.authenticated || this.isLoading(nodeId)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since resource is requires user to be authenticated I also added this logic here. otherwise we get erors on login screen
this.isLoading() - here stands for reducing requests amount. cause different trees in the app can trigger the same request

const pageInfo = navTreeResource.offsetPagination.getPageInfo(
CachedResourceOffsetPageKey(0, 0).setParent(CachedResourceOffsetPageTargetKey(nodeId)),
);
const hasMorePages = pageInfo && pageInfo.end === undefined;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also reduces the amount of requests and also it is more correct for the logic below cause it actually tries to get next page

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant