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

[Backed Receipt] Download and share a file not a link #10650

Merged

Conversation

kidinov
Copy link
Contributor

@kidinov kidinov commented Jan 30, 2024

Closes: #10648
Closes: #10625 (now clear that we target 8.7.0 as WC version that will have backend generated receipts)
Closes: #10632 (now clear that 2 days of expiration days will be enough)

Description

Before this PR we shared the receipt

  • Only via email intended
  • We attached a link

No, we share in a generic way so it can be sent via any supported app (e.g., messengers), and we share a file, which is downloaded by a given link. File sharing is needed to reduce the amount of issues in cases when a link on a receipt expires.

Testing instructions

Test on both the release version of WC and the dev version that has a backend receipt generation

  • Open a order that has a receipt (all paid orders have receipts if the backend supports receipt generation)

  • Click "See receipt" -> Send.

  • Notice that you can share it via different apps and that we attach file now

  • Collect payment via a card reader

  • On the last step click on "send receipt"

  • Notice that you can share it via different apps and that we attach file now

Images/gif

01-30--15-00.mp4
  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@kidinov kidinov changed the base branch from trunk to 10612-backend-receipts-abstract-old-stripe-and-the-new-way-of-retriving-receipts-based-on-wc-version January 30, 2024 14:04
@kidinov kidinov added the feature: order details Related to order details. label Jan 30, 2024
@kidinov kidinov modified the milestones: 17.1 ❄️, 17.2 Jan 30, 2024
@kidinov kidinov requested a review from samiuelson January 30, 2024 14:09
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 30, 2024

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
FlavorJalapeno
Build TypeDebug
Commitbd89dfb
Direct Downloadwoocommerce-prototype-build-pr10650-bd89dfb.apk

@peril-woocommerce
Copy link

peril-woocommerce bot commented Jan 31, 2024

Fails
🚫

Danger failed to run /app/danger-0.3ncu0dykfqc.ts.

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 2 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Error TypeError

Cannot read property 'diffForFile' of undefined
TypeError: Cannot read property 'diffForFile' of undefined
    at Object.exports.default (/app/danger-0.3ncu0dykfqc.ts:27:46)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Dangerfile

22|     // Calculate the changes to Test files because we don't want to limit those
23|     // Note that linesOfCode() function in the latest GitDSL can solve this problem in a more elegant way,
24|     // but the version of Danger currently used by Peril doesn't have it yet.
25|     const testFiles = [...modifiedFiles, ...createdFiles, ...deletedFiles].filter((path: string) => path.includes("/src/test"));
26|     let changesToTests = 0;
------------------------------------------------^
27|     for (let file of testFiles) {        
28|         const stringDiffs = await danger.git.diffForFile(file)
29|         const addedLines = stringDiffs.added.split("\n").filter(x => x.startsWith("+")).length;
30|         const removedLines = stringDiffs.removed.split("\n").filter(x => x.startsWith("-")).length;

Generated by 🚫 dangerJS

@codecov-commenter
Copy link

codecov-commenter commented Jan 31, 2024

Codecov Report

Attention: 37 lines in your changes are missing coverage. Please review.

Comparison is base (f3440e1) 41.44% compared to head (d143c97) 41.45%.
Report is 126 commits behind head on trunk.

❗ Current head d143c97 differs from pull request most recent head bd89dfb. Consider uploading reports for the commit bd89dfb to get more accurate results

Files Patch % Lines
...android/ui/payments/receipt/PaymentReceiptShare.kt 44.11% 17 Missing and 2 partials ⚠️
...ndroid/ui/payments/tracking/PaymentsFlowTracker.kt 0.00% 17 Missing ⚠️
...s/cardreader/payment/CardReaderPaymentViewModel.kt 94.11% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              trunk   #10650   +/-   ##
=========================================
  Coverage     41.44%   41.45%           
- Complexity     4991     4993    +2     
=========================================
  Files          1007     1008    +1     
  Lines         57765    57826   +61     
  Branches       7697     7710   +13     
=========================================
+ Hits          23942    23969   +27     
- Misses        31703    31737   +34     
  Partials       2120     2120           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dangermattic
Copy link
Collaborator

dangermattic commented Jan 31, 2024

1 Warning
⚠️ This PR is assigned to the milestone 17.2. The due date for this milestone has already passed.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

Base automatically changed from 10612-backend-receipts-abstract-old-stripe-and-the-new-way-of-retriving-receipts-based-on-wc-version to trunk February 1, 2024 09:45
@kidinov kidinov requested review from AnirudhBhat and removed request for samiuelson February 1, 2024 09:57
@AnirudhBhat AnirudhBhat self-assigned this Feb 1, 2024
Copy link
Contributor

@AnirudhBhat AnirudhBhat left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @kidinov. The functionality works as expected. :shipit:

context = context,
)

@Test
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick❓ How about adding test that verifies Success and Sharing state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not trivial to do so, as we need to hide all android classes and I decided that this the case when effort doesn't worth the impact 🤷‍♂️

