Skip to content

Commit 4264164

Browse files
ShashwatKedianicolas-raoul
ShashwatKedia
and
nicolas-raoul
authored
Fixes commons-app#4704: Remove 'Please Wait' dialog and do task in background (commons-app#5570)
* Initial changes to the flow, merged conflicts * Major changes to flow and logic * Final major changes to the flow and merged conflicts * Minor changes to thumbnail flow and merge conflicts * Fixed ImageProcessingServiceTest * Removed unnecessary file * Some code cleanup and fixed UploadRepositoryUnitTest * Minor javadoc changes and null checks * Fixed UMDFragmentUnitTest * Fixed and added new tests in UploadMediaPresenterTest * Optimised code for no connection cases and minor code cleanup * Minor bug fix * Fixed minor bug * Fixed a failing unit test * Removed values-yue-hant * Update UploadRepository.java --------- Co-authored-by: Nicolas Raoul <[email protected]>
1 parent 16960a3 commit 4264164

13 files changed

+586
-264
lines changed

‎app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,13 +196,23 @@ public Observable<UploadItem> preProcessImage(UploadableFile uploadableFile, Pla
196196
/**
197197
* query the RemoteDataSource for image quality
198198
*
199-
* @param uploadItem
200-
* @return
199+
* @param uploadItem UploadItem whose caption is to be checked
200+
* @return Quality of UploadItem
201201
*/
202202
public Single<Integer> getImageQuality(UploadItem uploadItem, LatLng ___location) {
203203
return uploadModel.getImageQuality(uploadItem, ___location);
204204
}
205205

206+
/**
207+
* query the RemoteDataSource for caption quality
208+
*
209+
* @param uploadItem UploadItem whose caption is to be checked
210+
* @return Quality of caption of the UploadItem
211+
*/
212+
public Single<Integer> getCaptionQuality(UploadItem uploadItem) {
213+
return uploadModel.getCaptionQuality(uploadItem);
214+
}
215+
206216
/**
207217
* asks the LocalDataSource to delete the file with the given file path
208218
*

‎app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22

33
import static fr.free.nrw.commons.utils.ImageUtils.EMPTY_CAPTION;
44
import static fr.free.nrw.commons.utils.ImageUtils.FILE_NAME_EXISTS;
5+
import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_DUPLICATE;
6+
import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_KEEP;
57
import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK;
68

79
import android.content.Context;
810
import fr.free.nrw.commons.___location.LatLng;
911
import fr.free.nrw.commons.media.MediaClient;
1012
import fr.free.nrw.commons.nearby.Place;
11-
import fr.free.nrw.commons.utils.ImageUtils;
1213
import fr.free.nrw.commons.utils.ImageUtilsWrapper;
1314
import io.reactivex.Single;
1415
import io.reactivex.schedulers.Schedulers;
@@ -45,13 +46,17 @@ public ImageProcessingService(FileUtilsWrapper fileUtilsWrapper,
4546

4647
/**
4748
* Check image quality before upload - checks duplicate image - checks dark image - checks
48-
* geolocation for image - check for valid title
49+
* geolocation for image
50+
*
51+
* @param uploadItem UploadItem whose quality is to be checked
52+
* @param inAppPictureLocation In app picture ___location (if any)
53+
* @return Quality of UploadItem
4954
*/
5055
Single<Integer> validateImage(UploadItem uploadItem, LatLng inAppPictureLocation) {
5156
int currentImageQuality = uploadItem.getImageQuality();
5257
Timber.d("Current image quality is %d", currentImageQuality);
53-
if (currentImageQuality == ImageUtils.IMAGE_KEEP) {
54-
return Single.just(ImageUtils.IMAGE_OK);
58+
if (currentImageQuality == IMAGE_KEEP || currentImageQuality == IMAGE_OK) {
59+
return Single.just(IMAGE_OK);
5560
}
5661
Timber.d("Checking the validity of image");
5762
String filePath = uploadItem.getMediaUri().getPath();
@@ -60,18 +65,34 @@ Single<Integer> validateImage(UploadItem uploadItem, LatLng inAppPictureLocation
6065
checkDuplicateImage(filePath),
6166
checkImageGeoLocation(uploadItem.getPlace(), filePath, inAppPictureLocation),
6267
checkDarkImage(filePath),
63-
validateItemTitle(uploadItem),
6468
checkFBMD(filePath),
6569
checkEXIF(filePath),
66-
(duplicateImage, wrongGeoLocation, darkImage, itemTitle, fbmd, exif) -> {
67-
Timber.d("duplicate: %d, geo: %d, dark: %d, title: %d" + "fbmd:" + fbmd + "exif:"
70+
(duplicateImage, wrongGeoLocation, darkImage, fbmd, exif) -> {
71+
Timber.d("duplicate: %d, geo: %d, dark: %d" + "fbmd:" + fbmd + "exif:"
6872
+ exif,
69-
duplicateImage, wrongGeoLocation, darkImage, itemTitle);
70-
return duplicateImage | wrongGeoLocation | darkImage | itemTitle | fbmd | exif;
73+
duplicateImage, wrongGeoLocation, darkImage);
74+
return duplicateImage | wrongGeoLocation | darkImage | fbmd | exif;
7175
}
7276
);
7377
}
7478

79+
/**
80+
* Checks caption of the given UploadItem
81+
*
82+
* @param uploadItem UploadItem whose caption is to be verified
83+
* @return Quality of caption of the UploadItem
84+
*/
85+
Single<Integer> validateCaption(UploadItem uploadItem) {
86+
int currentImageQuality = uploadItem.getImageQuality();
87+
Timber.d("Current image quality is %d", currentImageQuality);
88+
if (currentImageQuality == IMAGE_KEEP) {
89+
return Single.just(IMAGE_OK);
90+
}
91+
Timber.d("Checking the validity of caption");
92+
93+
return validateItemTitle(uploadItem);
94+
}
95+
7596
/**
7697
* We want to discourage users from uploading images to Commons that were taken from Facebook.
7798
* This attempts to detect whether an image was downloaded from Facebook by heuristically
@@ -126,7 +147,7 @@ private Single<Integer> checkDuplicateImage(String filePath) {
126147
.flatMap(mediaClient::checkFileExistsUsingSha)
127148
.map(b -> {
128149
Timber.d("Result for duplicate image %s", b);
129-
return b ? ImageUtils.IMAGE_DUPLICATE : ImageUtils.IMAGE_OK;
150+
return b ? IMAGE_DUPLICATE : IMAGE_OK;
130151
})
131152
.subscribeOn(Schedulers.io());
132153
}
@@ -152,13 +173,13 @@ private Single<Integer> checkDarkImage(String filePath) {
152173
private Single<Integer> checkImageGeoLocation(Place place, String filePath, LatLng inAppPictureLocation) {
153174
Timber.d("Checking for image geolocation %s", filePath);
154175
if (place == null || StringUtils.isBlank(place.getWikiDataEntityId())) {
155-
return Single.just(ImageUtils.IMAGE_OK);
176+
return Single.just(IMAGE_OK);
156177
}
157178
return Single.fromCallable(() -> filePath)
158179
.flatMap(path -> Single.just(fileUtilsWrapper.getGeolocationOfFile(path, inAppPictureLocation)))
159180
.flatMap(geoLocation -> {
160181
if (StringUtils.isBlank(geoLocation)) {
161-
return Single.just(ImageUtils.IMAGE_OK);
182+
return Single.just(IMAGE_OK);
162183
}
163184
return imageUtilsWrapper
164185
.checkImageGeolocationIsDifferent(geoLocation, place.getLocation());

‎app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import fr.free.nrw.commons.upload.license.MediaLicenseFragment;
5454
import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment;
5555
import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.UploadMediaDetailFragmentCallback;
56+
import fr.free.nrw.commons.upload.mediaDetails.UploadMediaPresenter;
5657
import fr.free.nrw.commons.upload.worker.WorkRequestHelper;
5758
import fr.free.nrw.commons.utils.DialogUtil;
5859
import fr.free.nrw.commons.utils.PermissionUtils;
@@ -96,7 +97,7 @@ public class UploadActivity extends BaseActivity implements UploadContract.View,
9697
private DepictsFragment depictsFragment;
9798
private MediaLicenseFragment mediaLicenseFragment;
9899
private ThumbnailsAdapter thumbnailsAdapter;
99-
100+
BasicKvStore store;
100101
private Place place;
101102
private LatLng prevLocation;
102103
private LatLng currLocation;
@@ -139,6 +140,9 @@ public class UploadActivity extends BaseActivity implements UploadContract.View,
139140
* Whether fragments have been saved.
140141
*/
141142
private boolean isFragmentsSaved = false;
143+
144+
public static final String keyForCurrentUploadImagesSize = "CurrentUploadImagesSize";
145+
public static final String storeNameForCurrentUploadImagesSize = "CurrentUploadImageQualities";
142146

143147
private ActivityUploadBinding binding;
144148

@@ -179,7 +183,10 @@ protected void onCreate(Bundle savedInstanceState) {
179183
}
180184
locationManager.requestLocationUpdatesFromProvider(LocationManager.GPS_PROVIDER);
181185
locationManager.requestLocationUpdatesFromProvider(LocationManager.NETWORK_PROVIDER);
186+
store = new BasicKvStore(this, storeNameForCurrentUploadImagesSize);
187+
store.clearAll();
182188
checkStoragePermissions();
189+
183190
}
184191

185192
private void init() {
@@ -470,6 +477,8 @@ private void receiveSharedItems() {
470477
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
471478
if (uploadableFiles.size() > 3
472479
&& !defaultKvStore.getBoolean("hasAlreadyLaunchedBigMultiupload")) {
480+
// When battery-optimisation dialog is shown don't show the image quality dialog
481+
UploadMediaPresenter.isBatteryDialogShowing = true;
473482
DialogUtil.showAlertDialog(
474483
this,
475484
getString(R.string.unrestricted_battery_mode),
@@ -490,8 +499,15 @@ private void receiveSharedItems() {
490499
Intent batteryOptimisationSettingsIntent = new Intent(
491500
Settings.ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS);
492501
startActivity(batteryOptimisationSettingsIntent);
502+
// calling checkImageQuality after battery dialog is interacted with
503+
// so that 2 dialogs do not pop up simultaneously
504+
presenter.checkImageQuality(0);
505+
UploadMediaPresenter.isBatteryDialogShowing = false;
493506
},
494-
() -> {}
507+
() -> {
508+
presenter.checkImageQuality(0);
509+
UploadMediaPresenter.isBatteryDialogShowing = false;
510+
}
495511
);
496512
defaultKvStore.putBoolean("hasAlreadyLaunchedBigMultiupload", true);
497513
}
@@ -525,6 +541,8 @@ private void receiveSharedItems() {
525541
UploadMediaDetailFragmentCallback uploadMediaDetailFragmentCallback = new UploadMediaDetailFragmentCallback() {
526542
@Override
527543
public void deletePictureAtIndex(int index) {
544+
store.putInt(keyForCurrentUploadImagesSize,
545+
(store.getInt(keyForCurrentUploadImagesSize) - 1));
528546
presenter.deletePictureAtIndex(index);
529547
}
530548

@@ -668,6 +686,8 @@ public boolean isWLMUpload() {
668686
binding.vpUpload.setOffscreenPageLimit(fragments.size());
669687

670688
}
689+
// Saving size of uploadableFiles
690+
store.putInt(keyForCurrentUploadImagesSize, uploadableFiles.size());
671691
}
672692

673693
/**
@@ -787,7 +807,11 @@ public void onNextButtonClicked(int index) {
787807
binding.vpUpload.setCurrentItem(index + 1, false);
788808
fragments.get(index + 1).onBecameVisible();
789809
((LinearLayoutManager) binding.rvThumbnails.getLayoutManager())
790-
.scrollToPositionWithOffset((index > 0) ? index-1 : 0, 0);
810+
.scrollToPositionWithOffset((index > 0) ? index - 1 : 0, 0);
811+
if (index < fragments.size() - 4) {
812+
// check image quality if next image exists
813+
presenter.checkImageQuality(index + 1);
814+
}
791815
} else {
792816
presenter.handleSubmit();
793817
}
@@ -800,7 +824,11 @@ public void onPreviousButtonClicked(int index) {
800824
fragments.get(index - 1).onBecameVisible();
801825
((LinearLayoutManager) binding.rvThumbnails.getLayoutManager())
802826
.scrollToPositionWithOffset((index > 3) ? index-2 : 0, 0);
803-
binding.llContainerTopCard.setVisibility(View.VISIBLE);
827+
if ((index != 1) && ((index - 1) < uploadableFiles.size())) {
828+
// Shows the top card if it was hidden because of the last image being deleted and
829+
// now the user has hit previous button to go back to the media details
830+
showHideTopCard(true);
831+
}
804832
}
805833
}
806834

@@ -854,6 +882,8 @@ public void onRlContainerTitleClicked() {
854882
@Override
855883
protected void onDestroy() {
856884
super.onDestroy();
885+
// Resetting all values in store by clearing them
886+
store.clearAll();
857887
presenter.onDetachView();
858888
compositeDisposable.clear();
859889
fragments = null;

‎app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,5 +64,12 @@ public interface UserActionListener extends BasePresenter<View> {
6464
void handleSubmit();
6565

6666
void deletePictureAtIndex(int index);
67+
68+
/**
69+
* Calls checkImageQuality of UploadMediaPresenter to check image quality of next image
70+
*
71+
* @param uploadItemIndex Index of next image, whose quality is to be checked
72+
*/
73+
void checkImageQuality(int uploadItemIndex);
6774
}
6875
}

‎app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,27 @@ public Observable<UploadItem> preProcessImage(final UploadableFile uploadableFil
9292
createAndAddUploadItem(uploadableFile, place, similarImageInterface, inAppPictureLocation));
9393
}
9494

95+
/**
96+
* Calls validateImage() of ImageProcessingService to check quality of image
97+
*
98+
* @param uploadItem UploadItem whose quality is to be checked
99+
* @param inAppPictureLocation In app picture ___location (if any)
100+
* @return Quality of UploadItem
101+
*/
95102
public Single<Integer> getImageQuality(final UploadItem uploadItem, LatLng inAppPictureLocation) {
96103
return imageProcessingService.validateImage(uploadItem, inAppPictureLocation);
97104
}
98105

106+
/**
107+
* Calls validateCaption() of ImageProcessingService to check caption of image
108+
*
109+
* @param uploadItem UploadItem whose caption is to be checked
110+
* @return Quality of caption of the UploadItem
111+
*/
112+
public Single<Integer> getCaptionQuality(final UploadItem uploadItem) {
113+
return imageProcessingService.validateCaption(uploadItem);
114+
}
115+
99116
private UploadItem createAndAddUploadItem(final UploadableFile uploadableFile,
100117
final Place place,
101118
final SimilarImageInterface similarImageInterface,

‎app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import fr.free.nrw.commons.filepicker.UploadableFile;
88
import fr.free.nrw.commons.kvstore.JsonKvStore;
99
import fr.free.nrw.commons.repository.UploadRepository;
10+
import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailsContract;
1011
import io.reactivex.Observer;
1112
import io.reactivex.disposables.CompositeDisposable;
1213
import io.reactivex.disposables.Disposable;
@@ -30,6 +31,8 @@ public class UploadPresenter implements UploadContract.UserActionListener {
3031
private final UploadRepository repository;
3132
private final JsonKvStore defaultKvStore;
3233
private UploadContract.View view = DUMMY;
34+
@Inject
35+
UploadMediaDetailsContract.UserActionListener presenter;
3336

3437
private CompositeDisposable compositeDisposable;
3538
public static final String COUNTER_OF_CONSECUTIVE_UPLOADS_WITHOUT_COORDINATES
@@ -135,17 +138,42 @@ public void onComplete() {
135138
}
136139
}
137140

141+
/**
142+
* Calls checkImageQuality of UploadMediaPresenter to check image quality of next image
143+
*
144+
* @param uploadItemIndex Index of next image, whose quality is to be checked
145+
*/
146+
@Override
147+
public void checkImageQuality(int uploadItemIndex) {
148+
UploadItem uploadItem = repository.getUploadItem(uploadItemIndex);
149+
presenter.checkImageQuality(uploadItem, uploadItemIndex);
150+
}
151+
152+
138153
@Override
139154
public void deletePictureAtIndex(int index) {
140155
List<UploadableFile> uploadableFiles = view.getUploadableFiles();
156+
if (index == uploadableFiles.size() - 1) {
157+
// If the next fragment to be shown is not one of the MediaDetailsFragment
158+
// lets hide the top card so that it doesn't appear on the other fragments
159+
view.showHideTopCard(false);
160+
}
141161
view.setImageCancelled(true);
142162
repository.deletePicture(uploadableFiles.get(index).getFilePath());
143163
if (uploadableFiles.size() == 1) {
144164
view.showMessage(R.string.upload_cancelled);
145165
view.finish();
146166
return;
147167
} else {
168+
if (presenter != null) {
169+
presenter.updateImageQualitiesJSON(uploadableFiles.size(), index);
170+
}
148171
view.onUploadMediaDeleted(index);
172+
if (!(index == uploadableFiles.size()) && index != 0) {
173+
// if the deleted image was not the last item to be uploaded, check quality of next
174+
UploadItem uploadItem = repository.getUploadItem(index);
175+
presenter.checkImageQuality(uploadItem, index);
176+
}
149177
}
150178
if (uploadableFiles.size() < 2) {
151179
view.showHideTopCard(false);

0 commit comments

Comments
 (0)