Fix for iOS Image Downloads not going through Photos lib #7849

mananjadhav · 2022-02-21T13:49:53Z

Details

Adds code to save the images and videos to Photos lib in iOS
Adds two dependencies - react-native-cameraroll and react-native-mime-types
Little code cleanup for splitting the logic for android and iOS
Adds translations for attachment download alerts

Fixed Issues

$ #7307

Tests

Tested download of file, image and video on all platforms
Tested the permissions for iOS. First download popup and subsequent settings redirect
Tested Android downloads to ensure nothing breaks + translations

Verify that no errors appear in the JS console

QA Steps

iOS Image

Go to any chat with the image attachment on the iOS native app
Open the attachment preview and click on the Download icon
It should show a success message
Check the attachment is saved in Photos lib for iOS

iOS Video

Go to any chat with the video attachment on the iOS native app
Click on the download icon on the right of the attachment view
It should show a success message
Check the attachment is saved in Photos lib for iOS

iOS Other files

Go to any chat with the doc/zip/other file type attachment on the iOS native app
Click on the download icon on the right of the attachment view
It should show a success message
Check the attachment is saved in the iOS Files app under Expensify directory

Android all files

Go to any chat with any attachment on the Android native app
Click on the download icon on the right of the attachment view for video and other file types. For image, open preview and click on the download button.
It should show a success message
Check the attachment is saved in the Downloads/Expensify directory

Web/MWeb/Desktop

Go to any chat with any attachment on the Web Desktop or Mobile Web
Click on the download icon on the right of the attachment view for video and other file types. For image, open preview and click on the download button.
It should download the file via browser or save on the desktop app

Verify that no errors appear in the JS console

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

ios-image-download.mp4

Android

mananjadhav · 2022-02-21T13:56:43Z

@thienlnam @parasharrajat While I am implementing this with the proposal I mentioned, I am guessing this isn't an issue with Android. I don't have a phone to test.

Should I move my changes to index.ios.js and move the functions of RNFetchBlob to nativeFileDownload like I did for others? and keep index.android.js will just use functions.
I want to check if to identify the type of the file, should we use mime-type or maintain some extension, etc.

thienlnam · 2022-02-22T20:14:38Z

I am guessing this isn't an issue with Android. I don't have a phone to test.

Yeah I don't think it is, seems like we deal with android permission properly already but can also be checked with an android simulator

Should I move my changes to index.ios.js and move the functions of RNFetchBlob to nativeFileDownload like I did for others? and keep index.android.js will just use functions.

Yeah I would create a seperate file

I want to check if to identify the type of the file, should we use mime-type or maintain some extension, etc.

You can use an external library to determine mime type - we probably won't want to maintain a library for that

thienlnam · 2022-03-07T20:54:08Z

👋🏽 Hi @mananjadhav is there is any progress here? I'm still looking for an update from the comment I left you #7307 (comment)

If there is no further progress on this issue within the next 24 hours we will be un-assigning this and reopening it back to the pool of other contributors. We'd like to keep working with you on this but we will need some more communication. For more details check step 11 here https://github.com/Expensify/App/blob/main/CONTRIBUTING.md#begin-coding-your-solution-in-a-pull-request

mananjadhav · 2022-03-08T03:23:12Z

Hey, @thienlnam missed your comment. Sorry. Almost done.

Works fine on Android, iOS images working fine, the issue with videos. I'll finish by today and push by tomorrow for review.

Also it seems I cannot build on iOS device and can only test on simulator.

thienlnam · 2022-03-08T19:04:41Z

Thanks for the update. That's fine if you can't build on a physical iOS device, simulator testing should suffice

…s-image-download

parasharrajat · 2022-04-06T16:33:07Z

is this ready @mananjadhav?

mananjadhav · 2022-04-06T16:42:31Z

Yes ready for review and merge.

parasharrajat

Ok, Video downloading is failing on iOS. When we directly download the attachment from the chat list for the first time you download an attachment. It asks for permission and then the video never downloads.

Steps:

install the app.
Download the video from any chat.
It should ask for permission. Give the permission. Wait to see the download.
Download never completes.

I don't see any Expensify folder in the Files app. I don't know where did the pdf go.

