[LIVE-10952][Common] Refactoring of list apps #6055

ofreyssinet-ledger · 2024-01-31T10:32:58Z

📝 Description

Refactor list apps v2:
- move entrypoint of listApps to live-common/src/device/use-cases/listAppsUseCase.ts
- move more of the manager/api.ts logic to ManagerApiRepository
- create StubManagerApiRepository for mocks
- implement some unit tests for listApps/v2.ts
Implement getProviderIdUseCase that takes forceProvider: number as a parameter

❓ Context

JIRA or GitHub link: LIVE-10951 (tests) LIVE-10952 (refacto)

✅ Checklist

Pull Requests must pass the CI and be code reviewed. Set as Draft if the PR is not ready.

npx changeset was attached.
Covered by automatic tests.
Impact of the changes:
- My Ledger
- Firmware update flow

🧐 Checklist for the PR Reviewers

The code aligns with the requirements described in the linked JIRA or GitHub issue.
The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
There are no undocumented trade-offs, technical debt, or maintainability issues.
The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
Any new dependencies have been justified and documented.
Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

vercel · 2024-01-31T10:33:08Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
web-tools	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 7, 2024 2:04pm

4 Ignored Deployments

Name	Status	Preview	Updated (UTC)
ledger-live-docs	⬜️ Ignored (Inspect)	Visit Preview	Feb 7, 2024 2:04pm
ledger-live-github-bot	⬜️ Ignored (Inspect)	Visit Preview	Feb 7, 2024 2:04pm
native-ui-storybook	⬜️ Ignored (Inspect)	Visit Preview	Feb 7, 2024 2:04pm
react-ui-storybook	⬜️ Ignored (Inspect)	Visit Preview	Feb 7, 2024 2:04pm

jdabbech-ledger · 2024-02-02T08:39:49Z

libs/ledger-live-common/src/device-core/managerApi/use-cases/getProviderIdUseCase.ts

