From e135fea20d216a771df83b05fd26bf8f7f3fb202 Mon Sep 17 00:00:00 2001 From: Ayan Sarkar <71203077+Ayan-10@users.noreply.github.com> Date: Sat, 5 Feb 2022 06:26:59 +0530 Subject: [PATCH] Fixes 1848 : Option for adding location for those pictures which have no location (#4755) * Location wizard * Location wizard * Minor fix * message changed * Test fail fixed * Test fail fixed * Test fail fixed * last location triggered * last location added * Some test added * Java docs added * More java docs added --- .../LocationPickerActivity.java | 20 +++++- .../nrw/commons/di/ActivityBuilderModule.java | 4 ++ .../UploadMediaDetailFragment.java | 65 +++++++++++++++---- .../UploadMediaDetailsContract.java | 4 +- .../mediaDetails/UploadMediaPresenter.java | 63 ++++++++++++------ .../fragment_upload_media_detail_fragment.xml | 2 +- app/src/main/res/values/strings.xml | 3 + .../upload/UploadMediaPresenterTest.kt | 28 +++++++- .../UploadMediaDetailFragmentUnitTest.kt | 16 +++-- 9 files changed, 161 insertions(+), 44 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/LocationPicker/LocationPickerActivity.java b/app/src/main/java/fr/free/nrw/commons/LocationPicker/LocationPickerActivity.java index 03bd678af..dee710794 100644 --- a/app/src/main/java/fr/free/nrw/commons/LocationPicker/LocationPickerActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/LocationPicker/LocationPickerActivity.java @@ -6,6 +6,7 @@ import static com.mapbox.mapboxsdk.style.layers.PropertyFactory.iconAllowOverlap import static com.mapbox.mapboxsdk.style.layers.PropertyFactory.iconIgnorePlacement; import static com.mapbox.mapboxsdk.style.layers.PropertyFactory.iconImage; import static com.mapbox.mapboxsdk.style.layers.PropertyFactory.visibility; +import static fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.LAST_LOCATION; import android.content.Intent; import android.graphics.BitmapFactory; @@ -49,13 +50,17 @@ import com.mapbox.mapboxsdk.style.layers.SymbolLayer; import com.mapbox.mapboxsdk.style.sources.GeoJsonSource; import fr.free.nrw.commons.R; import fr.free.nrw.commons.Utils; +import fr.free.nrw.commons.kvstore.JsonKvStore; +import fr.free.nrw.commons.theme.BaseActivity; +import javax.inject.Inject; +import javax.inject.Named; import org.jetbrains.annotations.NotNull; import timber.log.Timber; /** * Helps to pick location and return the result with an intent */ -public class LocationPickerActivity extends AppCompatActivity implements OnMapReadyCallback, +public class LocationPickerActivity extends BaseActivity implements OnMapReadyCallback, OnCameraMoveStartedListener, OnCameraIdleListener, Observer { /** @@ -114,6 +119,13 @@ public class LocationPickerActivity extends AppCompatActivity implements OnMapRe * smallToolbarText : textView of shadow */ private TextView smallToolbarText; + /** + * applicationKvStore : for storing values + */ + @Inject + @Named("default_preferences") + public + JsonKvStore applicationKvStore; @Override protected void onCreate(@Nullable final Bundle savedInstanceState) { @@ -365,6 +377,12 @@ public class LocationPickerActivity extends AppCompatActivity implements OnMapRe * Return the intent with required data */ void placeSelected() { + if (activity.equals("NoLocationUploadActivity")) { + applicationKvStore.putString(LAST_LOCATION, + mapboxMap.getCameraPosition().target.getLatitude() + + "," + + mapboxMap.getCameraPosition().target.getLongitude()); + } final Intent returningIntent = new Intent(); returningIntent.putExtra(LocationPickerConstants.MAP_CAMERA_POSITION, mapboxMap.getCameraPosition()); diff --git a/app/src/main/java/fr/free/nrw/commons/di/ActivityBuilderModule.java b/app/src/main/java/fr/free/nrw/commons/di/ActivityBuilderModule.java index 6381bdc8e..7607ba049 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/ActivityBuilderModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/ActivityBuilderModule.java @@ -3,6 +3,7 @@ package fr.free.nrw.commons.di; import dagger.Module; import dagger.android.ContributesAndroidInjector; import fr.free.nrw.commons.AboutActivity; +import fr.free.nrw.commons.LocationPicker.LocationPickerActivity; import fr.free.nrw.commons.WelcomeActivity; import fr.free.nrw.commons.auth.LoginActivity; import fr.free.nrw.commons.auth.SignupActivity; @@ -44,6 +45,9 @@ public abstract class ActivityBuilderModule { @ContributesAndroidInjector abstract AboutActivity bindAboutActivity(); + @ContributesAndroidInjector + abstract LocationPickerActivity bindLocationPickerActivity(); + @ContributesAndroidInjector abstract SignupActivity bindSignupActivity(); 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 3ab9dc2b3..68af49522 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 @@ -44,6 +44,7 @@ import fr.free.nrw.commons.utils.ImageUtils; import fr.free.nrw.commons.utils.ViewUtil; import java.util.List; import java.util.Locale; +import java.util.Objects; import javax.inject.Inject; import javax.inject.Named; import org.apache.commons.lang3.StringUtils; @@ -53,6 +54,12 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements UploadMediaDetailsContract.View, UploadMediaDetailAdapter.EventListener { private static final int REQUEST_CODE = 1211; + /** + * A key for applicationKvStore. + * By this key we can retrieve the location of last UploadItem ex. 12.3433,54.78897 + * from applicationKvStore. + */ + public static final String LAST_LOCATION = "last_location_while_uploading"; @BindView(R.id.tv_title) TextView tvTitle; @BindView(R.id.ib_map) @@ -397,10 +404,6 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements () -> {}, true); } - @Override public void showMapWithImageCoordinates(boolean shouldShow) { - ibMap.setVisibility(shouldShow ? View.VISIBLE : View.GONE); - } - @Override public void showExternalMap(final UploadItem uploadItem) { goToLocationPickerActivity(uploadItem); @@ -414,14 +417,36 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements private void goToLocationPickerActivity(final UploadItem uploadItem) { editableUploadItem = uploadItem; - startActivityForResult(new LocationPicker.IntentBuilder() - .defaultLocation(new CameraPosition.Builder() - .target(new com.mapbox.mapboxsdk.geometry.LatLng(uploadItem.getGpsCoords() - .getDecLatitude(), - uploadItem.getGpsCoords().getDecLongitude())) - .zoom(16).build()) - .activityKey("UploadActivity") - .build(getActivity()), REQUEST_CODE); + double defaultLatitude = 37.773972; + double defaultLongitude = -122.431297; + + if (uploadItem.getGpsCoords() + .getDecLatitude() != 0.0 && uploadItem.getGpsCoords().getDecLongitude() != 0.0) { + defaultLatitude = uploadItem.getGpsCoords() + .getDecLatitude(); + defaultLongitude = uploadItem.getGpsCoords().getDecLongitude(); + startActivityForResult(new LocationPicker.IntentBuilder() + .defaultLocation(new CameraPosition.Builder() + .target( + new com.mapbox.mapboxsdk.geometry.LatLng(defaultLatitude, defaultLongitude)) + .zoom(16).build()) + .activityKey("UploadActivity") + .build(getActivity()), REQUEST_CODE); + } else { + if (defaultKvStore.getString(LAST_LOCATION) != null) { + final String[] locationLatLng + = defaultKvStore.getString(LAST_LOCATION).split(","); + defaultLatitude = Double.parseDouble(locationLatLng[0]); + defaultLongitude = Double.parseDouble(locationLatLng[1]); + } + startActivityForResult(new LocationPicker.IntentBuilder() + .defaultLocation(new CameraPosition.Builder() + .target( + new com.mapbox.mapboxsdk.geometry.LatLng(defaultLatitude, defaultLongitude)) + .zoom(16).build()) + .activityKey("NoLocationUploadActivity") + .build(getActivity()), REQUEST_CODE); + } } /** @@ -470,6 +495,22 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements uploadMediaDetailAdapter.setItems(uploadMediaDetails); } + /** + * Showing dialog for adding location + * + * @param onSkipClicked proceed for verifying image quality + */ + @Override + public void displayAddLocationDialog(final Runnable onSkipClicked) { + DialogUtil.showAlertDialog(Objects.requireNonNull(getActivity()), + getString(R.string.no_location_found_title), + getString(R.string.no_location_found_message), + getString(R.string.add_location), + getString(R.string.skip_login), + this::onIbMapClicked, + onSkipClicked); + } + private void deleteThisPicture() { callback.deletePictureAtIndex(callback.getIndexInViewFlipper(this)); } 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 bd0a2fa12..e17a6f4da 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 @@ -34,11 +34,11 @@ public interface UploadMediaDetailsContract { void showConnectionErrorPopup(); - void showMapWithImageCoordinates(boolean shouldShow); - void showExternalMap(UploadItem uploadItem); void updateMediaDetails(List uploadMediaDetails); + + void displayAddLocationDialog(Runnable runnable); } interface UserActionListener extends BasePresenter { 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 de59a625d..c04de2681 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 @@ -114,7 +114,6 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt final ImageCoordinates gpsCoords = uploadItem.getGpsCoords(); final boolean hasImageCoordinates = gpsCoords != null && gpsCoords.getImageCoordsExists(); - view.showMapWithImageCoordinates(hasImageCoordinates); view.showProgress(false); if (hasImageCoordinates && place == null) { checkNearbyPlaces(uploadItem); @@ -169,28 +168,54 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt */ @Override public void verifyImageQuality(int uploadItemIndex) { - view.showProgress(true); - final UploadItem uploadItem = repository.getUploads().get(uploadItemIndex); - compositeDisposable.add( - repository - .getImageQuality(uploadItem) - .observeOn(mainThreadScheduler) - .subscribe(imageResult -> { - view.showProgress(false); - handleImageResult(imageResult, uploadItem); - }, - throwable -> { - view.showProgress(false); - if (throwable instanceof UnknownHostException) { + + if (uploadItem.getGpsCoords().getDecimalCoords() == null) { + final Runnable onSkipClicked = () -> { + view.showProgress(true); + compositeDisposable.add( + repository + .getImageQuality(uploadItem) + .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) + .observeOn(mainThreadScheduler) + .subscribe(imageResult -> { + view.showProgress(false); + handleImageResult(imageResult, uploadItem); + }, + throwable -> { + view.showProgress(false); + if (throwable instanceof UnknownHostException) { view.showConnectionErrorPopup(); - } else { + } else { view.showMessage("" + throwable.getLocalizedMessage(), R.color.color_error); - } - Timber.e(throwable, "Error occurred while handling image"); - }) - ); + } + Timber.e(throwable, "Error occurred while handling image"); + }) + ); + } } diff --git a/app/src/main/res/layout/fragment_upload_media_detail_fragment.xml b/app/src/main/res/layout/fragment_upload_media_detail_fragment.xml index ce020df9d..e58381772 100644 --- a/app/src/main/res/layout/fragment_upload_media_detail_fragment.xml +++ b/app/src/main/res/layout/fragment_upload_media_detail_fragment.xml @@ -61,7 +61,7 @@ android:layout_marginEnd="@dimen/standard_gap" android:layout_marginRight="@dimen/standard_gap" android:layout_toLeftOf="@id/ib_expand_collapse" - android:visibility="gone" + android:visibility="visible" app:srcCompat="@drawable/ic_map_white_24dp" tools:visibility="visible" /> diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f9d4738b3..2b4e4215e 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -682,5 +682,8 @@ Upload your first media by tapping on the add button. Apply Reset Location data helps Wiki editors find your picture, making it much more useful.\nYour recent uploads have no location.\nWe suggest you turn on location in your camera app\'s settings.\nThank you for uploading! + No location found + How about adding the place where this image was taken?\nLocation data helps Wiki editors find your picture, making it much more useful.\nThank you! + Add location Please remove from this email any information that you are not comfortable sharing publicly. Also, please be aware that your email address with which you are posting, and the associated name and profile picture, will be visible publicly. 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 114ca7194..b9584df14 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 @@ -44,6 +44,9 @@ class UploadMediaPresenterTest { @Mock private lateinit var uploadItem: UploadItem + @Mock + private lateinit var imageCoordinates: ImageCoordinates + @Mock private lateinit var uploadMediaDetails: List @@ -93,20 +96,41 @@ class UploadMediaPresenterTest { } /** - * unit test for method UploadMediaPresenter.verifyImageQuality + * unit test for method UploadMediaPresenter.verifyImageQuality (For else case) */ @Test fun verifyImageQualityTest() { whenever(repository.uploads).thenReturn(listOf(uploadItem)) whenever(repository.getImageQuality(uploadItem)) .thenReturn(testSingleImageResult) - whenever(uploadItem.imageQuality).thenReturn(ArgumentMatchers.anyInt()) + whenever(uploadItem.imageQuality).thenReturn(0) + whenever(uploadItem.gpsCoords) + .thenReturn(imageCoordinates) + whenever(uploadItem.gpsCoords.decimalCoords) + .thenReturn("imageCoordinates") uploadMediaPresenter.verifyImageQuality(0) verify(view).showProgress(true) testScheduler.triggerActions() verify(view).showProgress(false) } + /** + * unit test for method UploadMediaPresenter.verifyImageQuality (For if case) + */ + @Test + fun `verify ImageQuality Test while coordinates equals to null`() { + whenever(repository.uploads).thenReturn(listOf(uploadItem)) + whenever(repository.getImageQuality(uploadItem)) + .thenReturn(testSingleImageResult) + whenever(uploadItem.imageQuality).thenReturn(0) + whenever(uploadItem.gpsCoords) + .thenReturn(imageCoordinates) + whenever(uploadItem.gpsCoords.decimalCoords) + .thenReturn(null) + uploadMediaPresenter.verifyImageQuality(0) + testScheduler.triggerActions() + } + /** * unit test for method UploadMediaPresenter.handleImageResult */ 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 93b392858..585262b4b 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 @@ -58,6 +58,7 @@ class UploadMediaDetailFragmentUnitTest { private lateinit var context: Context private lateinit var layoutInflater: LayoutInflater private lateinit var view: View + private lateinit var runnable: Runnable private lateinit var tvTitle: TextView private lateinit var tooltip: ImageView @@ -338,13 +339,6 @@ class UploadMediaDetailFragmentUnitTest { fragment.showConnectionErrorPopup() } - @Test - @Throws(Exception::class) - fun testShowMapWithImageCoordinates() { - Shadows.shadowOf(Looper.getMainLooper()).idle() - fragment.showMapWithImageCoordinates(true) - } - @Test @Throws(Exception::class) fun testShowExternalMap() { @@ -429,4 +423,12 @@ class UploadMediaDetailFragmentUnitTest { fragment.onButtonCopyTitleDescToSubsequentMedia() } + @Test + @Throws(Exception::class) + fun testDisplayAddLocationDialog() { + Shadows.shadowOf(Looper.getMainLooper()).idle() + runnable = Runnable { } + fragment.displayAddLocationDialog(runnable) + } + } \ No newline at end of file