Skip to content

Commit 7fdb65c

Browse files
author
CsB-Polymesh
committed
updated code based on suggestions
1 parent 5352913 commit 7fdb65c

10 files changed

Lines changed: 92 additions & 45 deletions

File tree

src/layouts/SecondaryKeys/components/AddPermission/components/AssetPermissionSelector.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,9 @@ export const AssetPermissionSelector = ({
330330
return { error: '', assetId: trimmedValue };
331331
}
332332

333-
// Check if it could be a ticker (12 characters or less, alphanumeric with allowed symbols)
333+
// Check if it could be a ticker (12 characters or less, uppercase alphanumeric - Polymesh ticker format)
334334
const isTickerFormat =
335-
trimmedValue.length <= 12 && /^[A-Z0-9_\-./]+$/i.test(trimmedValue);
335+
trimmedValue.length <= 12 && /^[A-Z0-9]+$/.test(trimmedValue);
336336

337337
if (isTickerFormat) {
338338
// Try to fetch asset by ticker

src/layouts/SecondaryKeys/components/AddPermission/components/ExtrinsicPermissionSelector.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,8 @@ export const ExtrinsicPermissionSelector = ({
309309
checked={checked}
310310
ref={(checkboxInput: HTMLInputElement | null) => {
311311
if (checkboxInput) {
312+
// HTML checkbox `indeterminate` is not exposed as a React prop,
313+
// so we must set it directly on the DOM node in the ref callback.
312314
// eslint-disable-next-line no-param-reassign
313315
checkboxInput.indeterminate = indeterminate;
314316
}

src/layouts/SecondaryKeys/components/AddPermission/components/PermissionSummary.tsx

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { useContext, useMemo } from 'react';
22
import { AssetContext } from '~/context/AssetContext';
3+
import { deduplicateAssetsByID, formatAssetDisplay } from '../../../utils';
34
import {
45
SummarySection,
56
SummaryCard,
@@ -8,7 +9,6 @@ import {
89
SelectedItemsList,
910
SelectedItem,
1011
} from '../styles';
11-
import { formatAssetDisplay } from '../../../utils';
1212

1313
interface IPermissionSummaryProps {
1414
secondaryKey: string;
@@ -34,15 +34,11 @@ export const PermissionSummary = ({
3434
}: IPermissionSummaryProps) => {
3535
const { ownedAssets, managedAssets } = useContext(AssetContext);
3636

37-
// Combine all available assets
38-
const allAssets = useMemo(() => {
39-
const combined = [...ownedAssets, ...managedAssets];
40-
// Remove duplicates by ID
41-
return combined.filter(
42-
(asset, index, self) =>
43-
index === self.findIndex((a) => a.id === asset.id),
44-
);
45-
}, [ownedAssets, managedAssets]);
37+
// Combine all available assets and deduplicate
38+
const allAssets = useMemo(
39+
() => deduplicateAssetsByID(ownedAssets, managedAssets),
40+
[ownedAssets, managedAssets],
41+
);
4642

4743
const formatPermissionType = (
4844
type: 'Whole' | 'These' | 'Except' | 'None',

src/layouts/SecondaryKeys/components/AddPermission/components/ValidationError.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import styled from 'styled-components';
22

3+
/**
4+
* ValidationError component for displaying validation errors.
5+
* Uses hardcoded colors for now, but could be updated to use theme colors
6+
* if styled-components theme is passed through the component hierarchy.
7+
*/
38
export const ValidationError = styled.div`
49
padding: 12px;
510
margin-top: 12px;

src/layouts/SecondaryKeys/components/AddPermission/hooks.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export const useAddPermissionModal = (
4040
permissions.portfolios.type === 'These' &&
4141
permissions.portfolios.values.length === 0
4242
) {
43-
return 'At least one portfolio must be selected when using "Specific portfolios only" permission type';
43+
return VALIDATION_MESSAGES.PORTFOLIO_THESE_REQUIRED;
4444
}
4545
return '';
4646
}, [permissions.assets, permissions.transactions, permissions.portfolios]);

src/layouts/SecondaryKeys/components/NoSecondaryKeysView/styles.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export const StyledIcon = styled.div`
2121
margin-bottom: 16px;
2222
2323
& .icon {
24-
color: #ff2e72;
24+
color: ${({ theme }) => theme.colors.textPink};
2525
}
2626
`;
2727

src/layouts/SecondaryKeys/components/SecondaryKeyItem/index.tsx

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { PortfolioContext } from '~/context/PortfolioContext';
66
import { AssetContext } from '~/context/AssetContext';
77
import { formatBalance } from '~/helpers/formatters';
88
import { UI_CONSTANTS } from '../../uiConstants';
9-
import { formatAssetDisplay } from '../../utils';
9+
import { formatAssetDisplay, deduplicateAssetsByID } from '../../utils';
1010
import {
1111
StyledSecondaryKeyItem,
1212
StyledInfoWrapper,
@@ -88,15 +88,11 @@ export const SecondaryKeyItem = ({
8888
const { ownedAssets, managedAssets } = useContext(AssetContext);
8989
const [detailsExpanded, setDetailsExpanded] = useState(false);
9090

91-
// Combine all available assets
92-
const allAssets = useMemo(() => {
93-
const combined = [...ownedAssets, ...managedAssets];
94-
// Remove duplicates by ID
95-
return combined.filter(
96-
(asset, index, self) =>
97-
index === self.findIndex((a) => a.id === asset.id),
98-
);
99-
}, [ownedAssets, managedAssets]);
91+
// Combine all available assets and deduplicate
92+
const allAssets = useMemo(
93+
() => deduplicateAssetsByID(ownedAssets, managedAssets),
94+
[ownedAssets, managedAssets],
95+
);
10096

10197
// Memoize key metadata lookups
10298
const keyMeta = useMemo(

src/layouts/SecondaryKeys/index.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,15 @@ const SecondaryKeys = () => {
7979
);
8080
setSecondaryKeys(keysWithPermissions);
8181
} catch (error) {
82-
// Error is already handled by the transaction context
82+
// Error is already handled by the transaction context, but log here for debugging
83+
if (process.env.NODE_ENV === 'development') {
84+
// Log error in development to aid debugging while keeping production behavior unchanged
85+
// eslint-disable-next-line no-console
86+
console.error(
87+
'Failed to load secondary accounts in SecondaryKeys:',
88+
error,
89+
);
90+
}
8391
setSecondaryKeys([]);
8492
} finally {
8593
setLoading(false);
@@ -156,6 +164,11 @@ const SecondaryKeys = () => {
156164
);
157165
} catch (error) {
158166
// Error is already handled by the transaction context
167+
if (process.env.NODE_ENV === 'development') {
168+
// Log error in development to aid debugging while keeping production behavior unchanged
169+
// eslint-disable-next-line no-console
170+
console.error('Failed to remove secondary key permissions', error);
171+
}
159172
}
160173
},
161174
[identity, sdk, executeTransaction, refreshSecondaryKeys],

src/layouts/SecondaryKeys/uiConstants.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ export const VALIDATION_MESSAGES = {
99
'At least one extrinsic must be selected when using "Specific modules and/or methods" permission type',
1010
PORTFOLIO_EXCLUDE_REQUIRED:
1111
'At least one portfolio must be selected when using "Exclude" permission type',
12+
PORTFOLIO_THESE_REQUIRED:
13+
'At least one portfolio must be selected when using "Specific portfolios only" permission type',
1214
} as const;
1315

1416
export const UI_CONSTANTS = {
@@ -31,10 +33,10 @@ export const UI_CONSTANTS = {
3133
export const ERROR_STYLING = {
3234
padding: '12px',
3335
marginTop: '12px',
34-
backgroundColor: 'rgba(220, 53, 69, 0.1)',
35-
border: '1px solid rgba(220, 53, 69, 0.3)',
36+
backgroundColor: 'var(--error-background-color, rgba(220, 53, 69, 0.1))',
37+
border: 'var(--error-border-color, 1px solid rgba(220, 53, 69, 0.3))',
3638
borderRadius: '4px',
37-
color: '#dc3545',
39+
color: 'var(--error-text-color, #dc3545)',
3840
fontSize: '14px',
3941
} as const;
4042

src/layouts/SecondaryKeys/utils.ts

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,22 @@ export interface AssetDetails {
1212
ticker?: string;
1313
}
1414

15+
/**
16+
* Deduplicates assets by ID, keeping the first occurrence.
17+
* Useful when assets come from multiple sources (owned and managed).
18+
*/
19+
export const deduplicateAssetsByID = (
20+
ownedAssets: AssetDetails[] = [],
21+
managedAssets: AssetDetails[] = [],
22+
): AssetDetails[] => {
23+
const combined = [...ownedAssets, ...managedAssets];
24+
// Remove duplicates by ID, keeping first occurrence
25+
return combined.filter(
26+
(asset, index, self) =>
27+
index === self.findIndex((a) => a.id === asset.id),
28+
);
29+
};
30+
1531
/**
1632
* Formats an asset for display with priority: Name > Ticker > ID (Abbreviated)
1733
*/
@@ -258,6 +274,21 @@ export const createEmptyPermissions = () => ({
258274
},
259275
});
260276

277+
/**
278+
* Type guard for portfolio value validation
279+
*/
280+
const isPortfolioValueObject = (
281+
value: unknown,
282+
): value is { id: string; name?: string } => {
283+
if (!value || typeof value !== 'object') {
284+
return false;
285+
}
286+
287+
const candidate = value as { id?: unknown; name?: unknown };
288+
289+
return typeof candidate.id === 'string';
290+
};
291+
261292
/**
262293
* Converts SecondaryKeyData to PermissionFormData
263294
*/
@@ -266,35 +297,37 @@ export const convertKeyDataToFormData = (
266297
): IPermissionFormData => {
267298
// Handle portfolio values - convert from string array to object array if needed
268299
let portfolioValues: Array<{ id: string; name?: string }> = [];
269-
if (key.permissions.portfolios.values) {
270-
portfolioValues = key.permissions.portfolios.values.map((value) => {
271-
if (typeof value === 'object' && 'id' in value) {
272-
return value as { id: string; name?: string };
300+
301+
const portfolios = key.permissions?.portfolios;
302+
const rawPortfolioValues = portfolios?.values;
303+
304+
if (Array.isArray(rawPortfolioValues)) {
305+
portfolioValues = rawPortfolioValues.reduce<
306+
Array<{ id: string; name?: string }>
307+
>((acc, value) => {
308+
if (isPortfolioValueObject(value)) {
309+
acc.push({ id: value.id, name: value.name });
310+
} else if (typeof value === 'string') {
311+
// Legacy string format
312+
acc.push({ id: value, name: undefined });
273313
}
274-
// Legacy string format
275-
return { id: value as string, name: undefined };
276-
});
314+
315+
// Ignore any unexpected/invalid value types
316+
return acc;
317+
}, []);
277318
}
278319

279320
return {
280321
assets: {
281-
type: key.permissions.assets.type as
282-
| 'Whole'
283-
| 'These'
284-
| 'Except'
285-
| 'None',
322+
type: key.permissions.assets.type as EPermissionType,
286323
values: (key.permissions.assets.values as string[]) || [],
287324
},
288325
transactions: {
289326
type: key.permissions.transactions.type as 'Whole' | 'These' | 'None',
290327
values: key.permissions.transactions.values || [],
291328
},
292329
portfolios: {
293-
type: key.permissions.portfolios.type as
294-
| 'Whole'
295-
| 'These'
296-
| 'Except'
297-
| 'None',
330+
type: key.permissions.portfolios.type as EPermissionType,
298331
values: portfolioValues,
299332
},
300333
};

0 commit comments

Comments
 (0)