-
Notifications
You must be signed in to change notification settings - Fork 17
feat: allow pass create and delete DB funcs to UI #2087
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "create-database": "Create database", | ||
| "remove": "Remove" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import {registerKeysets} from '../../../utils/i18n'; | ||
|
|
||
| import en from './en.json'; | ||
|
|
||
| const COMPONENT = 'ydb-tenants-table'; | ||
|
|
||
| export default registerKeysets(COMPONENT, {en}); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,13 @@ | ||
| import type {Capability, SecuritySetting} from '../../../types/api/capabilities'; | ||
| import type {Capability, MetaCapability, SecuritySetting} from '../../../types/api/capabilities'; | ||
| import {useTypedSelector} from '../../../utils/hooks'; | ||
| import {useDatabaseFromQuery} from '../../../utils/hooks/useDatabaseFromQuery'; | ||
|
|
||
| import { | ||
| capabilitiesApi, | ||
| selectCapabilityVersion, | ||
| selectDatabaseCapabilities, | ||
| selectMetaCapabilities, | ||
| selectMetaCapabilityVersion, | ||
| selectSecuritySetting, | ||
| } from './capabilities'; | ||
|
|
||
|
|
@@ -87,3 +89,25 @@ export const useClusterWithoutAuthInUI = () => { | |
| export const useLoginWithDatabase = () => { | ||
| return useGetSecuritySetting('DomainLoginOnly') === false; | ||
| }; | ||
|
|
||
| export function useMetaCapabilitiesQuery() { | ||
| capabilitiesApi.useGetMetaCapabilitiesQuery({}); | ||
| } | ||
|
|
||
| export function useMetaCapabilitiesLoaded() { | ||
| const {data, error} = useTypedSelector(selectMetaCapabilities); | ||
|
|
||
| return Boolean(data || error); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as for me its not that obvious that Loaded is true when we receive error There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For capabilities queries error is a valid state and it means that there is no capability endpoint in current cluster / meta cluster. In that case all capabilities considered not available (version 0) The alternative for this code (return getMetaCapabilities: build.query({
queryFn: async () => {
try {
if (!window.api.meta) {
throw new Error('Method is not implemented.');
}
const data = await window.api.meta.getMetaCapabilities();
return {data};
} catch {
// If capabilities endpoint is not available, there will be an error
// That means no new features are available
return {data: {}};
}
},
}),What do you think, will it be more clear? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to handle real errors from this endpoint? I mean now HTTP 500 and non-existent state are the same data: {} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so. In case of error we don't know anything about cluster capabilities, so it's much safer to consider everything disabled. If response is 500, we can only retry, but we do not retry requests currently There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as for me I would split loaded and availability state like isMetaCapabilitiesAvailable=Boolean(window.api.meta.getMetaCapabilities) loading={!isCapabilitiesLoaded || (isMetaCapabilitiesAvailable && !isMetaCapabilitiesLoaded)} because when I see just !isMetaCapabilitiesLoaded I am expecting for it to be loaded but thats just me and just readability issue There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want to add additional code, but added some comments, I hope they will help |
||
| } | ||
|
|
||
| const useGetMetaFeatureVersion = (feature: MetaCapability) => { | ||
| return useTypedSelector((state) => selectMetaCapabilityVersion(state, feature) || 0); | ||
| }; | ||
|
|
||
| export const useCreateDatabaseFeatureAvailable = () => { | ||
| return useGetMetaFeatureVersion('/meta/create_database') >= 1; | ||
| }; | ||
|
|
||
| export const useDeleteDatabaseFeatureAvailable = () => { | ||
| return useGetMetaFeatureVersion('/meta/delete_database') >= 1; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| export interface UIFactory { | ||
| onCreateDB?: HandleCreateDB; | ||
| onDeleteDB?: HandleDeleteDB; | ||
| } | ||
|
|
||
| export type HandleCreateDB = (params: {clusterName: string}) => Promise<boolean>; | ||
|
|
||
| export type HandleDeleteDB = (params: { | ||
| clusterName: string; | ||
| databaseName: string; | ||
| databaseId: string; | ||
| }) => Promise<boolean>; |
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.
Did not dig deep but looks like if there is no meta metaCapabilitiesLoaded will always be falsy and loading will always be true?