Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EPIC] Update existing wallet models following the agreed architecture #14471

Open
2 tasks
noeliaSD opened this issue Apr 18, 2024 · 2 comments
Open
2 tasks

Comments

@noeliaSD
Copy link
Contributor

noeliaSD commented Apr 18, 2024

Description

According to the agreed architecture posted on the shared notion page, update the following identified wallet models following the pattern described.

Goals:

  • Unifying criteria across all desktop teams while defining / exposing models to the QML layer.
  • Have a unique source of truth in service layer with the most possible raw and generic data.
  • QML layer will be decomposed into (Store --> Adaptor --> Views) and will be the only one responsible of transforming data according to the UI needs.
  • Reduce NIM layer the most possible and use QML proxy models library instead.

This epic will be solved step by step, considering each model update as a different subtask. Here are the existing wallet models that should be part of the wallet section/module:

Exposition of context properties from NIM to QML

  • src/app/modules/main/wallet_section

    • accounts/model.nim --> walletSectionAccounts --> it seems to have basic model data structure
    • activity/model.nim
    • all_tokens // Assets
      • flat_tokens_model.nim:
        • Sources property it's not a direct value but an operation that can be a potential performance issue
        • Description and WebsiteURL also to be considered
        • MarketDetails can be lazy loaded?? Can we share the same market details object in the flat_tokens_model???
      • sources_of_tokens_model.nim --> ok
      • token_by_symbol_model.nim --> Needs the grouping model
    • assets/grouped_account_assets_model.nim --> Ok --> It's a mirror of the UI need so probably a flat model whenn grouping model is done***
    • buy_sell_crypto/model.nim
    • networks
      • model.nim
      • combined_model.nim (probably candidate to be removed, flat model should be enough with some transformations on top for particular views)
    • saved_addresses/model.nim
      • the question is if it could be merged with accounts model (because probably addresses from both models can be e.g. target of send operation)
    • send
      • We should consider moving dirty state management from NIM side to UI and therefore substantially simplify interdependence of both sides, also the SendModel as a result
      • accounts_model.nim --> walletSectionSend --> accounts --> It seems a subset of walletSectionAccounts. Can it be removed?
      • suggested_route_model.nim
      • network_model.nim
  • shared_modules

    • collectibles/controller.nim
    • collectible_details/controller.nim
    • keycard_popup/models/keycard_model.nim
  • profile_section/wallet/accounts/model.nim --> it's the one for settings -- > Can it be removed?

  • Collectibles model

    • Split into metadata / ownership models, to be joined QML-side
    • Account/Chain independent updates

Stores
Some stores are declared as Singletons:
status-desktop/ui/app/AppLayouts/Wallet/stores/RootStore.qml
status-desktop/ui/imports/shared/stores/RootStore.qml
It creates a dependency on context properties and breaks unit tests / storybook pages.

Subtasks:

  • Update accounts base model and remove duplications in (send and settings "sections")
  • In flat_tokens_model,Sources property it's not a direct value but an operation that can be a potential performance issue. Review Description and WebsiteURL
@noeliaSD
Copy link
Contributor Author

It still need proper decomposition of subtasks.

@noeliaSD
Copy link
Contributor Author

Some of the listed issues will be covered here:
#14618

Next steps:
#14715

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

1 participant