…s-image-download

mananjadhav · 2022-04-11T18:08:49Z

@parasharrajat I am not able to reproduce the video download error. I tried a fresh install with All Allow and Selected Photos both the options. I tried on iPhone 12 and iPhone 8 simulators. I did test a few more cases and it did work fine. Additionally retested the permission rejection case with the second settings popupview.

I also checked the PDF file. For me a new directory, "New Expensify" is created and the PDF is saved in Files -> On My iPhone -> New Expensify path.

Can you test once more from your side?

parasharrajat · 2022-04-11T18:13:36Z

Sure, I will do that.

parasharrajat

Ok Tested it. Everything looking good.

cc: @thienlnam

🎀 👀 🎀 C+ reviewed

thienlnam

Changes look good, just have a few comments. Going to have the copy double checked

src/CONST.js

src/languages/en.js

thienlnam · 2022-04-20T20:05:34Z

src/libs/fileDownload/index.android.js

+    return Promise.all([writePromise, readPromise]).then(([hasWritePermission, hasReadPermission]) => {
+        if (hasWritePermission && hasReadPermission) {
+            return true; // Return true if permission is already given
+        }
+
+        // Ask for permission if not given
+        return PermissionsAndroid.requestMultiple([
+            PermissionsAndroid.PERMISSIONS.READ_EXTERNAL_STORAGE,
+            PermissionsAndroid.PERMISSIONS.WRITE_EXTERNAL_STORAGE,
+        ]).then(status => status['android.permission.READ_EXTERNAL_STORAGE'] === 'granted'
+                    && status['android.permission.WRITE_EXTERNAL_STORAGE'] === 'granted');
+    });
+}


What if the user has given read permission but not write permission? Does it matter if we request it again?

I haven't tested this. Also considering that this issue was specifically for iOS, in Android I just checked if it doesn't break anything. Do you want me to test this thoroughly for Android as well?

During my testing, Android only ask once for permission which means it combines them together. So if you deny it shows an error. Downloading the file again asks for permission.

…s-image-download

src/languages/es.js

…s-image-download

thienlnam

Awesome, thanks! 🚀

OSBotify · 2022-05-05T00:08:53Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2022-05-09T20:02:50Z

🚀 Cherry-picked to staging by @sketchydroide in version: 1.1.57-8 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

OSBotify · 2022-05-13T16:11:47Z

🚀 Deployed to production by @chiragsalian in version: 1.1.57-17 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

mananjadhav added 3 commits February 21, 2022 19:17

feat: Added separate download for native file download

81aecef

feat: Added "react-native-cameraroll" dependency

9ce92c9

feat: Replaced old function call with new call

9e36c97

mananjadhav added 21 commits March 9, 2022 02:50

Merge branch 'main' of https://github.com/mananjadhav/App into fix/io…

36b71a3

…s-image-download

fix: added mimetypes db

9fa7b64

Merge branch 'main' of https://github.com/mananjadhav/App into fix/io…

567adc3

…s-image-download

fix: split the download functions for android and ios

78b2c1e

fix: moved code to FileUtils

ff6ba29

fix: split android to an independent file

8f67152

fix: moved code from util to index.ios.js and added back alerts

1bb1abe

fix: corrected import

86f9a62

fix: addd file type handling

f23b626

fix: updated attachment name null handling

709aaae

fix: file types in const

1074f05

fix: fixed uri content typed

aa622c2

fix: updated references for class

35974d5

fix: removed .info call for ios

24286ef

fix: added remote video save

a699224

fix: photo lib access error

c0db396

fix: error messages clean up

bf91492

fix: localized strings for alerts

8dd1eaf

fix: updated jsdocs

c93d809

fix: corrected translation path

7524322

refactor: split alerts to individual functions

ac2bbb4

mananjadhav marked this pull request as ready for review March 14, 2022 13:15

mananjadhav added 2 commits April 6, 2022 20:53

fix: rollback urlname change

bfd1523

style: make comment title case

043dda4

mananjadhav requested a review from parasharrajat April 6, 2022 16:42

parasharrajat suggested changes Apr 7, 2022

View reviewed changes

Merge branch 'main' of https://github.com/mananjadhav/App into fix/io…

