fix: Get random profile pictures from Unsplash #3081
Merged
Conversation
fixes https://wearezeta.atlassian.net/browse/AN-7047 I'm not exactly sure how long this functionality was broken but it looks like it was for a long time. The previous code tried to treat an Unsplash URL as if it was a regular asset that was supposed to be downloaded through our backend. The backend failed because it didn't understand the request.I rewrote `AccountManager.addUnsplashPicture()` and added methods to `AssetService` and `AssetClient` which download the picture from Unsplash. Then the picture is set as the profile one and sent to the backend exactly as if it was a regular profile picture. I also added a button in Dev Settings to test it. From now on it can be used if we want to test any change in code connected to profile pictures.
| @@ -39,7 +39,7 @@ class AndroidUriHelper(context: Context) extends UriHelper with DerivedLogTag { | |||
| private def cursor(uri: URI): Managed[Cursor] = | |||
| Managed.create( | |||
| context.getContentResolver.query(androidUri(uri), null, null, null, null) | |||
| )(_.close()) | |||
| ) { c => Option(c).foreach(_.close()) } | |||
makingthematrix
Nov 3, 2020
Author
Contributor
A fix for a potential bug: the cursor can be null and when close() is called on it, it can fail with NullPointerException.
| @@ -142,6 +150,7 @@ object AssetClient { | |||
| implicit val DefaultExpiryTime: Expiration = 1.hour | |||
|
|
|||
| val AssetsV3Path = "/assets/v3" | |||
| val UnsplashUrl: URL = new URL("https://source.unsplash.com/800x800/?landscape") | |||
mejdoo
Nov 3, 2020
•
Contributor
Define "https://source.unsplash.com/800x800/?landscape" as constant
|
|
mejdoo
added a commit
that referenced
this pull request
Nov 4, 2020
* fix: Get random profile pictures from Unsplash fixes https://wearezeta.atlassian.net/browse/AN-7047 I'm not exactly sure how long this functionality was broken but it looks like it was for a long time. The previous code tried to treat an Unsplash URL as if it was a regular asset that was supposed to be downloaded through our backend. The backend failed because it didn't understand the request.I rewrote `AccountManager.addUnsplashPicture()` and added methods to `AssetService` and `AssetClient` which download the picture from Unsplash. Then the picture is set as the profile one and sent to the backend exactly as if it was a regular profile picture. I also added a button in Dev Settings to test it. From now on it can be used if we want to test any change in code connected to profile pictures. * changes requested in the review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
fixes https://wearezeta.atlassian.net/browse/AN-7047
I'm not exactly sure how long this functionality was broken but it looks like it was for a long time. The previous code
tried to treat an Unsplash URL as if it was a regular asset that was supposed to be downloaded through our backend.
The backend failed because it didn't understand the request.I rewrote
AccountManager.addUnsplashPicture()and addedmethods to
AssetServiceandAssetClientwhich download the picture from Unsplash. Then the picture is set asthe profile one and sent to the backend exactly as if it was a regular profile picture.
I also added a button in Dev Settings to test it. From now on it can be used if we want to test any change in code connected to profile pictures.
APK
Download build #2926
Download build #2927