fix(Storage): fix disks view for degraded group #1930
Conversation
| const preparedPDisk = PDisk | ||
| ? prepareGroupsPDisk({...PDisk, NodeId: mergedVDiskData.NodeId}) | ||
| : undefined; | ||
| const preparedPDisk = prepareGroupsPDisk({...PDisk, NodeId: mergedVDiskData.NodeId}); |
There was a problem hiding this comment.
Add empty PDisk object even if there is no PDisk field
| @@ -89,7 +91,7 @@ function VDiskItem({ | |||
| const vDiskId = vDisk.StringifiedId; | |||
|
|
|||
| // show vdisks without AllocatedSize as having average width (#1433) | |||
| const minWidth = valueIsDefined(vDiskToShow.AllocatedSize) ? undefined : unavailableVDiskWidth; | |||
| const minWidth = isNumeric(vDiskToShow.AllocatedSize) ? undefined : unavailableVDiskWidth; | |||
There was a problem hiding this comment.
AllocatedSize could be numeric or NaN, but not undefined
| @@ -55,7 +57,7 @@ export function Disks({vDisks = [], viewContext}: DisksProps) { | |||
| <div className={b('pdisks-wrapper')}> | |||
| {vDisks?.map((vDisk, index) => ( | |||
| <PDiskItem | |||
| key={vDisk?.PDisk?.StringifiedId} | |||
| key={vDisk?.PDisk?.StringifiedId || index} | |||
There was a problem hiding this comment.
There could be no proper id, use index in such case. Array index will be always different from pdisk and vdisk ids, since ids include several numbers joined by dash (e.g. "1-1001")
There was a problem hiding this comment.
in which cases there may be no StringifiedId and why cant we always use index
I dont mean to rework anything - I just dont understand (:
There was a problem hiding this comment.
- If there is no data about PDisk, then there would be no id, so we use index to prevent undefined ids - with undefined ids there is a bug where new disks added on every rerender
- Generally it's not a good practice to use indexes as keys, since they could cause some unexpected behaviour if array is reordered or some elements added or deleted (and it's not recommended in react docs). So we use unique ids everywhere where it's possible. For the case of missing vdisks' pdisks there is no much difference, since they have no data and displayed the same way
| @@ -84,12 +85,21 @@ export function getStorageGroupsInitialEntitiesCount( | |||
| return DEFAULT_ENTITIES_COUNT; | |||
| } | |||
|
|
|||
| export function useVDisksWithDCMargins(vDisks: PreparedVDisk[] = []) { | |||
| function isErasureWithDifferentDC(erasure?: Erasure) { | |||
| return erasure === 'mirror-3-dc' || erasure === 'mirror-3of4'; | |||
There was a problem hiding this comment.
what do these magic constant strings mean?
comment would be nice (and may be move to actual constants)
| @@ -228,6 +228,8 @@ export interface TStoragePDisk { | |||
| Whiteboard?: TPDiskStateInfo; | |||
| } | |||
|
|
|||
| export type Erasure = 'none' | 'block-4-2' | 'mirror-3-dc' | 'mirror-3of4'; | |||
There was a problem hiding this comment.
maybe enum and use it in isErasureWithDifferentDC ?
There was a problem hiding this comment.
I don't like enums, you have to import them everywhere to satisfy TS, while for union string types every option will be suggested by autocomplete
artemmufazalov
force-pushed
the
1929-fix-broken-group
branch
from
b178fa4 to
cc4a5c9
Compare
Closes #1929
Stand with the example of degraded group: https://nda.ya.ru/t/H7kJOQl57BtQVV
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
😟 No changes in tests. 😕
Bundle Size: ✅
Current: 80.33 MB | Main: 80.32 MB
Diff: +1.20 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information