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.

Sign up for GitHub

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

[FC] Fix Accessible data callout copy logic #6375

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b0e60af
Fixes copies.
carlosmuvi-stripe Mar 16, 2023
28f329a
Updates changelog.
carlosmuvi-stripe Mar 16, 2023
c75dff0
Merge remote-tracking branch 'origin/master' into carlosmuvi/data-req…
carlosmuvi-stripe Mar 16, 2023
cb234e6
Merge fixes.
carlosmuvi-stripe Mar 16, 2023
ceaacfc
Regenerates screenshots.
carlosmuvi-stripe Mar 16, 2023
52c6b66
Updates stripe direct.
carlosmuvi-stripe Mar 16, 2023
e4133f5
Updates spaces.
carlosmuvi-stripe Mar 16, 2023
327838c
Moves method down.
carlosmuvi-stripe Mar 16, 2023
70e3ede
Ktlint.
carlosmuvi-stripe Mar 16, 2023
9120565
Regenerates API.
carlosmuvi-stripe Mar 17, 2023
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
View file
Open in desktop
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
View file
Open in desktop
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>
Copy link

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 •
edited
Loading

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
1 change: 1 addition & 0 deletions ...ava/com/stripe/android/financialconnections/features/accountpicker/AccountPickerStates.kt
View file
Open in desktop
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
9 changes: 1 addition & 8 deletions .../com/stripe/android/financialconnections/features/accountpicker/AccountPickerViewModel.kt
View file
Open in desktop
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
21 changes: 16 additions & 5 deletions ...ain/java/com/stripe/android/financialconnections/features/common/AccessibleDataCallout.kt
View file
Open in desktop
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) {
Copy link

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? 😄

Copy link

Choose a reason for hiding this comment

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

https://git.corp.stripe.com/stripe-internal/stripe-js-v3/blob/8bd67461/src/linkedAccounts/shared-ui/DataAccessText/MerchantDataAccessText.tsx#L38-L39

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.

52c6b66

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,6 +256,7 @@ internal fun AccessibleDataCalloutPreview() {
Permissions.TRANSACTIONS,
Permissions.ACCOUNT_NUMBERS
),
isNetworking = false,
isStripeDirect = true,
dataPolicyUrl = ""
),
Expand All @@ -272,6 +280,7 @@ internal fun AccessibleDataCalloutWithManyAccountsPreview() {
Permissions.TRANSACTIONS
),
isStripeDirect = true,
isNetworking = false,
dataPolicyUrl = ""
),
accounts = listOf(
Expand Down Expand Up @@ -356,6 +365,7 @@ internal fun AccessibleDataCalloutWithMultipleAccountsPreview() {
Permissions.TRANSACTIONS
),
isStripeDirect = true,
isNetworking = false,
dataPolicyUrl = ""
),
accounts = listOf(
Expand Down Expand Up @@ -413,6 +423,7 @@ internal fun AccessibleDataCalloutWithOneAccountPreview() {
Permissions.TRANSACTIONS
),
isStripeDirect = true,
isNetworking = false,
dataPolicyUrl = ""
),
accounts = listOf(
Expand Down
1 change: 1 addition & 0 deletions ...s/src/main/java/com/stripe/android/financialconnections/features/success/SuccessScreen.kt
View file
Open in desktop
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