-
Notifications
You must be signed in to change notification settings - Fork 1
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
[ABW-3465] Decode deep link and download metadata #180
Conversation
* wip callback_path fetching * pr review changes * add missing callback_path file * pr review refactors * add documentation for callback_path * use callback_path constructor when initializing * refactor to satisfy pr review --------- Co-authored-by: Ghenadie Vasiliev-Pusca <ghenadie.vasiliev-pusca@rdx.works>
to test the current state of POC
This reverts commit 3ea7600.
…models-from-sargon' into rcm_linking
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #180 +/- ##
=======================================
- Coverage 98.6% 98.4% -0.3%
=======================================
Files 765 777 +12
Lines 11747 11915 +168
Branches 38 39 +1
=======================================
+ Hits 11590 11726 +136
- Misses 153 185 +32
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
fixtures/models/gateway/state/response_entity_details__single_account_no_assets.json
Outdated
Show resolved
Hide resolved
src/gateway_api/models/types/response/state/entity/details/metadata/typed_value.rs
Show resolved
Hide resolved
src/wallet/home_cards/deferred_deep_link/deferred_deep_link_special_dapp.rs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice,
Well done!
src/wallet/home_cards/manager.rs
Outdated
@@ -76,16 +83,33 @@ impl HomeCardsManager { | |||
} | |||
|
|||
/// Handles a deferred deep link by parsing it and saving the generated `HomeCards` to `HomeCardsStorage`. | |||
/// `HomeCard::ContinueRadQuest` if found in the link parsing result, replaces `HomeCard::StartRadQuest` if it's stored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, this does not seem to be possible - by definition this function should be called only after user did install the wallet, so there cannot be StartRadQuest
stored.
Rather, it is the other way around - in wallet_created
, we should not add the HomeCard::StartRadQuest
if HomeCard::ContinueRadQuest
is already present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. What about keeping this logic in both methods to make sure there is no chance both StartRadQuest
and ContinueRadQuest
are present at the same time? This way Sargon won't care about the order of the method calls. Also, for the deep link to be resolved the AF SDK performs a network request, then we perform a network request for the dApp icon URL, so there is a tiny possibility the deep link won't be handled fast enough. Keeping this logic also would prevent any scenarios like this.
Changelog
Adds the
DeferredDeepLinkParser
that will be responsible for decoding the information sent by a deferred deep link. From such data, it will create the corresponding list ofHomeCards
that the host app need to display.Also, it will extend the API models to allow parsing the metadata when fetching an entity details. Such metadata will be use to retrieve the
icon_url
of adapp_referrer
.