# Status Desktop Architecture Guide ## Introduction This document contains a series of architectural decisions, principles, and best practices. They are largely derived from [SOLID](https://en.wikipedia.org/wiki/SOLID), adapted to the needs of the project. Many of them are not applied in the current code, but there is a great need to change this and establish a clear application architecture and improve the quality of the code. The goal is to follow these guideline in new code and gradually fix existing codebase. The diagram depicting top-level architecture can be found [here](https://miro.com/app/board/uXjVKS3chcc=/). ## General architecture good practices - Singletons should be stateless. Any state in a singleton is a flaw from the application architecture point of view. This creates a number of problems. First, it causes unit tests to be interdependent, introducing hard to detect bugs. Another, even more important problem is the order of initialization. In code like this: ```qml Component.onCompleted: { Global.applicationWindow = this // BAD } ``` there is no guarantee that other components will not call `Global.applicationWindow` before this variable is initialized. The order of initialization of QML components and therefore calls to `Component.onCompleted` is undefined. - Singletons should not refer to the backend. The only layer that has access to the backend is the layer of stores. Components relying on singletons accessing the backend are hard to test. Backend references used in the singleton must be mocked, which is problematic and requires additional exposure of context properties from the singleton only for testing and storybook purposes, as in the example below. ```qml // Utils.qml (singleton) QtObject { property var mainModuleInst: typeof mainModule !== "undefined" ? mainModule : null property var sharedUrlsModuleInst: typeof sharedUrlsModule !== "undefined" ? sharedUrlsModule : null property var globalUtilsInst: typeof globalUtils !== "undefined" ? globalUtils : null } ``` The testing/storybook code also becomes complicated because it is important to ensure that a singleton is mocked before the tested component is instantiated. Stateless singleton also means not using components like `Settings` inside singletons. - The API of components should be well thought out. Expose dependencies, hide internal details. Ideally, it should be enough for a developer to read the public API of a given component to understand what is needed to use it correctly. - Examples of hidden dependencies: - using stateful singletons - components communicate to each other implicitly, dependency is not visible in their public API - communicating by calling signals on global objects - accessing backend via singletons - taking via public API much more than needed (e.g. component needs two properties from store, but the whole store is provided with tens of properties and methods). It is violation of `ISP` from `SOLID` - Examples of not hidden details: - component contains searchable list and exposes search string assuming that filtering will be done externally - Favor composition over parameterization and inheritance. Overly parameterized components are fragile and not easy to maintain. Big set of switches, altering the appearance and behavior of the component, introduces a lot of conditional statements in the internal implementation. Finally, it is difficult to assure that all combinations are valid or specify which ones are valid and desired. - Do not make assumptions about the context in which the component will be used. ```qml Item { anchors.fill: parent // Bad, will lead to warnings when used in a Layout // ... } ``` In most cases, it is a good idea to use `Control` (or more specialized component) as the base component. ```qml Control { contentItem: ColumnLayout { // ... } } ``` - Action signals (e.g. click on delegate) should not provide metadata from the model. Only `index` or unique `key` should be an argument. The necessary metadata should be fetched on the signal receiver side. ```qml signal collectibleClicked(int chainId, string contractAddress, string tokenId, string uid, int tokenType) // BAD signal collectibleClicked(var collectible) // BAD signal collectibleClicked(string key) // OK ``` Otherwise, the component unnecessarily bypasses some values from the model. Callers may also want to access roles which are not used for displaying. It leads to creating unnecessary requirements for the input model. - use consistently `key` as a unique identifier of the model, even if there are other roles with unique values. On the UI side, `key` should be used only for identification, with no other assumptions and usage. Thanks to that, content of that role can be freely changed on the backend with no implications to the UI side. Even if `address` is unique, a separate `key` role should be used (providing the same content as `address` role). Other good name for that role - `id` - does not fit well in qml environment as it is reserved keyword. Defining such role with `ListElement` in tests or Storybook would be not possible. - `model.rowCount()` should not be used in bindings. This call provides count only once when it is called, and the expression will not be re-evaluated when count is changed. Solution is to use the attached property `ModelCount` (`model.ModelCount.count`) context property instead. However, in signal handling, using `rowCount()` is fully correct and preferred over `ModelCount` attached property (because it does not create any additional attached object unnecesarily). - `model.count` should not be used on models taken from outside as a dependency. `count` property is not a part of the `QAbstractItemModel` interface. Proxy models and backend models may not have that property defined. - Objects holding the whole model's row should not be used in bindings. ```qml readonly property var selectedAccount: ModelUtils.get(store.accounts, 0) // BAD ``` Such objects may be deleted at any time, e.g. because of model reset. The expression will not be automatically re-evaluated to take the current first item. - Avoid `QML`'s dynamic scoping. Refer for details [here](https://doc.qt.io/qt-5/qtqml-documents-scope.html#component-instance-hierarchy). - Follow [C++ Core Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) in C++ code. ## Stores Store are an intermediate layer between the backend and the UI components. They expose backend's functionality grouped by domain (e.g. `TransactionStore`, `AssetsStore`, `CommunitiesStore`), without following the UI layouts, where one view can interact with multiple domains (e.g. communities and wallet's tokens). In other words, changes in the UI should not result in changes in the store layer. > [!NOTE] > The store layer will undergo significant structural changes, including removing singletons, > moving some code elsewhere, deduplication to expose a given backend functionality in only one > store, and leaving at most one `RootStore`. For this reason, the following points are more > about the target state than the current state. - Stores are a thin wrapper over context properties exposed from the backend, no additional logic should be there like e.g. data transformations done by proxy models. Stores are cut-off line in tests and `Storybook pages`. When additional logic is put there, it is not available in unit tests and `Storybook`. It leads to more complicated mocking and code duplication (same logic repeated in real stores and mocked ones). ```qml // WalletAssetsStore.qml QtObject { property LeftJoinModel groupedAccountAssetsModel: LeftJoinModel { leftModel: root.baseGroupedAccountAssetModel rightModel: _jointTokensBySymbolModel joinRole: "tokensKey" } // Bad - transformation should be kept in adaptor object, // only basic models should be exposed from store } ``` - Stores completely hide the backend's context properties and not expose them directly. It makes the contract between backend and frontend clear already from the perspective of the store. It should be enough for a UI developer to rely on the store's API without inspection of the nim code in order to discover what methods and properties are exposed from objects directly exposed to the UI from the backend via store. ```qml // WalletStore.qml QtObject { property var networksModuleInst: networksModule // BAD } ``` ```qml // WalletStore.qml QtObject { id: root readonly property bool isGoerliEnabled: networksModule.isGoerliEnabled signal chainIdFetchedForUrl(string url, int chainId) function foo() { return networksModule.foo() } Component.onCompleted: { networksModule.chainIdFetchedForUrl.connect(root.chainIdFetchedForUrl) } } ``` This additional layer causes that backend changes affect only this part of the UI code, which can be adjusted here even without changing the API of the store itself. It is also clear what exactly is exposed. - A single context property injected from the backend should be exposed only once in a single store. Bad: ```qml // SwapStore.qml QtObject { readonly property var flatNetworks: networksModule.flatNetworks } // CommunityTokensStore.qml QtObject { readonly property var flatNetworks: networksModule.flatNetworks } ``` Stores represent the state of the application but they are not aware how that state is rendered and interacted by UI components. There is no rule that a single component must take a single store with everything exposed what is needed for that component. On the contrary, the UI component should take all the stores (assuming that stores do not duplicate exposition) it needs as a dependency. Moving on, in most cases a UI component should only take the models/properties it needs and not the entire store. - Stores are not singletons (if an existing singleton store needs to be used, still can be taken by a component as an explicit dependency). When stores are singleton, UI components can access backend from arbitrary places, in an implicit way. Dependencies are harder to track, APIs are not clear as they could be and it causes other problems covered in the `general` section. ```qml isGoerliEnabled: root.rootStore.isGoerliEnabled // OK isGoerliEnabled: WalletStores.RootStore.isGoerliEnabled // Bad ``` - Exposed properties are read-only, state modification is done by methods. This approach makes the data flow unidirectional. UI always transforms and renders read-only data. UI requests any changes via methods (also called actions). Those actions may result in updates of one or more read-only properties/models. - Stores should be always typed, no need to use `var`. Thanks to overriding proper import paths, typed stores are not a problem for tests and Storybook pages. They can be freely mocked, with the type preserved. ```qml required property TransactionStore store // OK required property var store // Bad ``` ## Adaptors Adaptors are special type of `QML` components responsible for transforming backend's data. Thanks to isolating data (especially models) transformations in adaptors, views take possibly simple, ready to display data (models and other read-only properties). Adaptors are usually a composition of proxy models (from external library `SortFilterProxyModel` and from own library of proxy models: `ObjectProxyModel`, `LeftJoinModel`, `GroupingModel`, `ConcatModel`, `RenamingProxyModel`, `MovableModel`, `WritableModel`). Diagram depicting custom proxy models can be found [here](https://miro.com/app/board/uXjVKTuFYOU=/) - Adaptors are data-oriented and not tightly coupled to a specific view. Adaptors' names should reflect the transformation they do instead of following the naming of the UI component consuming them. - API of adaptors, similarly as for UI components, should be possibly explicit and simple. Passing the whole store is usually not a good idea because it is not clear which part of the store's API is really used by the adaptor. Taking plain models with well-specified expected roles is usually a better idea. - Output of one adaptor (usually model) can be an input of another adaptor. This structure should intuitively reflect the branches in the data flow in the application. ## Storybook ### Features `Storybook` is an internal tool that supports rapid development of components in isolation, outside the application. It is also a kind of catalog of components from which the application is built. It offers a number of functionalities that improve development: - hot reloading - import paths overloading and `stubs` to isolate from the backend - integrated test runner - figma integration - built-in visual components inspector - pages organized in groups - bunch of dedicated components like `GenericListView` to create pages quickly > [!NOTE] > The `StatusQ` library also contains this type of utility application, called `Sandbox`, > which is currently deprecated as it offers less functionality. ### Running `Storybook` is a `cmake` project and can be easily opened, compiled and run in `QtCreator` from the `storybook/CMakeLists.txt` file. ### Good practices - Single `page` instantiates a single component and only necessary mocks/test data and auxiliary controls interacting with component's API. It makes the pages simple, easy to maintain and directly presenting the API of a given component. - Storybook pages are valuable for all types of components - both basic ones (delegates, buttons) and more complex ones, doing complex flows like e.g. funds transfer. - Storybook (and unit tests) has a mechanism of so-called `stubs` to replace real stores with empty objects, but in a way preserving types. It is needed because in Storybook and unit tests backend's context properties used in real stores are not available. Store's stubs are intended to be empty QtObjects, actual mocking should be done within a page. This approach allows making the dependencies of the component truly visible and explicit in a Storybook page. An alternative approach would be to implement single, shared mocks for every store and share the among pages. But it leads to situation that given mock provides much more than is needed by a single UI component. As a consequence, it is not clear on which part of the store given component depends on. A separate thing is that UI components should not depend on the store at all unless it is really necessary (only top level components covering complex flows). - A component in Storybook should be functional. The state of a component's Storybook page quite accurately reflects the state of the component. Components with a well-designed API and no unnecessary dependencies are easy to instantiate in isolation in Storybook. Conversely, overly complex components are a nightmare in Storybook. - Use identified modules to import stores (using unquoted identifiers). It is required for the stubs mechanism to work properly. In practice imports like `import AppLayouts.Wallet.stores 1.0 as WalletStores` should be used instead of `import "./stores" as WalletStores`. Second version is relative import from the file system. As a consequence, the mechanism for overriding import paths for tests and Storybook's pages will not work. ## Unit tests The project has unit tests for both C++ and QML code. ### C++ unit tests C++ unit tests are located under `ui/StatusQ/tests`. They can be executed easily from QtCreator. Just open the `StatusQ` project in `QtCreator` (`status-desktop/ui/StatusQ/CMakeLists.txt`) and select the appropriate target (each test is a separate target). C++ unit tests are located under `ui/StatusQ/tests`. They can be executed easily from QtCreator or make target: - Open the `StatusQ` project in `QtCreator` (`status-desktop/ui/StatusQ/CMakeLists.txt`) and select the appropriate target (each test is a separate target). - Run the `run-statusq-tests` target, e.g. `make -j10 run-statusq-tests ARGS="-R ModelEntryTest"` ### QML unit tests QML tests are located under `storybook/qmlTests/tests` and `ui/StatusQ/tests`. The first location is recommended for all new tests because it provides integration with the `Storybook`. Unit tests operate without backend. Backend layer is cut-off using stubs mechanism in the same way as for pages in `Storybook`. QML tests can be executed by running `QmlTests` target of `Storybook` project (setup described in section regarding `Storybook` above) or from `QtCreator` where only single test or subset of tests can be selected easily. Another convenient option is to use the test runner in `Storybook`, which gives ability to automatically execute tests for currently opened page when changes are detected in the code, or on demand using the button in the bottom bar. `Storybook` discovers tests by naming convention (e.g. `SwapModalPage.qml` will run `tst_SwapModal.qml` tests). ### Unit tests recommendations - Unit tests should be independent and isolated. They should not depend on anything except the unit of code under test. It means that dependencies should be mocked and input data provided in the simplest possible form. If the component under test needs a model, ideally it should be provided as a simple `ListModel` instead of being created using additional components such as adapters or stores. Using an additional component introduces a dependency on it in the test, even if the component under test itself is independent of it. > [!NOTE] > Separately testing components that are used together in an application may lead to the > conclusion that it is uncertain whether they are compatible. However, the point is not to > not test it, but to do it in an appropriate test (usually in test of component using those > two components together). - Unit tests should test units. This statement is also true regarding subcomponents used in a component tested in a given unit test. Subcomponents should not be subject of testing in unit test (as they form a different unit), they should be subject of testing in their own unit test. The unit tests should verify if subcomponents are used in the intended way and simply assume that internally they work as expected. Respecting this rule will help keep unit tests smaller, better structured and easier to maintain. - Unit tests should execute very quickly. Relying on `wait(...)` calls slows down tests and is unreliable, may lead to flakey tests, therefore should be avoided. - Unit tests should not generate any warnings and debug logs. Expected warning from the component can be supressed by using `TestCase::ignoreWarning`. ## Code Style - QML code should be in-line with [QML Coding Conventions](https://doc.qt.io/qt-5/qml-codingconventions.html). - Top level component `id` should be always `root`. - Private properties should be hidden in a `d` object (`QtObject { id: d }`, or `QObject { id: d }`). - `objectName` property should not be specified for the top level component in a given file. Component may be used in various context and usually should have different name in every context to disambiguate when doing lookup in unit tests, squish tests or monitoring tool. - `qmldir`: entries should be sorted (`Alt+Shift+S` on selection in `QtCreator`). - Comments and documentation should cover parts which are not obvious, can be skipped where intention is clear. - A declarative implementation should be favoured over imperative code in QML in most cases. - Use curly brackets to make complex expressions easier to read: ```qml readonly property bool errorMode: popup.isLoading || !recipientInputLoader.ready ? false : errorType !== Constants.NoError || networkSelector.errorMode || !(amountToSendInput.inputNumberValid || d.isCollectiblesTransfer) // Hard to read ``` ```qml readonly property bool errorMode: { // Easier to read if (popup.isLoading || !recipientInputLoader.ready) return false return errorType !== Constants.NoError || networkSelector.errorMode || !(amountToSend.ready || d.isCollectiblesTransfer) } ``` ## Other - Cryptocurrency balances should be always handled as a big integer strings and converted to localized human-friendly form only for displaying. Some basic rationale is provided in [this ticket](https://github.com/status-im/status-desktop/issues/11376). ## Useful links - [Qt Quick Layouts Overview](https://doc.qt.io/qt-6/qtquicklayouts-overview.html) - [Customizing Qt Quick Controls](https://doc.qt.io/qt-6/qtquickcontrols-customize.html) - [Keyboard Focus in Qt Quick](https://doc.qt.io/qt-6/qtquick-input-focus.html) - [Important Concepts In Qt Quick - Positioning ](https://doc.qt.io/qt-6/qtquick-positioning-topic.html)