From ba96ce733701703d11443506473ac7c301454a51 Mon Sep 17 00:00:00 2001 From: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com> Date: Wed, 7 Oct 2020 13:25:25 +0200 Subject: [PATCH] Groupe accounts by network and sort by name, path, creation date (#481) * sort by name * compare by path * fixed and added tests * sort by network first * add comment * use localCompare * use the same sorting name - path - creation everywhere * Apply suggestions from code review Co-authored-by: Jaco Greeff * Apply suggestions from code review Co-authored-by: Jaco Greeff --- .../src/util/buildHierarchy.test.ts | 47 +++++++++++++++++-- .../extension-ui/src/util/buildHierarchy.ts | 47 +++++++++++++++++-- .../extension-ui/src/util/getNetworkMap.ts | 14 ++++++ 3 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 packages/extension-ui/src/util/getNetworkMap.ts diff --git a/packages/extension-ui/src/util/buildHierarchy.test.ts b/packages/extension-ui/src/util/buildHierarchy.test.ts index 094b868e..c804373b 100644 --- a/packages/extension-ui/src/util/buildHierarchy.test.ts +++ b/packages/extension-ui/src/util/buildHierarchy.test.ts @@ -4,18 +4,27 @@ import { AccountJson, AccountWithChildren } from '@polkadot/extension-base/background/types'; import { buildHierarchy } from './buildHierarchy'; +const genesisExample = { + KUSAMA: '0xb0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe', + POLKADOT: '0x91b171bb158e2d3848fa23a9f1c25182fb8e20313b2c1eb49219da7a70ce90c3' +}; + const testHierarchy = (accounts: AccountJson[], expected: AccountWithChildren[]): void => { expect(buildHierarchy(accounts)).toEqual(expected); }; describe('Use Account Hierarchy', () => { - const acc = (address: string, parentAddress?: string, whenCreated?: number): { + const acc = (address: string, parentAddress?: string, whenCreated?: number, name?: string, suri?: string): { address: string; - whenCreated?: number; + name?: string; parentAddress?: string; + suri?: string; + whenCreated?: number; } => ({ address, + name, parentAddress, + suri, whenCreated }); @@ -56,10 +65,38 @@ describe('Use Account Hierarchy', () => { }]); }); - test('sorts accounts by creation time', () => { + test('sorts accounts by network', () => { testHierarchy( - [acc('a', undefined, 2), acc('b', 'a', 4), acc('c', 'a', 3), acc('d', undefined, 1)], - [acc('d', undefined, 1), { address: 'a', children: [acc('c', 'a', 3), acc('b', 'a', 4)], whenCreated: 2 }] + [{ address: 'b', genesisHash: genesisExample.KUSAMA }, { address: 'a', genesisHash: genesisExample.POLKADOT }, { address: 'c', genesisHash: genesisExample.KUSAMA }], + [{ address: 'b', genesisHash: genesisExample.KUSAMA }, { address: 'c', genesisHash: genesisExample.KUSAMA }, { address: 'a', genesisHash: genesisExample.POLKADOT }] + ); + }); + + test('sorts accounts by network and name', () => { + testHierarchy( + [{ address: 'b', genesisHash: genesisExample.KUSAMA, name: 'b-last-kusama' }, { address: 'a', genesisHash: genesisExample.POLKADOT }, { address: 'c', genesisHash: genesisExample.KUSAMA, name: 'a-first-kusama' }], + [{ address: 'c', genesisHash: genesisExample.KUSAMA, name: 'a-first-kusama' }, { address: 'b', genesisHash: genesisExample.KUSAMA, name: 'b-last-kusama' }, { address: 'a', genesisHash: genesisExample.POLKADOT }] + ); + }); + + test('sorts accounts by name and creation date', () => { + testHierarchy( + [acc('b', undefined, 2, 'b'), acc('z', undefined, 1, 'b'), acc('a', undefined, 4, 'a')], + [{ address: 'a', name: 'a', whenCreated: 4 }, { address: 'z', name: 'b', whenCreated: 1 }, { address: 'b', name: 'b', whenCreated: 2 }] + ); + }); + + test('sorts account children by name and path', () => { + testHierarchy( + [acc('a', undefined, 1, 'a'), acc('b', 'a', 1, 'b', '/2'), acc('b', 'a', 1, 'b', '/0')], + [{ address: 'a', children: [acc('b', 'a', 1, 'b', '/0'), acc('b', 'a', 1, 'b', '/2')], name: 'a', whenCreated: 1 }] + ); + }); + + test('sorts accounts with children by name and creation date', () => { + testHierarchy( + [acc('b', undefined, 2, 'b'), acc('z', undefined, 1, 'b'), acc('d', 'b', 2, 'd'), acc('c', 'b', 3, 'c'), acc('a', undefined, 4, 'a')], + [{ address: 'a', name: 'a', whenCreated: 4 }, { address: 'z', name: 'b', whenCreated: 1 }, { address: 'b', children: [acc('c', 'b', 3, 'c'), acc('d', 'b', 2, 'd')], name: 'b', whenCreated: 2 }] ); }); diff --git a/packages/extension-ui/src/util/buildHierarchy.ts b/packages/extension-ui/src/util/buildHierarchy.ts index 234ed6bb..05fc26d7 100644 --- a/packages/extension-ui/src/util/buildHierarchy.ts +++ b/packages/extension-ui/src/util/buildHierarchy.ts @@ -3,18 +3,58 @@ import { AccountJson, AccountWithChildren } from '@polkadot/extension-base/background/types'; +import getNetworkMap from './getNetworkMap'; + type ChildFilter = (account: AccountJson) => AccountWithChildren; -function compareByCreationTime (a: AccountJson, b: AccountJson): number { +function compareByCreation (a: AccountJson, b: AccountJson): number { return (a.whenCreated || Infinity) - (b.whenCreated || Infinity); } +function compareByName (a: AccountJson, b: AccountJson): number { + const nameA = a.name?.toUpperCase() || ''; + const nameB = b.name?.toUpperCase() || ''; + + return nameA.localeCompare(nameB); +} + +function compareByPath (a: AccountJson, b: AccountJson): number { + const suriA = a.suri?.toUpperCase() || ''; + const suriB = b.suri?.toUpperCase() || ''; + + return suriA.localeCompare(suriB); +} + +function compareByNetwork (a: AccountJson, b: AccountJson): number { + const networkMap = getNetworkMap(); + const networkA = networkMap.get(a?.genesisHash || '') || ''; + const networkB = networkMap.get(b?.genesisHash || '') || ''; + + return networkA.localeCompare(networkB); +} + +function compareByPathThenCreation (a: AccountJson, b: AccountJson): number { + // if the paths are equal, compare by creation time + return compareByPath(a, b) || compareByCreation(a, b); +} + +function compareByNameThenPathThenCreation (a: AccountJson, b: AccountJson): number { + // This comparison happens after an initial sorting by network. + // if the 2 accounts are from different networks, don't touch their order + if (a.genesisHash !== b.genesisHash) { + return 0; + } + + // if the names are equal, compare by path then creation time + return compareByName(a, b) || compareByPathThenCreation(a, b); +} + export function accountWithChildren (accounts: AccountJson[]): ChildFilter { return (account: AccountJson): AccountWithChildren => { const children = accounts .filter(({ parentAddress }) => account.address === parentAddress) .map(accountWithChildren(accounts)) - .sort(compareByCreationTime); + .sort(compareByNameThenPathThenCreation); return children.length === 0 ? account @@ -31,5 +71,6 @@ export function buildHierarchy (accounts: AccountJson[]): AccountWithChildren[] !accounts.some(({ address }) => parentAddress === address) ) .map(accountWithChildren(accounts)) - .sort(compareByCreationTime); + .sort(compareByNetwork) + .sort(compareByNameThenPathThenCreation); } diff --git a/packages/extension-ui/src/util/getNetworkMap.ts b/packages/extension-ui/src/util/getNetworkMap.ts new file mode 100644 index 00000000..9729fc33 --- /dev/null +++ b/packages/extension-ui/src/util/getNetworkMap.ts @@ -0,0 +1,14 @@ +// Copyright 2019-2020 @polkadot/extension-ui authors & contributors +// SPDX-License-Identifier: Apache-2.0 + +import chains from '@polkadot/extension-chains/chains'; + +export default function getNetworkMap (): Map { + const res = new Map(); + + chains.forEach((chain) => { + res.set(chain.genesisHash, chain.chain); + }); + + return res; +}