@@ -0,0 +1,23 @@
+import { DeviceInfoEntity } from "../entities/DeviceInfoEntity";
+
+export const PROVIDERS: Record<string, number> = {


could be an enum which also stores the default provider 1

Yes it could 👍 do you see an advantage of making it an enum ?
I will add the default provider regardless, good point!

actually using an enum breaks some tests because then you can do access stuff with an index like PROVIDERS[1] and then stuff like this behaves in an unwanted way:

const providerName = PROVIDERS[postDash] ? postDash : null;

cf. this test run https://github.com/LedgerHQ/ledger-live/actions/runs/7755719007/job/21151656301

Well the only advantage I see of making it an enum is the implicit typing.
I prefer using enum for readability and I was pretty sure that this behaviour was the same for a const or an enum.

Nevertheless LGTM 👍

it's not the same behaviour because enum[x] is a reverse mapping (it maps from value to name) https://www.typescriptlang.org/docs/handbook/enums.html#reverse-mappings
cf playground:

(I didn't know this playground by the way, nice one)

jdabbech-ledger

nice 👍

test(common/listApps/v2): more error cases

…ForDevice

chore: changeset

fix: imports fix: lint & bad import fix(llm): bad import

fix(getProviderIdUseCase): revert breaking change to an enum

fix(HttpManagerApiRepository): missing type + reword comment

live-github-bot · 2024-02-07T14:16:59Z

Desktop Bundle Checks

Comparing 095587b against 1975deb.

🚀 renderer bundle size decreased (36.1mb -> 35.9mb). Thanks ❤️

Mobile Bundle Checks

Comparing 095587b against 487845b.

gre

i see the creation of a new folder "device-core" in live-common, could you consider instead making a new library like "live-device-core" or even just "device-core" ?
we are taking a general direction to not add more logic into live-common but instead moving them out into dedicated library (e.g. live-countervalues, live-nft, ...)

by doing so, you can activate stricter version of typescript config that also help improving the quality. it's also a game changer for the decoupling of our stack

ofreyssinet-ledger · 2024-02-08T10:47:43Z

@gre yes that's the direction we're taking, we will probably do this in a subsequent PR

mcayuelas-ledger · 2024-02-08T10:55:14Z

libs/ledger-live-common/src/device-core/managerApi/repositories/ManagerApiRepository.ts

@@ -10,7 +13,7 @@ export interface ManagerApiRepository {
 userId: string;
 }): Promise<OsuFirmware | null | undefined>;

- fetchMcus(): Promise<any>;
+ fetchMcus(): Promise<any>; // TODO: type properly


Why not now?

[LIVE-10952][Common] Refactoring of list apps

github-actions bot assigned ofreyssinet-ledger Jan 31, 2024

live-github-bot bot added desktop Has changes in LLD mobile Has changes in LLM common Has changes in live-common cli labels Jan 31, 2024

vercel bot deployed to Preview – web-tools January 31, 2024 19:17 View deployment

vercel bot deployed to Preview – web-tools February 1, 2024 14:48 View deployment

vercel bot deployed to Preview – web-tools February 1, 2024 15:01 View deployment

vercel bot had a problem deploying to Preview – web-tools February 1, 2024 16:14 Failure

vercel bot deployed to Preview – web-tools February 1, 2024 16:38 View deployment

ofreyssinet-ledger marked this pull request as ready for review February 1, 2024 16:46

ofreyssinet-ledger requested review from a team as code owners February 1, 2024 16:46

vercel bot deployed to Preview – web-tools February 1, 2024 16:53 View deployment

jdabbech-ledger reviewed Feb 2, 2024

View reviewed changes

jdabbech-ledger previously approved these changes Feb 2, 2024

View reviewed changes

ofreyssinet-ledger dismissed jdabbech-ledger’s stale review via 9c2c71f February 2, 2024 09:24

vercel bot deployed to Preview – web-tools February 2, 2024 12:12 View deployment

vercel bot deployed to Preview – web-tools February 2, 2024 15:40 View deployment

jdabbech-ledger previously approved these changes Feb 2, 2024

View reviewed changes

ofreyssinet-ledger force-pushed the support/refacto-list-apps-v2 branch from 34830ce to 093cb1f Compare February 5, 2024 11:08

vercel bot deployed to Preview – web-tools February 5, 2024 11:12 View deployment

ofreyssinet-ledger dismissed jdabbech-ledger’s stale review via d9d5116 February 5, 2024 17:17

vercel bot deployed to Preview – web-tools February 5, 2024 17:21 View deployment

ofreyssinet-ledger force-pushed the support/refacto-list-apps-v2 branch from d9d5116 to 0cba9b2 Compare February 5, 2024 17:23

vercel bot had a problem deploying to Preview – web-tools February 5, 2024 17:25 Failure

vercel bot deployed to Preview – web-tools February 5, 2024 17:41 View deployment

ofreyssinet-ledger force-pushed the support/refacto-list-apps-v2 branch from 602f2df to 4f1371f Compare February 5, 2024 17:42

ofreyssinet-ledger added 16 commits February 7, 2024 14:52

refactor(common): add TODO, rename variables

f0a8a7d

test(common/listApps/v2): test basic error cases

088ea0f

test(common/listApps/v2): more error cases

refactor(ManagerApiRepository): add getAppsByHash

8e508eb

refactor(ManagerApiRepository): implement StubManagerApiRepository

f092831

refactor(listAppsV2): use managerApiRepository

425d15b

refactor(ManagerApiRepository): catalogForDevice, getLanguagePackages…

8bf97b1

…ForDevice

test(getLatestFirmwareForDevice): replace mocks by stub api repository

a2e90f0

test(listAppsV2): cleanup

7836e4f

refactor(mocks): move entities mocks to device-core

7921319

chore: changeset

9806dd6

chore: changeset

refactor(StubManagerApiRepository): replace mocks

606ffa5

chore: cleanup

bee690c

fix: lint & bad import

80ec450

fix: imports fix: lint & bad import fix(llm): bad import

refactor(common/getProviderIdUseCase): add a default value to PROVIDERS

de8d735

fix(getProviderIdUseCase): revert breaking change to an enum

fix(HttpManagerApiRepository): badly infered types for makeLRUCache

9b405f4

fix(HttpManagerApiRepository): missing type + reword comment

refactor(listAppsV1): pass managerApiRepository argument

095587b

ofreyssinet-ledger force-pushed the support/refacto-list-apps-v2 branch from 90d4ff7 to 095587b Compare February 7, 2024 13:58

vercel bot deployed to Preview – web-tools February 7, 2024 14:04 View deployment

gre reviewed Feb 8, 2024

View reviewed changes

valpinkman approved these changes Feb 8, 2024

View reviewed changes

mcayuelas-ledger approved these changes Feb 8, 2024

View reviewed changes

ghost approved these changes Feb 8, 2024

View reviewed changes

ofreyssinet-ledger added the HODL Do not merge label Feb 8, 2024

hedi-edelbloute approved these changes Feb 8, 2024

View reviewed changes

ofreyssinet-ledger merged commit a52838d into develop Feb 12, 2024
68 of 71 checks passed

ofreyssinet-ledger deleted the support/refacto-list-apps-v2 branch February 12, 2024 09:04

jdabbech-ledger pushed a commit that referenced this pull request Feb 19, 2024

Merge pull request #6055 from LedgerHQ/support/refacto-list-apps-v2

413e81f

[LIVE-10952][Common] Refactoring of list apps

jdabbech-ledger pushed a commit that referenced this pull request Feb 19, 2024

Merge pull request #6055 from LedgerHQ/support/refacto-list-apps-v2

54317dd

[LIVE-10952][Common] Refactoring of list apps

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LIVE-10952][Common] Refactoring of list apps #6055

