dbeaver/pro#7548 fixes "load more" button for main nodes#4408
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
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 => { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)) { |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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
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