- 
                Notifications
    
You must be signed in to change notification settings  - Fork 484
 
dbeaver/pro#6364 add option to set default view for project #3831
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.
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
base: devel
Are you sure you want to change the base?
dbeaver/pro#6364 add option to set default view for project #3831
Conversation
…view-for-project
| async deleteProjectSettings(projectId: string, settingIds: string[], subjectId?: string): Promise<void> { | ||
| if (subjectId) { | ||
| await this.graphQLService.sdk.adminDeleteUserProjectSettings({ | ||
| projectId, | ||
| subjectId, | ||
| settingIds, | ||
| }); | ||
| } else { | ||
| await this.graphQLService.sdk.deleteUserProjectSettings({ | ||
| projectId, | ||
| settingIds, | ||
| }); | ||
| } | ||
| 
               | 
          ||
| this.markOutdated(projectId); | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not in use yet. may be delete it?
| getProjectSettings(projectId: string, subjectId?: string) { | ||
| if (subjectId) { | ||
| return this.graphQLService.sdk.getAdminUserProjectSettings({ | ||
| projectId, | ||
| subjectId, | ||
| }); | ||
| } else { | ||
| return this.graphQLService.sdk.getUserProjectSettings({ | ||
| projectId, | ||
| }); | ||
| } | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it work for users?
I can see this in constructor which means this resource only available for an admin
sessionPermissionsResource.require(this, EAdminPermission.admin);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better to split into two resources
But user one is not gonna be in use, so maybe we just simplty delete the user part
…view-for-project
| rmUserProjectSettings(projectId: ID!): Object @since(version: "25.2.1") | ||
| 
               | 
          ||
| "Returns project settings that are specified for a subject" | ||
| rmAdminUserProjectSettings(projectId: ID!, subjectId: String!): Object @since(version: "25.2.3") | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe rmAdminDefaultProjectSettings?
| }); | ||
| } | ||
| 
               | 
          ||
| this.markOutdated(projectId); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to mark data as outdated, you can use returned information to update changed data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I don't see what we should update here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, this is because you don't cache project settings, so you have two options:
- remove 
markOutdatedbecauseRMProjecttype not changed so we don't need to reset cache for it - create separate resource for project settings
 
…view-for-project
… to compare two values with different types)
Closes https://github.com/dbeaver/pro/issues/6364