-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fixes #1545 Top crashes on Play Store for v2.7.1 [A and E] #1672
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
Fixes #1545 Top crashes on Play Store for v2.7.1 [A and E] #1672
Conversation
It feels like the template for reporting bugs is a bit redundant - we have summary, steps to reproduce, observed behaviour AND expected behaviour, which all are very similar. I think that by paring it down to the essentials, people will be less likely to skip steps.
…efresh if fetching fails" (commons-app#1609) * Revert "Modify allow_gps_summary string (commons-app#1608)" This reverts commit 3b6c94c. * Revert "Fix issue commons-app#1332 : Contribution Image auto/manual refresh if fetching fails" This reverts commit 5aba745.
* Versioning for v2.7.2 * Update changelog.md
* Fixfor#1479 * Issue fixed * IssueSolved
* Added Basic Pic Of the Day App Widget * Added Java Lib for XML to JSON * Added missing json library from xml to json * Undone formatting * Consolidate the networking libraries - drop volley in favor of OkHttp * Extracted a few networking related items into a new Dagger module and finished the process of mocking the main component for tests. * Refactoring to extract GpsCategoryModel and ensure single-responsibility-principle is maintained in CategoryApi. * Updated PicOfDayAppWidget class to parse HTML * fixed featured image back bug * Localisation updates from https://translatewiki.net. * Javadocs added * Add option to set image as wallpaper (commons-app#1535) * Add option to set image as wallpaper * Added java docs * Toast message on setting the wallpaper successfully * Localisation updates from https://translatewiki.net. * Add dependencies to com.android.support.test.rules and runner Needed for ActivityTestRule used in SettingsActivityTest * Added Basic Pic Of the Day App Widget * Added Java Lib for XML to JSON * Added missing json library from xml to json * Undone formatting * Updated PicOfDayAppWidget class to parse HTML
… from contributions list.
… from nearby list and map
…l save the file to a internal path and it will be deleted by another method after upload process is done.
Hi all, currently camera bug is solved bud I couldn't find the reason behind undeletable reference to previously uploaded file. On you first upload everything is fine. Our file will be uploaded gets saved to ExternalStorage/1_tmp . Then we delete that file. File.delete() returns true. File becomes invisible on file system (manually checked). On our second upload, we save our new file to ExternalStorage/1_tmp on Share Activity background and behind progress bar on ContributionsListFragment we somehow see previously uploaded file (that we deleted already). If we don't delete files and continue as 1_tmp, 2_tmp... right images becomes visible during upload process. So our problem is deleting and overriding to same file name. It somehow preserves a reference to old file. Did you ever experienced such situation? |
Uri uri = ContributionUtils.saveFileBeingUploadedTemporarily(getActivity(), data.getData()); | ||
controller.handleImagePicked(requestCode, uri, false, null); | ||
if (requestCode == ContributionController.SELECT_FROM_CAMERA) { | ||
controller.handleImagePicked(requestCode, null, false, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neslihanturan Why is the uri
being passed as null for camera clicks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, please update the handleImagePicked
method signature annotating this param as @Nullable
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is null because, data coming from camera intent is already null. Instead, photo gets saved to a directory and its path will be get from intent extras on subsequent steps.
@@ -96,7 +95,7 @@ public void cleanup() { | |||
* @param decimalCoords the coordinates in decimal. (e.g. "37.51136|-77.602615") | |||
* @param onComplete the progress tracker | |||
*/ | |||
public void startUpload(String title, Uri mediaUri, String description, String mimeType, String source, String decimalCoords, String wikiDataEntityId, ContributionUploadProgress onComplete) { | |||
public void startUpload(String title, Uri contentProviderUri, Uri mediaUri, String description, String mimeType, String source, String decimalCoords, String wikiDataEntityId, ContributionUploadProgress onComplete) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the javadocs to reflect the change
@@ -148,7 +148,9 @@ public void startUpload(final Contribution contribution, final ContributionUploa | |||
protected Contribution doInBackground(Void... voids /* stare into you */) { | |||
long length; | |||
ContentResolver contentResolver = context.getContentResolver(); | |||
try { | |||
/*try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain by adding javadocs to why this block has been commented out?
* @param contentProviderUri | ||
*/ | ||
public static void removeTemporaryFile(Context context, Uri tempFileUri, Uri contentProviderUri) { | ||
//TODO: do I have to notify file system about deletion? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am not sure about notifying to the file system but our contributionDao should be updating the table if it has a corresponding tuple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only delete temp file here, after contribution is uploaded. If we delete contribution from contributionDao, then our successful contribution will be removed from contributions table. Then it won't be visible on contribution list.
@@ -279,6 +280,9 @@ public void onCreate(Bundle savedInstanceState) { | |||
contribution = savedInstanceState.getParcelable("contribution"); | |||
} | |||
|
|||
//receiveImageIntent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add javadocs to explain why this has been commented out?
Here is the demo, I tried to display what is going wrong: https://mega.nz/#!bM4xDJ6K!C2Dd4cfNM5DzJVcnrCT2GspeopqUxLV_z4oBb-5rwbs |
Help still wanted. This problem postpones our release. I hope some of you experienced such issue before:( |
… sure if it is useless or not. Requires discussion
…ame file and access old file reference. It was a weird problem though
Codecov Report
@@ Coverage Diff @@
## 2.8-release #1672 +/- ##
==============================================
+ Coverage 3.89% 3.9% +<.01%
==============================================
Files 151 154 +3
Lines 7521 7630 +109
Branches 705 721 +16
==============================================
+ Hits 293 298 +5
- Misses 7211 7314 +103
- Partials 17 18 +1
Continue to review full report at Codecov.
|
@neslihanturan Is help still wanted or were you able to figure it out with the random file name approach? |
@maskaravivek , no everything should be fine now. Solved 🎆 💃 . But Let me test all upload scenarios in 3 hours, than it will be ready for your tests. |
Ready for test! @maskaravivek |
Hi @neslihanturan , I tried testing this but unfortunately I didn't get very far! :(
|
Description
Fixes A and E from #1545 (I hope). It saves file which is being uploaded to a temp location, and uses Uri of that file as reference during upload process, instead of using Uri that content provider supported.
Tests performed (required)
Tested on API 23, beta debug. Tested actions: share photo from another application, share multiple photos from another application and contributing via contributions list.