89427a1

…s-image-download

parasharrajat previously approved these changes Apr 19, 2022

View reviewed changes

thienlnam requested changes Apr 20, 2022

View reviewed changes

mananjadhav added 3 commits May 1, 2022 03:41

Merge branch 'main' of https://github.com/mananjadhav/App into fix/io…

7b2c04a

…s-image-download

fix: updated message copy

66f48c7

fix: added comment

70e6d62

mananjadhav dismissed parasharrajat’s stale review via 70e6d62 April 30, 2022 23:16

mananjadhav requested a review from thienlnam April 30, 2022 23:24

parasharrajat reviewed May 1, 2022

View reviewed changes

src/languages/es.js Outdated Show resolved Hide resolved

mananjadhav added 2 commits May 2, 2022 23:13

Merge branch 'main' of https://github.com/mananjadhav/App into fix/io…

d6c3039

…s-image-download

fix: corrected copies for modal dialogs

4066699

thienlnam approved these changes May 4, 2022

View reviewed changes

thienlnam merged commit 60690ee into Expensify:main May 4, 2022

OSBotify mentioned this pull request May 10, 2022

Deploy Checklist: New Expensify 2022-04-25 #8777

Closed

kbecciv mentioned this pull request May 10, 2022

IOS - Can't play videos or download videos in any format #8923

Closed

This was referenced May 11, 2022

[High] Keyboard: Edit button is not accessible using keyboard. #8605

Closed

[high] : Many pages: Chrome : The 'More' button is not accessible using screen reader. #8527

Closed

This was referenced May 12, 2022

[HOLD for payment 2022-05-20][$500] Some image and video downloads do not get stored in Photo Gallery in iOS - Reported by @mananjadhav #7307

Closed

Revert "Open old dot links with openOldDotLink() function" #8952

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for iOS Image Downloads not going through Photos lib #7849

Fix for iOS Image Downloads not going through Photos lib #7849

mananjadhav commented Feb 21, 2022 •

edited

Loading

mananjadhav commented Feb 21, 2022

thienlnam commented Feb 22, 2022

thienlnam commented Mar 7, 2022

mananjadhav commented Mar 8, 2022

thienlnam commented Mar 8, 2022

parasharrajat commented Apr 6, 2022

mananjadhav commented Apr 6, 2022

parasharrajat left a comment •

edited

Loading

mananjadhav commented Apr 11, 2022

parasharrajat commented Apr 11, 2022

parasharrajat left a comment

thienlnam left a comment

thienlnam Apr 20, 2022

mananjadhav Apr 21, 2022

parasharrajat Apr 21, 2022 •

edited

Loading

thienlnam left a comment

OSBotify commented May 5, 2022

OSBotify commented May 9, 2022

OSBotify commented May 13, 2022

Fix for iOS Image Downloads not going through Photos lib #7849

Fix for iOS Image Downloads not going through Photos lib #7849

Conversation

mananjadhav commented Feb 21, 2022 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

mananjadhav commented Feb 21, 2022

thienlnam commented Feb 22, 2022

thienlnam commented Mar 7, 2022

mananjadhav commented Mar 8, 2022

thienlnam commented Mar 8, 2022

parasharrajat commented Apr 6, 2022

mananjadhav commented Apr 6, 2022

parasharrajat left a comment • edited Loading

Choose a reason for hiding this comment

mananjadhav commented Apr 11, 2022

parasharrajat commented Apr 11, 2022

parasharrajat left a comment

Choose a reason for hiding this comment

thienlnam left a comment

Choose a reason for hiding this comment

thienlnam Apr 20, 2022

Choose a reason for hiding this comment

mananjadhav Apr 21, 2022

Choose a reason for hiding this comment

parasharrajat Apr 21, 2022 • edited Loading

Choose a reason for hiding this comment

thienlnam left a comment

Choose a reason for hiding this comment

OSBotify commented May 5, 2022

OSBotify commented May 9, 2022

OSBotify commented May 13, 2022

mananjadhav commented Feb 21, 2022 •

edited

Loading

parasharrajat left a comment •

edited

Loading

parasharrajat Apr 21, 2022 •

edited

Loading