Skip to content

feat: set firebase events for merchant details - #925

Merged
HashEngineering merged 5 commits into
masterfrom
feature-explore-dash-firebase-merchant-details
Mar 31, 2022
Merged

feat: set firebase events for merchant details#925
HashEngineering merged 5 commits into
masterfrom
feature-explore-dash-firebase-merchant-details

Conversation

@ClaudeHangui

@ClaudeHangui ClaudeHangui commented Mar 28, 2022

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

We are required to record a set of events related to user interaction within the explore dash for merchant details

Related PR's and Dependencies

Screenshots / Videos

How Has This Been Tested?

  • QA (Mobile Team)

Checklist:

  • I have performed a self-review of my own code and added comments where necessary
  • I have added or updated relevant unit/integration/functional/e2e tests

@ClaudeHangui
ClaudeHangui requested a review from Syn-McJ March 28, 2022 18:26
@ClaudeHangui ClaudeHangui changed the title feat: set firebase events merchant details feat: set firebase events for merchant details Mar 28, 2022
private val binding = ItemDetailsViewBinding.inflate(LayoutInflater.from(context), this)

private var onSendDashClicked: (() -> Unit)? = null
private var onSendDashClicked: ((isPayingWithDash: Boolean) -> Unit)? = null

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are required to track only when the Pay with Dash button is clicked

@Syn-McJ Syn-McJ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but methods in the viewModel can be simplified to only have one general method, instead of one for each event.

AnalyticsConstants.ExploreDash.MERCHANT_DETAILS_PAY_WITH_DASH,
bundleOf()
)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it need to be a separate method for each event? Perhaps a trackEvent(name: String) method is sufficient, with the caller deciding which event name constant to provide.

Otherwise, half of the viewModel will consist of these methods.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Syn-McJ I set up a function to handle tracking in vm
please check it out !

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ClaudeHangui looks great!

@HashEngineering
HashEngineering merged commit 82526db into master Mar 31, 2022
@HashEngineering
HashEngineering deleted the feature-explore-dash-firebase-merchant-details branch July 20, 2022 23:20
bfoss765 added a commit that referenced this pull request Aug 11, 2026
…oled surface

dash-sdk-android 0.1.0-v41int18 -> v41int19 (platform#4329 pooled funding on
rust-dashcore 8f26456b: ALL_SPENDABLE account type through Rust/FFI/JNI/Kotlin,
sendToAddresses/buildSignedPayment defaults flipped; carries #931 dedup, #915,
#925 add_funding, #929 contact-account funding, #918 AddressState).

CoreSendAllNative migrates off the v1 split surface (#4323 deprecated
setFunding$sdk_release/buildSigned$sdk_release — the split is not
concurrency-safe): the drain now runs new -> addOutput ->
setSelectionStrategy(ALL) -> finalizeAtomic$sdk_release (atomic
select+reserve+sign under the wallet-manager lock) -> ManagedCoreWallet
.broadcastTransaction(FinalizedCoreTransaction).

The user-facing send-all now drains the pooled ALL_SPENDABLE set (BIP44 +
BIP32 + every DashPay receival account in ONE transaction); the CoinJoin
drain stays pinned to the COIN_JOIN account — de-mixing remains a separate,
explicit flow.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants