From 42641644cb5448c427dab6e8e5189ba7c85692f5 Mon Sep 17 00:00:00 2001 From: Shashwat Kedia <142137555+ShashwatKedia@users.noreply.github.com> Date: Wed, 27 Mar 2024 13:03:28 +0530 Subject: [PATCH] Fixes #4704: Remove 'Please Wait' dialog and do task in background (#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 --- .../commons/repository/UploadRepository.java | 14 +- .../upload/ImageProcessingService.java | 45 ++- .../nrw/commons/upload/UploadActivity.java | 38 +- .../nrw/commons/upload/UploadContract.java | 7 + .../free/nrw/commons/upload/UploadModel.java | 17 + .../nrw/commons/upload/UploadPresenter.java | 28 ++ .../UploadMediaDetailFragment.java | 179 +++++---- .../UploadMediaDetailsContract.java | 50 ++- .../mediaDetails/UploadMediaPresenter.java | 347 +++++++++++++----- .../upload/ImageProcessingServiceTest.kt | 2 +- .../upload/UploadMediaPresenterTest.kt | 93 ++--- .../upload/UploadRepositoryUnitTest.kt | 9 + .../UploadMediaDetailFragmentUnitTest.kt | 25 +- 13 files changed, 588 insertions(+), 266 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java index bde9984cd..2374ebfc3 100644 --- a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java +++ b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java @@ -196,13 +196,23 @@ public class UploadRepository { /** * query the RemoteDataSource for image quality * - * @param uploadItem - * @return + * @param uploadItem UploadItem whose caption is to be checked + * @return Quality of UploadItem */ public Single getImageQuality(UploadItem uploadItem, LatLng location) { return uploadModel.getImageQuality(uploadItem, location); } + /** + * query the RemoteDataSource for caption quality + * + * @param uploadItem UploadItem whose caption is to be checked + * @return Quality of caption of the UploadItem + */ + public Single getCaptionQuality(UploadItem uploadItem) { + return uploadModel.getCaptionQuality(uploadItem); + } + /** * asks the LocalDataSource to delete the file with the given file path * diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java index 6aa637ee5..45c4b87d9 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java @@ -2,13 +2,14 @@ package fr.free.nrw.commons.upload; import static fr.free.nrw.commons.utils.ImageUtils.EMPTY_CAPTION; import static fr.free.nrw.commons.utils.ImageUtils.FILE_NAME_EXISTS; +import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_DUPLICATE; +import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_KEEP; import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK; import android.content.Context; import fr.free.nrw.commons.location.LatLng; import fr.free.nrw.commons.media.MediaClient; import fr.free.nrw.commons.nearby.Place; -import fr.free.nrw.commons.utils.ImageUtils; import fr.free.nrw.commons.utils.ImageUtilsWrapper; import io.reactivex.Single; import io.reactivex.schedulers.Schedulers; @@ -45,13 +46,17 @@ public class ImageProcessingService { /** * Check image quality before upload - checks duplicate image - checks dark image - checks - * geolocation for image - check for valid title + * geolocation for image + * + * @param uploadItem UploadItem whose quality is to be checked + * @param inAppPictureLocation In app picture location (if any) + * @return Quality of UploadItem */ Single validateImage(UploadItem uploadItem, LatLng inAppPictureLocation) { int currentImageQuality = uploadItem.getImageQuality(); Timber.d("Current image quality is %d", currentImageQuality); - if (currentImageQuality == ImageUtils.IMAGE_KEEP) { - return Single.just(ImageUtils.IMAGE_OK); + if (currentImageQuality == IMAGE_KEEP || currentImageQuality == IMAGE_OK) { + return Single.just(IMAGE_OK); } Timber.d("Checking the validity of image"); String filePath = uploadItem.getMediaUri().getPath(); @@ -60,18 +65,34 @@ public class ImageProcessingService { checkDuplicateImage(filePath), checkImageGeoLocation(uploadItem.getPlace(), filePath, inAppPictureLocation), checkDarkImage(filePath), - validateItemTitle(uploadItem), checkFBMD(filePath), checkEXIF(filePath), - (duplicateImage, wrongGeoLocation, darkImage, itemTitle, fbmd, exif) -> { - Timber.d("duplicate: %d, geo: %d, dark: %d, title: %d" + "fbmd:" + fbmd + "exif:" + (duplicateImage, wrongGeoLocation, darkImage, fbmd, exif) -> { + Timber.d("duplicate: %d, geo: %d, dark: %d" + "fbmd:" + fbmd + "exif:" + exif, - duplicateImage, wrongGeoLocation, darkImage, itemTitle); - return duplicateImage | wrongGeoLocation | darkImage | itemTitle | fbmd | exif; + duplicateImage, wrongGeoLocation, darkImage); + return duplicateImage | wrongGeoLocation | darkImage | fbmd | exif; } ); } + /** + * Checks caption of the given UploadItem + * + * @param uploadItem UploadItem whose caption is to be verified + * @return Quality of caption of the UploadItem + */ + Single validateCaption(UploadItem uploadItem) { + int currentImageQuality = uploadItem.getImageQuality(); + Timber.d("Current image quality is %d", currentImageQuality); + if (currentImageQuality == IMAGE_KEEP) { + return Single.just(IMAGE_OK); + } + Timber.d("Checking the validity of caption"); + + return validateItemTitle(uploadItem); + } + /** * We want to discourage users from uploading images to Commons that were taken from Facebook. * This attempts to detect whether an image was downloaded from Facebook by heuristically @@ -126,7 +147,7 @@ public class ImageProcessingService { .flatMap(mediaClient::checkFileExistsUsingSha) .map(b -> { Timber.d("Result for duplicate image %s", b); - return b ? ImageUtils.IMAGE_DUPLICATE : ImageUtils.IMAGE_OK; + return b ? IMAGE_DUPLICATE : IMAGE_OK; }) .subscribeOn(Schedulers.io()); } @@ -152,13 +173,13 @@ public class ImageProcessingService { private Single checkImageGeoLocation(Place place, String filePath, LatLng inAppPictureLocation) { Timber.d("Checking for image geolocation %s", filePath); if (place == null || StringUtils.isBlank(place.getWikiDataEntityId())) { - return Single.just(ImageUtils.IMAGE_OK); + return Single.just(IMAGE_OK); } return Single.fromCallable(() -> filePath) .flatMap(path -> Single.just(fileUtilsWrapper.getGeolocationOfFile(path, inAppPictureLocation))) .flatMap(geoLocation -> { if (StringUtils.isBlank(geoLocation)) { - return Single.just(ImageUtils.IMAGE_OK); + return Single.just(IMAGE_OK); } return imageUtilsWrapper .checkImageGeolocationIsDifferent(geoLocation, place.getLocation()); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java index c4632e7e2..5bb9fa173 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java @@ -53,6 +53,7 @@ import fr.free.nrw.commons.upload.depicts.DepictsFragment; import fr.free.nrw.commons.upload.license.MediaLicenseFragment; import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment; import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.UploadMediaDetailFragmentCallback; +import fr.free.nrw.commons.upload.mediaDetails.UploadMediaPresenter; import fr.free.nrw.commons.upload.worker.WorkRequestHelper; import fr.free.nrw.commons.utils.DialogUtil; import fr.free.nrw.commons.utils.PermissionUtils; @@ -96,7 +97,7 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, private DepictsFragment depictsFragment; private MediaLicenseFragment mediaLicenseFragment; private ThumbnailsAdapter thumbnailsAdapter; - + BasicKvStore store; private Place place; private LatLng prevLocation; private LatLng currLocation; @@ -139,6 +140,9 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, * Whether fragments have been saved. */ private boolean isFragmentsSaved = false; + + public static final String keyForCurrentUploadImagesSize = "CurrentUploadImagesSize"; + public static final String storeNameForCurrentUploadImagesSize = "CurrentUploadImageQualities"; private ActivityUploadBinding binding; @@ -179,7 +183,10 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, } locationManager.requestLocationUpdatesFromProvider(LocationManager.GPS_PROVIDER); locationManager.requestLocationUpdatesFromProvider(LocationManager.NETWORK_PROVIDER); + store = new BasicKvStore(this, storeNameForCurrentUploadImagesSize); + store.clearAll(); checkStoragePermissions(); + } private void init() { @@ -470,6 +477,8 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { if (uploadableFiles.size() > 3 && !defaultKvStore.getBoolean("hasAlreadyLaunchedBigMultiupload")) { + // When battery-optimisation dialog is shown don't show the image quality dialog + UploadMediaPresenter.isBatteryDialogShowing = true; DialogUtil.showAlertDialog( this, getString(R.string.unrestricted_battery_mode), @@ -490,8 +499,15 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, Intent batteryOptimisationSettingsIntent = new Intent( Settings.ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS); startActivity(batteryOptimisationSettingsIntent); + // calling checkImageQuality after battery dialog is interacted with + // so that 2 dialogs do not pop up simultaneously + presenter.checkImageQuality(0); + UploadMediaPresenter.isBatteryDialogShowing = false; }, - () -> {} + () -> { + presenter.checkImageQuality(0); + UploadMediaPresenter.isBatteryDialogShowing = false; + } ); defaultKvStore.putBoolean("hasAlreadyLaunchedBigMultiupload", true); } @@ -525,6 +541,8 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, UploadMediaDetailFragmentCallback uploadMediaDetailFragmentCallback = new UploadMediaDetailFragmentCallback() { @Override public void deletePictureAtIndex(int index) { + store.putInt(keyForCurrentUploadImagesSize, + (store.getInt(keyForCurrentUploadImagesSize) - 1)); presenter.deletePictureAtIndex(index); } @@ -668,6 +686,8 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, binding.vpUpload.setOffscreenPageLimit(fragments.size()); } + // Saving size of uploadableFiles + store.putInt(keyForCurrentUploadImagesSize, uploadableFiles.size()); } /** @@ -787,7 +807,11 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, binding.vpUpload.setCurrentItem(index + 1, false); fragments.get(index + 1).onBecameVisible(); ((LinearLayoutManager) binding.rvThumbnails.getLayoutManager()) - .scrollToPositionWithOffset((index > 0) ? index-1 : 0, 0); + .scrollToPositionWithOffset((index > 0) ? index - 1 : 0, 0); + if (index < fragments.size() - 4) { + // check image quality if next image exists + presenter.checkImageQuality(index + 1); + } } else { presenter.handleSubmit(); } @@ -800,7 +824,11 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, fragments.get(index - 1).onBecameVisible(); ((LinearLayoutManager) binding.rvThumbnails.getLayoutManager()) .scrollToPositionWithOffset((index > 3) ? index-2 : 0, 0); - binding.llContainerTopCard.setVisibility(View.VISIBLE); + if ((index != 1) && ((index - 1) < uploadableFiles.size())) { + // Shows the top card if it was hidden because of the last image being deleted and + // now the user has hit previous button to go back to the media details + showHideTopCard(true); + } } } @@ -854,6 +882,8 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, @Override protected void onDestroy() { super.onDestroy(); + // Resetting all values in store by clearing them + store.clearAll(); presenter.onDetachView(); compositeDisposable.clear(); fragments = null; diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java index b498563aa..4df778746 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java @@ -64,5 +64,12 @@ public interface UploadContract { void handleSubmit(); void deletePictureAtIndex(int index); + + /** + * Calls checkImageQuality of UploadMediaPresenter to check image quality of next image + * + * @param uploadItemIndex Index of next image, whose quality is to be checked + */ + void checkImageQuality(int uploadItemIndex); } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java index bb447cf6d..ed1193e73 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java @@ -92,10 +92,27 @@ public class UploadModel { createAndAddUploadItem(uploadableFile, place, similarImageInterface, inAppPictureLocation)); } + /** + * Calls validateImage() of ImageProcessingService to check quality of image + * + * @param uploadItem UploadItem whose quality is to be checked + * @param inAppPictureLocation In app picture location (if any) + * @return Quality of UploadItem + */ public Single getImageQuality(final UploadItem uploadItem, LatLng inAppPictureLocation) { return imageProcessingService.validateImage(uploadItem, inAppPictureLocation); } + /** + * Calls validateCaption() of ImageProcessingService to check caption of image + * + * @param uploadItem UploadItem whose caption is to be checked + * @return Quality of caption of the UploadItem + */ + public Single getCaptionQuality(final UploadItem uploadItem) { + return imageProcessingService.validateCaption(uploadItem); + } + private UploadItem createAndAddUploadItem(final UploadableFile uploadableFile, final Place place, final SimilarImageInterface similarImageInterface, diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java index 021eefae6..144859432 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java @@ -7,6 +7,7 @@ import fr.free.nrw.commons.contributions.Contribution; import fr.free.nrw.commons.filepicker.UploadableFile; import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.repository.UploadRepository; +import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailsContract; import io.reactivex.Observer; import io.reactivex.disposables.CompositeDisposable; import io.reactivex.disposables.Disposable; @@ -30,6 +31,8 @@ public class UploadPresenter implements UploadContract.UserActionListener { private final UploadRepository repository; private final JsonKvStore defaultKvStore; private UploadContract.View view = DUMMY; + @Inject + UploadMediaDetailsContract.UserActionListener presenter; private CompositeDisposable compositeDisposable; public static final String COUNTER_OF_CONSECUTIVE_UPLOADS_WITHOUT_COORDINATES @@ -135,9 +138,26 @@ public class UploadPresenter implements UploadContract.UserActionListener { } } + /** + * Calls checkImageQuality of UploadMediaPresenter to check image quality of next image + * + * @param uploadItemIndex Index of next image, whose quality is to be checked + */ + @Override + public void checkImageQuality(int uploadItemIndex) { + UploadItem uploadItem = repository.getUploadItem(uploadItemIndex); + presenter.checkImageQuality(uploadItem, uploadItemIndex); + } + + @Override public void deletePictureAtIndex(int index) { List uploadableFiles = view.getUploadableFiles(); + if (index == uploadableFiles.size() - 1) { + // If the next fragment to be shown is not one of the MediaDetailsFragment + // lets hide the top card so that it doesn't appear on the other fragments + view.showHideTopCard(false); + } view.setImageCancelled(true); repository.deletePicture(uploadableFiles.get(index).getFilePath()); if (uploadableFiles.size() == 1) { @@ -145,7 +165,15 @@ public class UploadPresenter implements UploadContract.UserActionListener { view.finish(); return; } else { + if (presenter != null) { + presenter.updateImageQualitiesJSON(uploadableFiles.size(), index); + } view.onUploadMediaDeleted(index); + if (!(index == uploadableFiles.size()) && index != 0) { + // if the deleted image was not the last item to be uploaded, check quality of next + UploadItem uploadItem = repository.getUploadItem(index); + presenter.checkImageQuality(uploadItem, index); + } } if (uploadableFiles.size() < 2) { view.showHideTopCard(false); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java index 9f4a966f9..aa3d5bb79 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java @@ -2,10 +2,9 @@ package fr.free.nrw.commons.upload.mediaDetails; import static android.app.Activity.RESULT_OK; import static fr.free.nrw.commons.utils.ActivityUtils.startActivityWithFlags; -import static fr.free.nrw.commons.utils.ImageUtils.FILE_NAME_EXISTS; -import static fr.free.nrw.commons.utils.ImageUtils.getErrorMessageForResult; import android.annotation.SuppressLint; +import android.app.Activity; import android.content.Intent; import android.graphics.drawable.Drawable; import android.net.Uri; @@ -44,6 +43,7 @@ import fr.free.nrw.commons.upload.UploadMediaDetail; import fr.free.nrw.commons.upload.UploadMediaDetailAdapter; import fr.free.nrw.commons.utils.DialogUtil; import fr.free.nrw.commons.utils.ImageUtils; +import fr.free.nrw.commons.utils.NetworkUtils; import fr.free.nrw.commons.utils.ViewUtil; import java.io.File; import java.util.ArrayList; @@ -52,7 +52,6 @@ import java.util.Locale; import java.util.Objects; import javax.inject.Inject; import javax.inject.Named; -import org.apache.commons.lang3.StringUtils; import timber.log.Timber; public class UploadMediaDetailFragment extends UploadBaseFragment implements @@ -62,6 +61,10 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements private static final int REQUEST_CODE_FOR_EDIT_ACTIVITY = 1212; private static final int REQUEST_CODE_FOR_VOICE_INPUT = 1213; + public static Activity activity ; + + private int indexOfFragment; + /** * A key for applicationKvStore. By this key we can retrieve the location of last UploadItem ex. * 12.3433,54.78897 from applicationKvStore. @@ -119,12 +122,17 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements */ private UploadItem editableUploadItem; + private BasicKvStore basicKvStore; + + private final String keyForShowingAlertDialog = "isNoNetworkAlertDialogShowing"; + private UploadMediaDetailFragmentCallback callback; private FragmentUploadMediaDetailFragmentBinding binding; public void setCallback(UploadMediaDetailFragmentCallback callback) { this.callback = callback; + UploadMediaPresenter.presenterCallback = callback; } @Override @@ -160,24 +168,38 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) { super.onViewCreated(view, savedInstanceState); + activity = getActivity(); + basicKvStore = new BasicKvStore(activity, "CurrentUploadImageQualities"); + if (callback != null) { + indexOfFragment = callback.getIndexInViewFlipper(this); init(); } if(savedInstanceState!=null){ if(uploadMediaDetailAdapter.getItems().size()==0 && callback != null){ uploadMediaDetailAdapter.setItems(savedInstanceState.getParcelableArrayList(UPLOAD_MEDIA_DETAILS)); - presenter.setUploadMediaDetails(uploadMediaDetailAdapter.getItems(), callback.getIndexInViewFlipper(this)); + presenter.setUploadMediaDetails(uploadMediaDetailAdapter.getItems(), + indexOfFragment); } } + try { + if(!presenter.getImageQuality(indexOfFragment, inAppPictureLocation, getActivity())) { + startActivityWithFlags( + getActivity(), MainActivity.class, Intent.FLAG_ACTIVITY_CLEAR_TOP, + Intent.FLAG_ACTIVITY_SINGLE_TOP); + } + } catch (Exception e) { + } + } private void init() { if (binding == null) { return; } - binding.tvTitle.setText(getString(R.string.step_count, callback.getIndexInViewFlipper(this) + 1, + binding.tvTitle.setText(getString(R.string.step_count, (indexOfFragment + 1), callback.getTotalNumberOfSteps(), getString(R.string.media_detail_step_title))); binding.tooltip.setOnClickListener( v -> showInfoAlert(R.string.media_detail_step_title, R.string.media_details_tooltip)); @@ -185,7 +207,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements presenter.receiveImage(uploadableFile, place, inAppPictureLocation); initRecyclerView(); - if (callback.getIndexInViewFlipper(this) == 0) { + if (indexOfFragment == 0) { binding.btnPrevious.setEnabled(false); binding.btnPrevious.setAlpha(0.5f); } else { @@ -208,7 +230,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements } //If this is the last media, we have nothing to copy, lets not show the button - if (callback.getIndexInViewFlipper(this) == callback.getTotalNumberOfSteps() - 4) { + if (indexOfFragment == callback.getTotalNumberOfSteps() - 4) { binding.btnCopySubsequentMedia.setVisibility(View.GONE); } else { binding.btnCopySubsequentMedia.setVisibility(View.VISIBLE); @@ -273,23 +295,18 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements if (callback == null) { return; } - boolean isValidUploads = presenter.verifyImageQuality(callback.getIndexInViewFlipper(this), inAppPictureLocation); - if (!isValidUploads) { - startActivityWithFlags( - getActivity(), MainActivity.class, Intent.FLAG_ACTIVITY_CLEAR_TOP, - Intent.FLAG_ACTIVITY_SINGLE_TOP); - } + presenter.displayLocDialog(indexOfFragment, inAppPictureLocation); } public void onPreviousButtonClicked() { if (callback == null) { return; } - callback.onPreviousButtonClicked(callback.getIndexInViewFlipper(this)); + callback.onPreviousButtonClicked(indexOfFragment); } public void onEditButtonClicked() { - presenter.onEditButtonClicked(callback.getIndexInViewFlipper(this)); + presenter.onEditButtonClicked(indexOfFragment); } @Override public void showSimilarImageFragment(String originalFilePath, String possibleFilePath, @@ -302,7 +319,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements public void onPositiveResponse() { Timber.d("positive response from similar image fragment"); presenter.useSimilarPictureCoordinates(similarImageCoordinates, - callback.getIndexInViewFlipper(UploadMediaDetailFragment.this)); + indexOfFragment); // set the description text when user selects to use coordinate from the other image // which was taken within 120s @@ -346,13 +363,13 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements if (callback == null) { return; } - if (callback.getIndexInViewFlipper(this) == 0) { + if (indexOfFragment == 0) { if (UploadActivity.nearbyPopupAnswers.containsKey(nearbyPlace)) { final boolean response = UploadActivity.nearbyPopupAnswers.get(nearbyPlace); if (response) { if (callback != null) { presenter.onUserConfirmedUploadIsOfPlace(nearbyPlace, - callback.getIndexInViewFlipper(this)); + indexOfFragment); } } } else { @@ -379,7 +396,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements place.getName()), () -> { UploadActivity.nearbyPopupAnswers.put(place, true); - presenter.onUserConfirmedUploadIsOfPlace(place, callback.getIndexInViewFlipper(this)); + presenter.onUserConfirmedUploadIsOfPlace(place, indexOfFragment); }, () -> { UploadActivity.nearbyPopupAnswers.put(place, false); @@ -400,7 +417,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements if (callback == null) { return; } - callback.onNextButtonClicked(callback.getIndexInViewFlipper(this)); + callback.onNextButtonClicked(indexOfFragment); } /** @@ -412,15 +429,13 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements if (callback == null) { return; } - presenter.fetchTitleAndDescription(callback.getIndexInViewFlipper(this)); + presenter.fetchTitleAndDescription(indexOfFragment); if (showNearbyFound) { if (UploadActivity.nearbyPopupAnswers.containsKey(nearbyPlace)) { final boolean response = UploadActivity.nearbyPopupAnswers.get(nearbyPlace); if (response) { - if (callback != null) { - presenter.onUserConfirmedUploadIsOfPlace(nearbyPlace, - callback.getIndexInViewFlipper(this)); - } + presenter.onUserConfirmedUploadIsOfPlace(nearbyPlace, + indexOfFragment); } } else { showNearbyPlaceFound(nearbyPlace); @@ -466,51 +481,70 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements false); } else { uploadItem.setImageQuality(ImageUtils.IMAGE_KEEP); - // Calling below, instead of onNextButtonClicked() to not show locationDialog twice onImageValidationSuccess(); } } + /** + * Shows a dialog alerting the user that internet connection is required for upload process + * Does nothing if there is network connectivity and then the user presses okay + */ @Override - public void showBadImagePopup(Integer errorCode, - UploadItem uploadItem) { - String errorMessageForResult = getErrorMessageForResult(getContext(), errorCode); - if (!StringUtils.isBlank(errorMessageForResult)) { - DialogUtil.showAlertDialog(getActivity(), - getString(R.string.upload_problem_image), - errorMessageForResult, - getString(R.string.upload), - getString(R.string.cancel), - () -> { - /* - User skipped the warning of low quality image, so we call - onImageValidationSuccess rather than onNextButtonClicked to avoid showing - other warning popups again. - */ - - // validate image only when same file name error does not occur - // show the same file name error if exists. - // If image with same file name exists check the bit in errorCode is set or not - if ((errorCode & FILE_NAME_EXISTS) != 0) { - Timber.d("Trying to show duplicate picture popup"); - showDuplicatePicturePopup(uploadItem); - } else { - uploadItem.setImageQuality(ImageUtils.IMAGE_KEEP); - onImageValidationSuccess(); - } - }, - () -> deleteThisPicture() - ); - } - //If the error message is null, we will probably not show anything - } - - @Override - public void showConnectionErrorPopup() { + public void showConnectionErrorPopupForCaptionCheck() { DialogUtil.showAlertDialog(getActivity(), getString(R.string.upload_connection_error_alert_title), - getString(R.string.upload_connection_error_alert_detail), getString(R.string.ok), - () -> {}, true); + getString(R.string.upload_connection_error_alert_detail), + getString(R.string.ok), + getString(R.string.cancel_upload), + () -> { + if (!NetworkUtils.isInternetConnectionEstablished(activity)) { + showConnectionErrorPopupForCaptionCheck(); + } + }, + () -> { + activity.finish(); + }); + } + + /** + * Shows a dialog alerting the user that internet connection is required for upload process + * Recalls UploadMediaPresenter.getImageQuality for all the next upload items, + * if there is network connectivity and then the user presses okay + */ + @Override + public void showConnectionErrorPopup() { + try { + boolean FLAG_ALERT_DIALOG_SHOWING = basicKvStore.getBoolean( + keyForShowingAlertDialog, false); + if (!FLAG_ALERT_DIALOG_SHOWING) { + basicKvStore.putBoolean(keyForShowingAlertDialog, true); + DialogUtil.showAlertDialog(getActivity(), + getString(R.string.upload_connection_error_alert_title), + getString(R.string.upload_connection_error_alert_detail), + getString(R.string.ok), + getString(R.string.cancel_upload), + () -> { + basicKvStore.putBoolean(keyForShowingAlertDialog, false); + if (NetworkUtils.isInternetConnectionEstablished(activity)) { + int sizeOfUploads = basicKvStore.getInt( + UploadActivity.keyForCurrentUploadImagesSize); + for (int i = indexOfFragment; i < sizeOfUploads; i++) { + presenter.getImageQuality(i, inAppPictureLocation, activity); + } + } else { + showConnectionErrorPopup(); + } + }, + () -> { + basicKvStore.putBoolean(keyForShowingAlertDialog, false); + activity.finish(); + }, + null, + false + ); + } + } catch (Exception e) { + } } @Override @@ -622,9 +656,8 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements binding.backgroundImage.setImageURI(Uri.fromFile(new File(result))); } editableUploadItem.setContentUri(Uri.fromFile(new File(result))); - if (callback != null) { - callback.changeThumbnail(callback.getIndexInViewFlipper(this), result); - } + callback.changeThumbnail(indexOfFragment, + result); } catch (Exception e) { Timber.e(e); } @@ -711,13 +744,6 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements onSkipClicked); } - private void deleteThisPicture() { - if (callback == null) { - return; - } - callback.deletePictureAtIndex(callback.getIndexInViewFlipper(this)); - } - @Override public void onDestroyView() { super.onDestroyView(); @@ -745,7 +771,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements if (callback == null) { return; } - presenter.onMapIconClicked(callback.getIndexInViewFlipper(this)); + presenter.onMapIconClicked(indexOfFragment); } @Override @@ -781,10 +807,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements public void onButtonCopyTitleDescToSubsequentMedia(){ - if (callback == null) { - return; - } - presenter.copyTitleAndDescriptionToSubsequentMedia(callback.getIndexInViewFlipper(this)); + presenter.copyTitleAndDescriptionToSubsequentMedia(indexOfFragment); Toast.makeText(getContext(), getResources().getString(R.string.copied_successfully), Toast.LENGTH_SHORT).show(); } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailsContract.java b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailsContract.java index 0869b7eaa..254b44f72 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailsContract.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailsContract.java @@ -1,5 +1,6 @@ package fr.free.nrw.commons.upload.mediaDetails; +import android.app.Activity; import fr.free.nrw.commons.BasePresenter; import fr.free.nrw.commons.filepicker.UploadableFile; import fr.free.nrw.commons.location.LatLng; @@ -31,10 +32,19 @@ public interface UploadMediaDetailsContract { void showDuplicatePicturePopup(UploadItem uploadItem); - void showBadImagePopup(Integer errorCode, UploadItem uploadItem); - + /** + * Shows a dialog alerting the user that internet connection is required for upload process + * Recalls UploadMediaPresenter.getImageQuality for all the next upload items, + * if there is network connectivity and then the user presses okay + */ void showConnectionErrorPopup(); + /** + * Shows a dialog alerting the user that internet connection is required for upload process + * Does nothing if there is network connectivity and then the user presses okay + */ + void showConnectionErrorPopupForCaptionCheck(); + void showExternalMap(UploadItem uploadItem); void showEditActivity(UploadItem uploadItem); @@ -50,7 +60,41 @@ public interface UploadMediaDetailsContract { void setUploadMediaDetails(List uploadMediaDetails, int uploadItemIndex); - boolean verifyImageQuality(int uploadItemIndex, LatLng inAppPictureLocation); + /** + * Calculates the image quality + * + * @param uploadItemIndex Index of the UploadItem whose quality is to be checked + * @param inAppPictureLocation In app picture location (if any) + * @param activity Context reference + * @return true if no internal error occurs, else returns false + */ + boolean getImageQuality(int uploadItemIndex, LatLng inAppPictureLocation, Activity activity); + + /** + * Checks if the image has a location or not. Displays a dialog alerting user that no + * location has been to added to the image and asking them to add one + * + * @param uploadItemIndex Index of the uploadItem which has no location + * @param inAppPictureLocation In app picture location (if any) + */ + void displayLocDialog(int uploadItemIndex, LatLng inAppPictureLocation); + + /** + * Used to check image quality from stored qualities and display dialogs + * + * @param uploadItem UploadItem whose quality is to be checked + * @param index Index of the UploadItem whose quality is to be checked + */ + void checkImageQuality(UploadItem uploadItem, int index); + + /** + * Updates the image qualities stored in JSON, whenever an image is deleted + * + * @param size Size of uploadableFiles + * @param index Index of the UploadItem which was deleted + */ + void updateImageQualitiesJSON(int size, int index); + void copyTitleAndDescriptionToSubsequentMedia(int indexInViewFlipper); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java index 05390544c..2e93a4d21 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java @@ -2,24 +2,31 @@ package fr.free.nrw.commons.upload.mediaDetails; import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; +import static fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.activity; import static fr.free.nrw.commons.utils.ImageUtils.EMPTY_CAPTION; import static fr.free.nrw.commons.utils.ImageUtils.FILE_NAME_EXISTS; import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_KEEP; import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK; +import static fr.free.nrw.commons.utils.ImageUtils.getErrorMessageForResult; +import android.app.Activity; import androidx.annotation.Nullable; import fr.free.nrw.commons.R; import fr.free.nrw.commons.filepicker.UploadableFile; +import fr.free.nrw.commons.kvstore.BasicKvStore; import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.location.LatLng; import fr.free.nrw.commons.nearby.Place; import fr.free.nrw.commons.repository.UploadRepository; import fr.free.nrw.commons.upload.ImageCoordinates; import fr.free.nrw.commons.upload.SimilarImageInterface; +import fr.free.nrw.commons.upload.UploadActivity; import fr.free.nrw.commons.upload.UploadItem; import fr.free.nrw.commons.upload.UploadMediaDetail; +import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.UploadMediaDetailFragmentCallback; import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailsContract.UserActionListener; import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailsContract.View; +import fr.free.nrw.commons.utils.DialogUtil; import io.github.coordinates2country.Coordinates2Country; import io.reactivex.Maybe; import io.reactivex.Scheduler; @@ -35,7 +42,9 @@ import java.util.Locale; import java.util.Map; import javax.inject.Inject; import javax.inject.Named; +import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; +import org.json.JSONObject; import timber.log.Timber; public class UploadMediaPresenter implements UserActionListener, SimilarImageInterface { @@ -55,9 +64,18 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt private Scheduler ioScheduler; private Scheduler mainThreadScheduler; + public static UploadMediaDetailFragmentCallback presenterCallback ; + private final List WLM_SUPPORTED_COUNTRIES= Arrays.asList("am","at","az","br","hr","sv","fi","fr","de","gh","in","ie","il","mk","my","mt","pk","pe","pl","ru","rw","si","es","se","tw","ug","ua","us"); private Map countryNamesAndCodes = null; + private final String keyForCurrentUploadImageQualities = "UploadedImagesQualities"; + + /** + * Variable used to determine if the battery-optimisation dialog is being shown or not + */ + public static boolean isBatteryDialogShowing; + @Inject public UploadMediaPresenter(UploadRepository uploadRepository, @Named("default_preferences") JsonKvStore defaultKVStore, @@ -123,10 +141,10 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt { view.onImageProcessed(uploadItem, place); view.updateMediaDetails(uploadItem.getUploadMediaDetails()); + view.showProgress(false); final ImageCoordinates gpsCoords = uploadItem.getGpsCoords(); final boolean hasImageCoordinates = - gpsCoords != null && gpsCoords.getImageCoordsExists(); - view.showProgress(false); + gpsCoords != null && gpsCoords.getImageCoordsExists(); if (hasImageCoordinates && place == null) { checkNearbyPlaces(uploadItem); } @@ -184,68 +202,78 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt } /** - * asks the repository to verify image quality + * Checks if the image has a location or not. Displays a dialog alerting user that no + * location has been to added to the image and asking them to add one * - * @param uploadItemIndex + * @param uploadItemIndex Index of the uploadItem which has no location + * @param inAppPictureLocation In app picture location (if any) */ @Override - public boolean verifyImageQuality(int uploadItemIndex, LatLng inAppPictureLocation) { - final List uploadItems = repository.getUploads(); - if (uploadItems.size()==0) { - view.showProgress(false); - // No internationalization required for this error message because it's an internal error. - view.showMessage("Internal error: Zero upload items received by the Upload Media Detail Fragment. Sorry, please upload again.",R.color.color_error); - return false; - } - UploadItem uploadItem = uploadItems.get(uploadItemIndex); + public void displayLocDialog(int uploadItemIndex, LatLng inAppPictureLocation) { + final List uploadItems = repository.getUploads(); + UploadItem uploadItem = uploadItems.get(uploadItemIndex); + if (uploadItem.getGpsCoords().getDecimalCoords() == null && inAppPictureLocation == null) { + final Runnable onSkipClicked = () -> { + verifyCaptionQuality(uploadItem); + }; + view.displayAddLocationDialog(onSkipClicked); + } else { + verifyCaptionQuality(uploadItem); + } + } - if (uploadItem.getGpsCoords().getDecimalCoords() == null && inAppPictureLocation == null) { - final Runnable onSkipClicked = () -> { - view.showProgress(true); - compositeDisposable.add( - repository - .getImageQuality(uploadItem, inAppPictureLocation) - .observeOn(mainThreadScheduler) - .subscribe(imageResult -> { - view.showProgress(false); - handleImageResult(imageResult, uploadItem); - }, - throwable -> { - view.showProgress(false); - if (throwable instanceof UnknownHostException) { - view.showConnectionErrorPopup(); - } else { - view.showMessage("" + throwable.getLocalizedMessage(), - R.color.color_error); - } - Timber.e(throwable, "Error occurred while handling image"); - }) - ); - }; - view.displayAddLocationDialog(onSkipClicked); - } else { - view.showProgress(true); - compositeDisposable.add( - repository - .getImageQuality(uploadItem, inAppPictureLocation) - .observeOn(mainThreadScheduler) - .subscribe(imageResult -> { - view.showProgress(false); - handleImageResult(imageResult, uploadItem); - }, - throwable -> { - view.showProgress(false); - if (throwable instanceof UnknownHostException) { - view.showConnectionErrorPopup(); - } else { - view.showMessage("" + throwable.getLocalizedMessage(), - R.color.color_error); - } - Timber.e(throwable, "Error occurred while handling image"); - }) - ); - } - return true; + /** + * Verifies the image's caption and calls function to handle the result + * + * @param uploadItem UploadItem whose caption is checked + */ + private void verifyCaptionQuality(UploadItem uploadItem) { + view.showProgress(true); + compositeDisposable.add( + repository + .getCaptionQuality(uploadItem) + .observeOn(mainThreadScheduler) + .subscribe(capResult -> { + view.showProgress(false); + handleCaptionResult(capResult, uploadItem); + }, + throwable -> { + view.showProgress(false); + if (throwable instanceof UnknownHostException) { + view.showConnectionErrorPopupForCaptionCheck(); + } else { + view.showMessage("" + throwable.getLocalizedMessage(), + R.color.color_error); + } + Timber.e(throwable, "Error occurred while handling image"); + }) + ); + } + + /** + * Handles image's caption results and shows dialog if necessary + * + * @param errorCode Error code of the UploadItem + * @param uploadItem UploadItem whose caption is checked + */ + public void handleCaptionResult(Integer errorCode, UploadItem uploadItem) { + // If errorCode is empty caption show message + if (errorCode == EMPTY_CAPTION) { + Timber.d("Captions are empty. Showing toast"); + view.showMessage(R.string.add_caption_toast, R.color.color_error); + } + + // If image with same file name exists check the bit in errorCode is set or not + if ((errorCode & FILE_NAME_EXISTS) != 0) { + Timber.d("Trying to show duplicate picture popup"); + view.showDuplicatePicturePopup(uploadItem); + } + + // If caption is not duplicate or user still wants to upload it + if (errorCode == IMAGE_OK) { + Timber.d("Image captions are okay or user still wants to upload it"); + view.onImageValidationSuccess(); + } } @@ -309,50 +337,193 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt view.updateMediaDetails(uploadMediaDetails); } - /** - * handles image quality verifications + /** + * Calculates the image quality * - * @param imageResult - * @param uploadItem - */ - public void handleImageResult(Integer imageResult, + * @param uploadItemIndex Index of the UploadItem whose quality is to be checked + * @param inAppPictureLocation In app picture location (if any) + * @param activity Context reference + * @return true if no internal error occurs, else returns false + */ + @Override + public boolean getImageQuality(int uploadItemIndex, LatLng inAppPictureLocation, + Activity activity) { + final List uploadItems = repository.getUploads(); + view.showProgress(true); + if (uploadItems.size() == 0) { + view.showProgress(false); + // No internationalization required for this error message because it's an internal error. + view.showMessage( + "Internal error: Zero upload items received by the Upload Media Detail Fragment. Sorry, please upload again.", + R.color.color_error); + return false; + } + UploadItem uploadItem = uploadItems.get(uploadItemIndex); + compositeDisposable.add( + repository + .getImageQuality(uploadItem, inAppPictureLocation) + .observeOn(mainThreadScheduler) + .subscribe(imageResult -> { + storeImageQuality(imageResult, uploadItemIndex, activity, uploadItem); + }, + throwable -> { + if (throwable instanceof UnknownHostException) { + view.showProgress(false); + view.showConnectionErrorPopup(); + } else { + view.showMessage("" + throwable.getLocalizedMessage(), + R.color.color_error); + } + Timber.e(throwable, "Error occurred while handling image"); + }) + ); + return true; + } + + /** + * Stores the image quality in JSON format in SharedPrefs + * + * @param imageResult Image quality + * @param uploadItemIndex Index of the UploadItem whose quality is calculated + * @param activity Context reference + * @param uploadItem UploadItem whose quality is to be checked + */ + private void storeImageQuality(Integer imageResult, int uploadItemIndex, Activity activity, UploadItem uploadItem) { - if (imageResult == IMAGE_KEEP || imageResult == IMAGE_OK) { - view.onImageValidationSuccess(); - uploadItem.setHasInvalidLocation(false); - } else { - handleBadImage(imageResult, uploadItem); + BasicKvStore store = new BasicKvStore(activity, + UploadActivity.storeNameForCurrentUploadImagesSize); + String value = store.getString(keyForCurrentUploadImageQualities, null); + JSONObject jsonObject; + try { + if (value != null) { + jsonObject = new JSONObject(value); + } else { + jsonObject = new JSONObject(); + } + jsonObject.put("UploadItem" + uploadItemIndex, imageResult); + store.putString(keyForCurrentUploadImageQualities, jsonObject.toString()); + } catch (Exception e) { + } + + if (uploadItemIndex == 0) { + if (!isBatteryDialogShowing) { + // if battery-optimisation dialog is not being shown, call checkImageQuality + checkImageQuality(uploadItem, uploadItemIndex); + } else { + view.showProgress(false); + } } } /** - * Handle images, say empty caption, duplicate file name, bad picture(in all other cases) + * Used to check image quality from stored qualities and display dialogs * - * @param errorCode - * @param uploadItem + * @param uploadItem UploadItem whose quality is to be checked + * @param index Index of the UploadItem whose quality is to be checked + */ + @Override + public void checkImageQuality(UploadItem uploadItem, int index) { + if ((uploadItem.getImageQuality() != IMAGE_OK) && (uploadItem.getImageQuality() + != IMAGE_KEEP)) { + BasicKvStore store = new BasicKvStore(activity, + UploadActivity.storeNameForCurrentUploadImagesSize); + String value = store.getString(keyForCurrentUploadImageQualities, null); + JSONObject jsonObject; + try { + if (value != null) { + jsonObject = new JSONObject(value); + } else { + jsonObject = new JSONObject(); + } + Integer imageQuality = (int) jsonObject.get("UploadItem" + index); + view.showProgress(false); + if (imageQuality == IMAGE_OK) { + uploadItem.setHasInvalidLocation(false); + uploadItem.setImageQuality(imageQuality); + } else { + handleBadImage(imageQuality, uploadItem, index); + } + } catch (Exception e) { + } + } + } + + /** + * Updates the image qualities stored in JSON, whenever an image is deleted + * + * @param size Size of uploadableFiles + * @param index Index of the UploadItem which was deleted + */ + @Override + public void updateImageQualitiesJSON(int size, int index) { + BasicKvStore store = new BasicKvStore(activity, + UploadActivity.storeNameForCurrentUploadImagesSize); + String value = store.getString(keyForCurrentUploadImageQualities, null); + JSONObject jsonObject; + try { + if (value != null) { + jsonObject = new JSONObject(value); + } else { + jsonObject = new JSONObject(); + } + for (int i = index; i < (size - 1); i++) { + jsonObject.put("UploadItem" + i, jsonObject.get("UploadItem" + (i + 1))); + } + jsonObject.remove("UploadItem" + (size - 1)); + store.putString(keyForCurrentUploadImageQualities, jsonObject.toString()); + } catch (Exception e) { + } + } + + /** + * Handles bad pictures, like too dark, already on wikimedia, downloaded from internet + * + * @param errorCode Error code of the bad image quality + * @param uploadItem UploadItem whose quality is bad + * @param index Index of item whose quality is bad */ public void handleBadImage(Integer errorCode, - UploadItem uploadItem) { + UploadItem uploadItem, int index) { Timber.d("Handle bad picture with error code %d", errorCode); - if (errorCode - >= 8) { // If location of image and nearby does not match + if (errorCode >= 8) { // If location of image and nearby does not match uploadItem.setHasInvalidLocation(true); } - // If errorCode is empty caption show message - if (errorCode == EMPTY_CAPTION) { - Timber.d("Captions are empty. Showing toast"); - view.showMessage(R.string.add_caption_toast, R.color.color_error); + // If image has some other problems, show popup accordingly + if (errorCode != EMPTY_CAPTION && errorCode != FILE_NAME_EXISTS) { + showBadImagePopup(errorCode, index, activity, uploadItem); } - // If image has some problems, show popup accordingly - if (errorCode != EMPTY_CAPTION && errorCode != FILE_NAME_EXISTS) { - view.showBadImagePopup(errorCode, uploadItem); - } else if ((errorCode & FILE_NAME_EXISTS) != 0) { - // When image has duplicate caption problem - Timber.d("Trying to show duplicate picture popup"); - view.showDuplicatePicturePopup(uploadItem); + } + + /** + * Shows a dialog describing the potential problems in the current image + * + * @param errorCode Has the potential problems in the current image + * @param index Index of the UploadItem which has problems + * @param activity Context reference + * @param uploadItem UploadItem which has problems + */ + public void showBadImagePopup(Integer errorCode, + int index, Activity activity, UploadItem uploadItem) { + String errorMessageForResult = getErrorMessageForResult(activity, errorCode); + if (!StringUtils.isBlank(errorMessageForResult)) { + DialogUtil.showAlertDialog(activity, + activity.getString(R.string.upload_problem_image), + errorMessageForResult, + activity.getString(R.string.upload), + activity.getString(R.string.cancel), + () -> { + view.showProgress(false); + uploadItem.setImageQuality(IMAGE_OK); + }, + () -> { + presenterCallback.deletePictureAtIndex(index); + } + ).setCancelable(false); + } else { } + //If the error message is null, we will probably not show anything } /** diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/ImageProcessingServiceTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/ImageProcessingServiceTest.kt index 9efcfa468..cf720c6a4 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/ImageProcessingServiceTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/ImageProcessingServiceTest.kt @@ -128,7 +128,7 @@ class ImageProcessingServiceTest { fun validateImageForFileNameExistsWithCheckTitleOn() { `when`(mediaClient?.checkPageExistsUsingTitle(ArgumentMatchers.anyString())) .thenReturn(Single.just(true)) - val validateImage = imageProcessingService!!.validateImage(uploadItem, location) + val validateImage = imageProcessingService!!.validateCaption(uploadItem) assertEquals(ImageUtils.FILE_NAME_EXISTS, validateImage.blockingGet()) } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt index 4862a6b2b..bf3aa5d68 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt @@ -69,6 +69,9 @@ class UploadMediaPresenterTest { @Mock private lateinit var jsonKvStore: JsonKvStore + @Mock + lateinit var mockActivity: UploadActivity + /** * initial setup unit test environment */ @@ -79,8 +82,9 @@ class UploadMediaPresenterTest { testObservableUploadItem = Observable.just(uploadItem) testSingleImageResult = Single.just(1) testScheduler = TestScheduler() - uploadMediaPresenter = UploadMediaPresenter(repository, - jsonKvStore,testScheduler, testScheduler) + uploadMediaPresenter = UploadMediaPresenter( + repository, jsonKvStore, testScheduler, testScheduler + ) uploadMediaPresenter.onAttachView(view) mockedCountry = mockStatic(Coordinates2Country::class.java) } @@ -111,14 +115,13 @@ class UploadMediaPresenterTest { ArgumentMatchers.any(UploadItem::class.java), ArgumentMatchers.any(Place::class.java) ) - verify(view).showProgress(false) } /** - * unit test for method UploadMediaPresenter.verifyImageQuality (For else case) + * unit test for method UploadMediaPresenter.getImageQuality (For else case) */ @Test - fun verifyImageQualityTest() { + fun getImageQualityTest() { whenever(repository.uploads).thenReturn(listOf(uploadItem)) whenever(repository.getImageQuality(uploadItem, location)) .thenReturn(testSingleImageResult) @@ -127,17 +130,16 @@ class UploadMediaPresenterTest { .thenReturn(imageCoordinates) whenever(uploadItem.gpsCoords.decimalCoords) .thenReturn("imageCoordinates") - uploadMediaPresenter.verifyImageQuality(0, location) + uploadMediaPresenter.getImageQuality(0, location, mockActivity) verify(view).showProgress(true) testScheduler.triggerActions() - verify(view).showProgress(false) } /** - * unit test for method UploadMediaPresenter.verifyImageQuality (For if case) + * unit test for method UploadMediaPresenter.getImageQuality (For if case) */ @Test - fun `verify ImageQuality Test while coordinates equals to null`() { + fun `get ImageQuality Test while coordinates equals to null`() { whenever(repository.uploads).thenReturn(listOf(uploadItem)) whenever(repository.getImageQuality(uploadItem, location)) .thenReturn(testSingleImageResult) @@ -146,30 +148,35 @@ class UploadMediaPresenterTest { .thenReturn(imageCoordinates) whenever(uploadItem.gpsCoords.decimalCoords) .thenReturn(null) - uploadMediaPresenter.verifyImageQuality(0, location) + uploadMediaPresenter.getImageQuality(0, location, mockActivity) testScheduler.triggerActions() } /** - * unit test for method UploadMediaPresenter.handleImageResult + * Test for empty file name when the user presses the NEXT button */ @Test - fun handleImageResult() { - //Positive case test - uploadMediaPresenter.handleImageResult(IMAGE_KEEP, uploadItem) - verify(view).onImageValidationSuccess() - - //Duplicate file name - uploadMediaPresenter.handleImageResult(FILE_NAME_EXISTS, uploadItem) - verify(view).showDuplicatePicturePopup(uploadItem) - - //Empty Caption test - uploadMediaPresenter.handleImageResult(EMPTY_CAPTION, uploadItem) + fun emptyFileNameTest() { + uploadMediaPresenter.handleCaptionResult(EMPTY_CAPTION, uploadItem); verify(view).showMessage(ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()) + } - // Bad Picture Test - uploadMediaPresenter.handleImageResult(-7, uploadItem) - verify(view)?.showBadImagePopup(ArgumentMatchers.anyInt(), ArgumentMatchers.eq(uploadItem)) + /** + * Test for duplicate file name when the user presses the NEXT button + */ + @Test + fun duplicateFileNameTest() { + uploadMediaPresenter.handleCaptionResult(FILE_NAME_EXISTS, uploadItem) + verify(view).showDuplicatePicturePopup(uploadItem) + } + + /** + * Test for correct file name when the user presses the NEXT button + */ + @Test + fun correctFileNameTest() { + uploadMediaPresenter.handleCaptionResult(IMAGE_OK, uploadItem) + verify(view).onImageValidationSuccess() } @Test @@ -218,34 +225,6 @@ class UploadMediaPresenterTest { verify(view).updateMediaDetails(ArgumentMatchers.any()) } - /** - * Test bad image invalid location - */ - @Test - fun handleBadImageBaseTestInvalidLocation() { - uploadMediaPresenter.handleBadImage(8, uploadItem) - verify(view).showBadImagePopup(8, uploadItem) - } - - /** - * Test bad image empty title - */ - @Test - fun handleBadImageBaseTestEmptyTitle() { - uploadMediaPresenter.handleBadImage(-3, uploadItem) - verify(view).showMessage(ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()) - } - - /** - * Teste show file already exists - */ - @Test - fun handleBadImageBaseTestFileNameExists() { - uploadMediaPresenter.handleBadImage(64, uploadItem) - verify(view).showDuplicatePicturePopup(uploadItem) - } - - /** * Test show SimilarImageFragment */ @@ -259,7 +238,8 @@ class UploadMediaPresenterTest { @Test fun setCorrectCountryCodeForReceivedImage() { - val germanyAsPlace = Place(null,null, null, null, LatLng(50.1, 10.2, 1.0f), null, null, null, true) + val germanyAsPlace = + Place(null, null, null, null, LatLng(50.1, 10.2, 1.0f), null, null, null, true) germanyAsPlace.isMonument = true whenever( @@ -269,7 +249,8 @@ class UploadMediaPresenterTest { ) ).thenReturn("Germany") - val item: Observable = Observable.just(UploadItem(Uri.EMPTY, null, null, germanyAsPlace, 0, null, null, null)) + val item: Observable = + Observable.just(UploadItem(Uri.EMPTY, null, null, germanyAsPlace, 0, null, null, null)) whenever( repository.preProcessImage( @@ -291,7 +272,5 @@ class UploadMediaPresenterTest { ) assertEquals("Exptected contry code", "de", captor.value.countryCode); - - verify(view).showProgress(false) } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadRepositoryUnitTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadRepositoryUnitTest.kt index a5850bf91..209566931 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadRepositoryUnitTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadRepositoryUnitTest.kt @@ -191,6 +191,15 @@ class UploadRepositoryUnitTest { ) } + @Test + fun testGetCaptionQuality() { + assertEquals( + repository.getCaptionQuality(uploadItem), + uploadModel.getCaptionQuality(uploadItem) + ) + } + + @Test fun testDeletePicture() { assertEquals(repository.deletePicture(""), uploadModel.deletePicture("")) diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragmentUnitTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragmentUnitTest.kt index 0b209be9a..fabe51a0f 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragmentUnitTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragmentUnitTest.kt @@ -321,18 +321,12 @@ class UploadMediaDetailFragmentUnitTest { fragment.showDuplicatePicturePopup(uploadItem) } - @Test - @Throws(Exception::class) - fun testShowBadImagePopup() { - Shadows.shadowOf(Looper.getMainLooper()).idle() - fragment.showBadImagePopup(8, uploadItem) - } @Test @Throws(Exception::class) - fun testShowConnectionErrorPopup() { + fun testShowConnectionErrorPopupForCaptionCheck() { Shadows.shadowOf(Looper.getMainLooper()).idle() - fragment.showConnectionErrorPopup() + fragment.showConnectionErrorPopupForCaptionCheck() } @Test @@ -364,7 +358,7 @@ class UploadMediaDetailFragmentUnitTest { `when`(latLng.longitude).thenReturn(0.0) `when`(uploadItem.gpsCoords).thenReturn(imageCoordinates) fragment.onActivityResult(1211, Activity.RESULT_OK, intent) - Mockito.verify(presenter, Mockito.times(0)).verifyImageQuality(0, location) + Mockito.verify(presenter, Mockito.times(0)).getImageQuality(0, location, activity) } @Test @@ -388,7 +382,7 @@ class UploadMediaDetailFragmentUnitTest { `when`(latLng.longitude).thenReturn(0.0) `when`(uploadItem.gpsCoords).thenReturn(imageCoordinates) fragment.onActivityResult(1211, Activity.RESULT_OK, intent) - Mockito.verify(presenter, Mockito.times(1)).verifyImageQuality(0, null) + Mockito.verify(presenter, Mockito.times(1)).displayLocDialog(0, null) } @Test @@ -398,17 +392,6 @@ class UploadMediaDetailFragmentUnitTest { fragment.updateMediaDetails(null) } - @Test - @Throws(Exception::class) - fun testDeleteThisPicture() { - Shadows.shadowOf(Looper.getMainLooper()).idle() - val method: Method = UploadMediaDetailFragment::class.java.getDeclaredMethod( - "deleteThisPicture" - ) - method.isAccessible = true - method.invoke(fragment) - } - @Test @Throws(Exception::class) fun testOnDestroyView() {