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

[FC] Fix Accessible data callout copy logic #6375

Merged
merged 10 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## XX.XX.XX - 2023-XX-XX

### Financial Connections
* [FIXED][6375](https://github.com/stripe/stripe-android/pull/6375) Fixed Accessible data call out texts

### PaymentSheet
* [FIXED][6366](https://github.com/stripe/stripe-android/pull/6366) Fixed an issue where the result couldn't be parsed in `PaymentSheetContract`.

Expand Down
2 changes: 2 additions & 0 deletions financial-connections/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
<string name="data_accessible_callout_no_business"><annotation bold="accessible_data">Data accessible to this business: </annotation>%1$s.<annotation clickable="learn_more"> Learn more</annotation></string>

Choose a reason for hiding this comment

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

This is debatable and likely doesn't cause an issue unless we do things like making the link underline, but the annotation itself contains a space:

<annotation clickable="learn_more"> Learn more</annotation>

so if annotation did underline, the space would have an underline

(again, likely not a legit issue, just calling it out in case there could be issues)

Copy link
Collaborator Author

@carlosmuvi-stripe carlosmuvi-stripe Mar 16, 2023

Choose a reason for hiding this comment

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

It's a good point, fixed it! e4133f5

<string name="data_accessible_callout_through_stripe"><annotation bold="accessible_data">Data accessible to %1$s: </annotation>%2$s through Stripe.<annotation clickable="learn_more"> Learn more</annotation></string>
<string name="data_accessible_callout_through_stripe_no_business"><annotation bold="accessible_data">Data accessible to this business: </annotation>%2$s through Stripe.<annotation clickable="learn_more"> Learn more</annotation></string>
<string name="data_accessible_callout_through_link"><annotation bold="accessible_data">Data accessible to %1$s: </annotation>%2$s through Link.<annotation clickable="learn_more"> Learn more</annotation></string>
<string name="data_accessible_callout_through_link_no_business"><annotation bold="accessible_data">Data accessible to this business: </annotation>%2$s through Link.<annotation clickable="learn_more"> Learn more</annotation></string>
<string name="data_accessible_type_accountdetails">account details</string>
<string name="data_accessible_type_balances">balances</string>
<string name="data_accessible_type_transactions">transactions</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ internal class AccountPickerStates : PreviewParameterProvider<AccountPickerState
FinancialConnectionsAccount.Permissions.TRANSACTIONS
),
isStripeDirect = true,
isNetworking = false,
dataPolicyUrl = ""
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import com.stripe.android.financialconnections.domain.PollAuthorizationSessionAc
import com.stripe.android.financialconnections.domain.SelectAccounts
import com.stripe.android.financialconnections.features.accountpicker.AccountPickerState.SelectionMode
import com.stripe.android.financialconnections.features.common.AccessibleDataCalloutModel
import com.stripe.android.financialconnections.features.consent.ConsentTextBuilder
import com.stripe.android.financialconnections.features.consent.FinancialConnectionsUrlResolver
import com.stripe.android.financialconnections.model.FinancialConnectionsInstitution
import com.stripe.android.financialconnections.model.FinancialConnectionsSessionManifest.Pane
import com.stripe.android.financialconnections.model.PartnerAccount
Expand Down Expand Up @@ -83,12 +81,7 @@ internal class AccountPickerViewModel @Inject constructor(
activeAuthSession.skipAccountSelection == true,
accounts = accounts,
selectionMode = if (manifest.singleAccount) SelectionMode.RADIO else SelectionMode.CHECKBOXES,
accessibleData = AccessibleDataCalloutModel(
businessName = ConsentTextBuilder.getBusinessName(manifest),
permissions = manifest.permissions,
isStripeDirect = manifest.isStripeDirect ?: false,
dataPolicyUrl = FinancialConnectionsUrlResolver.getDataPolicyUrl(manifest)
),
accessibleData = AccessibleDataCalloutModel.fromManifest(manifest),
kgaidis-stripe marked this conversation as resolved.
Show resolved Hide resolved
/**
* in the special case that this is single account and the institution would have
* skipped account selection but _didn't_ (because we still saw this), we should
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,21 @@ private fun AccessibleDataText(
val permissionsReadable = remember(model.permissions) { model.permissions.toStringRes() }
AnnotatedText(
text = TextResource.StringId(
value = when (model.isStripeDirect) {
true -> when (model.businessName) {
null -> R.string.data_accessible_callout_through_stripe_no_business
else -> R.string.data_accessible_callout_through_stripe
value = when {
model.isNetworking -> when (model.businessName) {
null -> R.string.data_accessible_callout_through_link_no_business
else -> R.string.data_accessible_callout_through_link
}

false -> when (model.businessName) {
model.isStripeDirect -> when (model.businessName) {

Choose a reason for hiding this comment

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

for iOS there is no conditional around businessName for isStripeDirect

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure if it can happen, but let's leave it just in case? 😄

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh! yeah this is wrong then. Updating it, thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

null -> R.string.data_accessible_callout_no_business
else -> R.string.data_accessible_callout
}

else -> when (model.businessName) {
null -> R.string.data_accessible_callout_through_stripe_no_business
else -> R.string.data_accessible_callout_through_stripe
}
},
args = listOfNotNull(
model.businessName,
Expand Down Expand Up @@ -221,6 +226,7 @@ internal data class AccessibleDataCalloutModel(
val businessName: String?,
val permissions: List<Permissions>,
val isStripeDirect: Boolean,
val isNetworking: Boolean,
val dataPolicyUrl: String
) {

Expand All @@ -229,6 +235,7 @@ internal data class AccessibleDataCalloutModel(
AccessibleDataCalloutModel(
businessName = ConsentTextBuilder.getBusinessName(manifest),
permissions = manifest.permissions,
isNetworking = manifest.isNetworkingUserFlow ?: false,
isStripeDirect = manifest.isStripeDirect ?: false,
dataPolicyUrl = FinancialConnectionsUrlResolver.getDataPolicyUrl(manifest)
)
Expand All @@ -249,7 +256,8 @@ internal fun AccessibleDataCalloutPreview() {
Permissions.TRANSACTIONS,
Permissions.ACCOUNT_NUMBERS
),
isStripeDirect = true,
isNetworking = false,
isStripeDirect = false,
dataPolicyUrl = ""
),
onLearnMoreClick = {}
Expand All @@ -271,62 +279,78 @@ internal fun AccessibleDataCalloutWithManyAccountsPreview() {
Permissions.OWNERSHIP,
Permissions.TRANSACTIONS
),
isStripeDirect = true,
isStripeDirect = false,
isNetworking = false,
dataPolicyUrl = ""
),
accounts = listOf(
PartnerAccount(
authorization = "Authorization",
institutionName = "Random bank",
category = FinancialConnectionsAccount.Category.CASH,
id = "id1",
name = "Account 1 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.CHECKING,
supportedPaymentMethodTypes = emptyList()
),
PartnerAccount(
authorization = "Authorization",
category = FinancialConnectionsAccount.Category.CASH,
id = "id2",
name = "Account 2 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.SAVINGS,
supportedPaymentMethodTypes = emptyList()
),
PartnerAccount(
authorization = "Authorization",
category = FinancialConnectionsAccount.Category.CASH,
id = "id3",
name = "Account 3 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.SAVINGS,
supportedPaymentMethodTypes = emptyList()
accounts = partnerAccountsForPreview(),
institution = FinancialConnectionsInstitution(
id = "id",
name = "name",
url = "url",
featured = true,
icon = null,
logo = null,
featuredOrder = null,
mobileHandoffCapable = false
),
onLearnMoreClick = {}
)
}
}

@Preview
@Composable
@Suppress("LongMethod")
internal fun AccessibleDataCalloutStripeDirectPreview() {
FinancialConnectionsPreview {
AccessibleDataCalloutWithAccounts(
AccessibleDataCalloutModel(
businessName = "My business",
permissions = listOf(
Permissions.PAYMENT_METHOD,
Permissions.BALANCES,
Permissions.OWNERSHIP,
Permissions.TRANSACTIONS
),
PartnerAccount(
authorization = "Authorization",
category = FinancialConnectionsAccount.Category.CASH,
id = "id4",
name = "Account 4 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.SAVINGS,
supportedPaymentMethodTypes = emptyList()
isStripeDirect = true,
isNetworking = false,
dataPolicyUrl = ""
),
accounts = partnerAccountsForPreview(),
institution = FinancialConnectionsInstitution(
id = "id",
name = "name",
url = "url",
featured = true,
icon = null,
logo = null,
featuredOrder = null,
mobileHandoffCapable = false
),
onLearnMoreClick = {}
)
}
}
@Preview
@Composable
@Suppress("LongMethod")
internal fun AccessibleDataCalloutNetworkingPreview() {
FinancialConnectionsPreview {
AccessibleDataCalloutWithAccounts(
AccessibleDataCalloutModel(
businessName = "My business",
permissions = listOf(
Permissions.PAYMENT_METHOD,
Permissions.BALANCES,
Permissions.OWNERSHIP,
Permissions.TRANSACTIONS
),
PartnerAccount(
authorization = "Authorization",
category = FinancialConnectionsAccount.Category.CASH,
id = "id5",
name = "Account 5 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.SAVINGS,
supportedPaymentMethodTypes = emptyList()
)
isStripeDirect = false,
isNetworking = true,
dataPolicyUrl = ""
),
accounts = partnerAccountsForPreview(),
institution = FinancialConnectionsInstitution(
id = "id",
name = "name",
Expand All @@ -342,6 +366,61 @@ internal fun AccessibleDataCalloutWithManyAccountsPreview() {
}
}

@Composable
private fun partnerAccountsForPreview() = listOf(
PartnerAccount(
authorization = "Authorization",
institutionName = "Random bank",
category = FinancialConnectionsAccount.Category.CASH,
id = "id1",
name = "Account 1 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.CHECKING,
supportedPaymentMethodTypes = emptyList()
),
PartnerAccount(
authorization = "Authorization",
category = FinancialConnectionsAccount.Category.CASH,
id = "id2",
name = "Account 2 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.SAVINGS,
supportedPaymentMethodTypes = emptyList()
),
PartnerAccount(
authorization = "Authorization",
category = FinancialConnectionsAccount.Category.CASH,
id = "id3",
name = "Account 3 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.SAVINGS,
supportedPaymentMethodTypes = emptyList()
),
PartnerAccount(
authorization = "Authorization",
category = FinancialConnectionsAccount.Category.CASH,
id = "id4",
name = "Account 4 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.SAVINGS,
supportedPaymentMethodTypes = emptyList()
),
PartnerAccount(
authorization = "Authorization",
category = FinancialConnectionsAccount.Category.CASH,
id = "id5",
name = "Account 5 - no acct numbers",
_allowSelection = true,
allowSelectionMessage = "",
subcategory = FinancialConnectionsAccount.Subcategory.SAVINGS,
supportedPaymentMethodTypes = emptyList()
)
)

@Preview
@Composable
internal fun AccessibleDataCalloutWithMultipleAccountsPreview() {
Expand All @@ -356,6 +435,7 @@ internal fun AccessibleDataCalloutWithMultipleAccountsPreview() {
Permissions.TRANSACTIONS
),
isStripeDirect = true,
isNetworking = false,
dataPolicyUrl = ""
),
accounts = listOf(
Expand Down Expand Up @@ -413,6 +493,7 @@ internal fun AccessibleDataCalloutWithOneAccountPreview() {
Permissions.TRANSACTIONS
),
isStripeDirect = true,
isNetworking = false,
dataPolicyUrl = ""
),
accounts = listOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ internal fun SuccessScreenPreview() {
FinancialConnectionsAccount.Permissions.TRANSACTIONS
),
isStripeDirect = true,
isNetworking = false,
dataPolicyUrl = ""
),
disconnectUrl = "",
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Diff not rendered.
Diff not rendered.