Skip to content

Commit

Permalink
fix: rank featured Safe Apps first on dashboard (safe-global#4644)
Browse files Browse the repository at this point in the history
* fix: show featured Safe Apps first on dashboard

* fix: add test coverage

* fix: only return featured/pinned Safe Apps

* Remove unnecessary test
  • Loading branch information
iamacook authored Dec 16, 2024
1 parent 41a8ec9 commit d61021d
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ jest.mock('@safe-global/safe-gateway-typescript-sdk', () => ({
features: [],
socialProfiles: [],
developerWebsite: '',
featured: true,
},
{
id: 3,
Expand All @@ -40,6 +41,7 @@ jest.mock('@safe-global/safe-gateway-typescript-sdk', () => ({
features: [],
socialProfiles: [],
developerWebsite: '',
featured: true,
},
{
id: 14,
Expand All @@ -56,6 +58,7 @@ jest.mock('@safe-global/safe-gateway-typescript-sdk', () => ({
features: [],
socialProfiles: [],
developerWebsite: '',
featured: false,
},
{
id: 24,
Expand All @@ -73,6 +76,7 @@ jest.mock('@safe-global/safe-gateway-typescript-sdk', () => ({
features: [],
socialProfiles: [],
developerWebsite: '',
featured: true,
},
]),
}))
Expand Down Expand Up @@ -116,8 +120,7 @@ describe('Safe Apps Dashboard Section', () => {
expect(screen.getByText('Decentralised naming for wallets, websites, & more.')).toBeInTheDocument(),
)

await waitFor(() => expect(screen.getByText('Synthetix')).toBeInTheDocument())
await waitFor(() => expect(screen.getByText('Trade synthetic assets on Ethereum')).toBeInTheDocument())
// Synthetix is not displayed as it is not featured

await waitFor(() => expect(screen.getByText('Transaction Builder')).toBeInTheDocument())
await waitFor(() => expect(screen.getByText('A Safe app to compose custom transactions')).toBeInTheDocument())
Expand Down
16 changes: 9 additions & 7 deletions src/hooks/__tests__/useRankedSafeApps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,24 @@ describe('useRankedSafeApps', () => {
})

it('returns 5 safe apps at most', () => {
const mockSafeApp1 = getMockSafeApp({ id: 1 })
const mockSafeApp1 = getMockSafeApp({ id: 1, featured: true } as Partial<SafeAppData>)
const mockSafeApp2 = getMockSafeApp({ id: 2 })
const mockSafeApp3 = getMockSafeApp({ id: 3 })
const mockSafeApp4 = getMockSafeApp({ id: 4 })
const mockSafeApp5 = getMockSafeApp({ id: 5 })
const mockSafeApp6 = getMockSafeApp({ id: 6 })

const { result } = renderHook(() =>
useRankedSafeApps([mockSafeApp1, mockSafeApp2, mockSafeApp3, mockSafeApp4, mockSafeApp5, mockSafeApp6], []),
useRankedSafeApps([mockSafeApp1], [mockSafeApp2, mockSafeApp3, mockSafeApp4, mockSafeApp5, mockSafeApp6]),
)

expect(result.current.length).toEqual(5)
})

it('returns pinned apps first', () => {
it('returns featured, then pinned apps', () => {
const mockSafeApp1 = getMockSafeApp({ id: 1 })
const mockSafeApp2 = getMockSafeApp({ id: 2 })
const mockSafeApp3 = getMockSafeApp({ id: 3 })
const mockSafeApp2 = getMockSafeApp({ id: 2, featured: true } as Partial<SafeAppData>)
const mockSafeApp3 = getMockSafeApp({ id: 3, featured: true } as Partial<SafeAppData>)
const mockSafeApp4 = getMockSafeApp({ id: 4 })
const mockSafeApp5 = getMockSafeApp({ id: 5 })

Expand All @@ -48,7 +48,9 @@ describe('useRankedSafeApps', () => {
),
)

expect(result.current[0]).toStrictEqual(mockPinnedApp1)
expect(result.current[1]).toStrictEqual(mockPinnedApp2)
expect(result.current[0]).toStrictEqual(mockSafeApp2)
expect(result.current[1]).toStrictEqual(mockSafeApp3)
expect(result.current[2]).toStrictEqual(mockPinnedApp1)
expect(result.current[3]).toStrictEqual(mockPinnedApp2)
})
})
6 changes: 3 additions & 3 deletions src/hooks/safe-apps/useRankedSafeApps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ const useRankedSafeApps = (safeApps: SafeAppData[], pinnedSafeApps: SafeAppData[
return useMemo(() => {
if (!safeApps.length) return []

const mostUsedApps = rankSafeApps(safeApps)
// TODO: Remove assertion after migrating to new SDK
const featuredApps = safeApps.filter((app) => (app as SafeAppData & { featured: boolean }).featured)
const rankedPinnedApps = rankSafeApps(pinnedSafeApps)
const randomApps = safeApps.slice().sort(() => Math.random() - 0.5)

const allRankedApps = rankedPinnedApps.concat(pinnedSafeApps, mostUsedApps, randomApps)
const allRankedApps = featuredApps.concat(rankedPinnedApps, pinnedSafeApps)

// Use a Set to remove duplicates
return [...new Set(allRankedApps)].slice(0, NUMBER_OF_SAFE_APPS)
Expand Down

0 comments on commit d61021d

Please sign in to comment.