[LIVE-10952][Common] Refactoring of list apps #6055

ofreyssinet-ledger commented Jan 31, 2024 •

edited

Loading

vercel bot commented Jan 31, 2024 •

edited

Loading

jdabbech-ledger Feb 2, 2024 •

edited

Loading

ofreyssinet-ledger Feb 2, 2024

ofreyssinet-ledger Feb 2, 2024

jdabbech-ledger Feb 2, 2024 •

edited

Loading

ofreyssinet-ledger Feb 6, 2024

jdabbech-ledger left a comment

live-github-bot bot commented Feb 7, 2024 •

edited

Loading

gre left a comment •

edited

Loading

ofreyssinet-ledger commented Feb 8, 2024

mcayuelas-ledger Feb 8, 2024

		@@ -0,0 +1,23 @@
		import { DeviceInfoEntity } from "../entities/DeviceInfoEntity";

		export const PROVIDERS: Record<string, number> = {

[LIVE-10952][Common] Refactoring of list apps #6055

[LIVE-10952][Common] Refactoring of list apps #6055

Conversation

ofreyssinet-ledger commented Jan 31, 2024 • edited Loading

📝 Description

❓ Context

✅ Checklist

🧐 Checklist for the PR Reviewers

vercel bot commented Jan 31, 2024 • edited Loading

jdabbech-ledger Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

ofreyssinet-ledger Feb 2, 2024

Choose a reason for hiding this comment

ofreyssinet-ledger Feb 2, 2024

Choose a reason for hiding this comment

jdabbech-ledger Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

ofreyssinet-ledger Feb 6, 2024

Choose a reason for hiding this comment

jdabbech-ledger left a comment

Choose a reason for hiding this comment

live-github-bot bot commented Feb 7, 2024 • edited Loading

Desktop Bundle Checks

Mobile Bundle Checks

gre left a comment • edited Loading

Choose a reason for hiding this comment

ofreyssinet-ledger commented Feb 8, 2024

mcayuelas-ledger Feb 8, 2024

Choose a reason for hiding this comment

ofreyssinet-ledger commented Jan 31, 2024 •

edited

Loading

vercel bot commented Jan 31, 2024 •

edited

Loading

jdabbech-ledger Feb 2, 2024 •

edited

Loading

jdabbech-ledger Feb 2, 2024 •

edited

Loading

live-github-bot bot commented Feb 7, 2024 •

edited

Loading

gre left a comment •

edited

Loading