@kidinov kidinov enabled auto-merge February 2, 2024 08:10
@wpmobilebot
Copy link
Collaborator

Found 1 violations:

The PR caused the following dependency changes:

expand

-+--- org.wordpress:fluxc:2948-235834ae9467ae3ca367a4b13c4f9c6aea0f31bf
-|    +--- org.wordpress:wellsql:2.0.0
-|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
-|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-|    +--- org.wordpress.fluxc:fluxc-annotations:2948-235834ae9467ae3ca367a4b13c4f9c6aea0f31bf
-|    +--- org.greenrobot:eventbus:3.3.1 (*)
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
-|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
-|    +--- androidx.paging:paging-runtime:2.1.2
-|    |    +--- androidx.paging:paging-common:2.1.2
-|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.7.0 (*)
-|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
-|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
-|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.6.2 (*)
-|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.2 (*)
-|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-|    +--- com.goterl:lazysodium-android:5.0.2
-|    +--- net.java.dev.jna:jna:5.5.0
-|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.8.22 (*)
-|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.8.22
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 (*)
-|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.5.0 (*)
-|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.3
-|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
-|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
-|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.7.0 (*)
-|    |    +--- com.google.crypto.tink:tink-android:1.5.0
-|    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
-|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
-|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.8.22 (*)
-|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-|    +--- org.apache.commons:commons-text:1.10.0 (*)
-|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
-|    +--- androidx.room:room-ktx:2.4.2 -> 2.5.2
-|    |    +--- androidx.room:room-common:2.5.2 (*)
-|    |    +--- androidx.room:room-runtime:2.5.2 (*)
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.8.22 (*)
-|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.7.3 (*)
-|    +--- com.google.dagger:dagger:2.42 -> 2.47
-|    |    \--- javax.inject:javax.inject:1
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
-|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
++--- org.wordpress:fluxc:trunk-b34cc5eed609ca5274fbb442991f657f2380de17
+|    +--- org.wordpress:wellsql:2.0.0
+|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
+|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
+|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-b34cc5eed609ca5274fbb442991f657f2380de17
+|    +--- org.greenrobot:eventbus:3.3.1 (*)
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
+|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
+|    +--- androidx.paging:paging-runtime:2.1.2
+|    |    +--- androidx.paging:paging-common:2.1.2
+|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.7.0 (*)
+|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
+|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
+|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.6.2 (*)
+|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.2 (*)
+|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+|    +--- com.goterl:lazysodium-android:5.0.2
+|    +--- net.java.dev.jna:jna:5.5.0
+|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.8.22 (*)
+|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.8.22
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 (*)
+|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.5.0 (*)
+|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.3
+|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
+|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
+|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.7.0 (*)
+|    |    +--- com.google.crypto.tink:tink-android:1.5.0
+|    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
+|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
+|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.8.22 (*)
+|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+|    +--- org.apache.commons:commons-text:1.10.0 (*)
+|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
+|    +--- androidx.room:room-ktx:2.4.2 -> 2.5.2
+|    |    +--- androidx.room:room-common:2.5.2 (*)
+|    |    +--- androidx.room:room-runtime:2.5.2 (*)
+|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.8.22 (*)
+|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.7.3 (*)
+|    +--- com.google.dagger:dagger:2.42 -> 2.47
+|    |    \--- javax.inject:javax.inject:1
+|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
+|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
-\--- org.wordpress.fluxc.plugins:woocommerce:2948-235834ae9467ae3ca367a4b13c4f9c6aea0f31bf
-     +--- org.wordpress:wellsql:2.0.0 (*)
-     +--- org.wordpress.fluxc:fluxc-annotations:2948-235834ae9467ae3ca367a4b13c4f9c6aea0f31bf
-     +--- androidx.room:room-ktx:2.4.2 -> 2.5.2 (*)
-     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.8.22 (*)
-     +--- org.wordpress:fluxc:2948-235834ae9467ae3ca367a4b13c4f9c6aea0f31bf (*)
-     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-     +--- com.google.dagger:dagger:2.42 -> 2.47 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
-     \--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
+\--- org.wordpress.fluxc.plugins:woocommerce:trunk-b34cc5eed609ca5274fbb442991f657f2380de17
+     +--- org.wordpress:wellsql:2.0.0 (*)
+     +--- org.wordpress.fluxc:fluxc-annotations:trunk-b34cc5eed609ca5274fbb442991f657f2380de17
+     +--- androidx.room:room-ktx:2.4.2 -> 2.5.2 (*)
+     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.8.22 (*)
+     +--- org.wordpress:fluxc:trunk-b34cc5eed609ca5274fbb442991f657f2380de17 (*)
+     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+     +--- com.google.dagger:dagger:2.42 -> 2.47 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
+     \--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)

Please review and act accordingly

@kidinov kidinov merged commit 8206e6e into trunk Feb 2, 2024
14 of 15 checks passed
@kidinov kidinov deleted the 10648-backed-receipt-download-and-share-a-file-not-a-link branch February 2, 2024 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: order details Related to order details.
Projects
None yet
5 participants