-
Notifications
You must be signed in to change notification settings - Fork 107
feat(pci-block-storage): delete snapshots before retyping #19163
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: epic/TAPC-4377-volume-retyping
Are you sure you want to change the base?
feat(pci-block-storage): delete snapshots before retyping #19163
Conversation
…hots ref: #TAPC-4569 Signed-off-by: Adrien Turmo <[email protected]>
@@ -0,0 +1,5 @@ | |||
import { TVolumeSnapshot } from '@/api/data/snapshot'; | |||
|
|||
export const selectSnapshotForVolume = (volumeId: string) => ( |
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.
Why not simple function with two params ?
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.
So I can more easilly use it as a mapper in useVolumeSnapshots
export const useVolumeSnapshots = (projectId: string, volumeId: string) => {
const { data: volume } = useVolume(projectId, volumeId);
const select = useMemo(
() => (volume ? selectSnapshotForVolume(volumeId) : undefined),
[volume],
);
return useQuery({
queryKey: getVolumeSnapshotsQueryKey(projectId, volume?.region),
queryFn: () => getSnapshotsByRegion(projectId, volume?.region),
enabled: !!volume,
select,
});
};
})} | ||
className={clsx( | ||
displayFieldError && | ||
'bg-red-100 border-red-500 text-red-500 focus:text-red-500', |
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.
No need to focus text color cause you already have the color without focus
352e9bd
to
dbc2202
Compare
"pci_projects_project_storages_blocks_retype_detach_volume_label": "Entrer \"{{ confirmWord }}\" pour confirmer votre choix.", | ||
"pci_projects_project_storages_blocks_retype_detach_volume_confirm_button": "Confirmer et continuer", | ||
"pci_projects_project_storages_blocks_retype_detach_volume_error": "Une erreur s'est produite lors du détachement de l'instance. Nous vous invitons à vous rendre sur <0>la page block storage</0> afin de détacher l'instance manuellement" | ||
"pci_projects_project_storages_blocks_retype_detach_volume_error": "Une erreur s'est produite lors du détachement de l'instance. Nous vous invitons à vous rendre sur <0>la page block storage</0> afin de détacher l'instance manuellement.", |
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.
Can you use another than number tag ? I think they are no encoded very well when you request translation. Especially on fast pass
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.
I tried with but it doesn't work
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.
export const useVolumeSnapshots = (projectId: string, volumeId: string) => { | ||
const { data: volume } = useVolume(projectId, volumeId); | ||
|
||
const select = useMemo( |
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.
suggestions : How about exit this then the hook can take select as params and if tomorrow you want other use case it will more flexible
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.
No because it's integral to the logic of what we are expecting.
When I do "useVolumeSnapshots" i'm expecting to have the snapshots of the corresponding volum and not every snapshot I have (otherwise it would be just "useSnapshot").
So without the select the method doesn't make sense.
And at some point will we have a regionalized api that will do exactly that for us, and the select will go away.
packages/manager/apps/pci-block-storage/src/api/hooks/useSnapshot.ts
Outdated
Show resolved
Hide resolved
ref: #TAPC-4569 Signed-off-by: Adrien Turmo <[email protected]>
dbc2202
to
d7b7b2b
Compare
ref: #TAPC-4569