Skip to content

Commit fec6dba

Browse files
Fix Crash in LocationPickerActivity when device configuration is changed (commons-app#5500)
* Fix Crash in LocationPickerActivity when device configuration is changed (UI Modes) * clean up * fix * fix * fix * fix * removed faulty test * cleanup * fix and tests added
1 parent 161e2ed commit fec6dba

File tree

5 files changed

+111
-60
lines changed

5 files changed

+111
-60
lines changed

app/src/main/java/fr/free/nrw/commons/LocationPicker/LocationPicker.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import android.app.Activity;
44
import android.content.Intent;
55
import com.mapbox.mapboxsdk.camera.CameraPosition;
6+
import fr.free.nrw.commons.Media;
67

78
/**
89
* Helper class for starting the activity
@@ -52,6 +53,17 @@ public LocationPicker.IntentBuilder activityKey(
5253
return this;
5354
}
5455

56+
/**
57+
* Gets and puts media in intent
58+
* @param media Media
59+
* @return LocationPicker.IntentBuilder
60+
*/
61+
public LocationPicker.IntentBuilder media(
62+
final Media media) {
63+
intent.putExtra(LocationPickerConstants.MEDIA, media);
64+
return this;
65+
}
66+
5567
/**
5668
* Gets and sets the activity
5769
* @param activity Activity

app/src/main/java/fr/free/nrw/commons/LocationPicker/LocationPickerActivity.java

Lines changed: 82 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,10 @@
3131
import com.google.android.material.floatingactionbutton.FloatingActionButton;
3232
import com.mapbox.mapboxsdk.camera.CameraPosition;
3333
import com.mapbox.mapboxsdk.geometry.LatLng;
34+
import fr.free.nrw.commons.Media;
3435
import fr.free.nrw.commons.R;
3536
import fr.free.nrw.commons.Utils;
37+
import fr.free.nrw.commons.coordinates.CoordinateEditHelper;
3638
import fr.free.nrw.commons.filepicker.Constants;
3739
import fr.free.nrw.commons.kvstore.JsonKvStore;
3840
import fr.free.nrw.commons.___location.LocationPermissionsHelper;
@@ -41,6 +43,9 @@
4143
import fr.free.nrw.commons.___location.LocationServiceManager;
4244
import fr.free.nrw.commons.theme.BaseActivity;
4345
import fr.free.nrw.commons.utils.SystemThemeUtils;
46+
import fr.free.nrw.commons.utils.ViewUtilWrapper;
47+
import io.reactivex.android.schedulers.AndroidSchedulers;
48+
import io.reactivex.schedulers.Schedulers;
4449
import java.util.List;
4550
import javax.inject.Inject;
4651
import javax.inject.Named;
@@ -52,13 +57,22 @@
5257
import org.osmdroid.views.overlay.Overlay;
5358
import org.osmdroid.views.overlay.ScaleDiskOverlay;
5459
import org.osmdroid.views.overlay.TilesOverlay;
60+
import timber.log.Timber;
5561

5662
/**
5763
* Helps to pick ___location and return the result with an intent
5864
*/
5965
public class LocationPickerActivity extends BaseActivity implements
6066
LocationPermissionCallback {
61-
67+
/**
68+
* coordinateEditHelper: helps to edit coordinates
69+
*/
70+
@Inject
71+
CoordinateEditHelper coordinateEditHelper;
72+
/**
73+
* media : Media object
74+
*/
75+
private Media media;
6276
/**
6377
* cameraPosition : position of picker
6478
*/
@@ -125,6 +139,13 @@ public class LocationPickerActivity extends BaseActivity implements
125139
@Inject
126140
LocationServiceManager locationManager;
127141

142+
/**
143+
* Constants
144+
*/
145+
private static final String CAMERA_POS = "cameraPosition";
146+
private static final String ACTIVITY = "activity";
147+
148+
128149
@SuppressLint("ClickableViewAccessibility")
129150
@Override
130151
protected void onCreate(@Nullable final Bundle savedInstanceState) {
@@ -145,8 +166,12 @@ protected void onCreate(@Nullable final Bundle savedInstanceState) {
145166
cameraPosition = getIntent()
146167
.getParcelableExtra(LocationPickerConstants.MAP_CAMERA_POSITION);
147168
activity = getIntent().getStringExtra(LocationPickerConstants.ACTIVITY_KEY);
169+
media = getIntent().getParcelableExtra(LocationPickerConstants.MEDIA);
170+
}else{
171+
cameraPosition = savedInstanceState.getParcelable(CAMERA_POS);
172+
activity = savedInstanceState.getString(ACTIVITY);
173+
media = savedInstanceState.getParcelable("sMedia");
148174
}
149-
150175
bindViews();
151176
addBackButtonListener();
152177
addPlaceSelectedButton();
@@ -220,7 +245,10 @@ private void darkThemeSetup() {
220245
*/
221246
private void addBackButtonListener() {
222247
final ImageView backButton = findViewById(R.id.maplibre_place_picker_toolbar_back_button);
223-
backButton.setOnClickListener(view -> finish());
248+
backButton.setOnClickListener(v -> {
249+
finish();
250+
});
251+
224252
}
225253

226254
/**
@@ -311,14 +339,42 @@ void placeSelected() {
311339
+ mapView.getMapCenter().getLongitude());
312340
applicationKvStore.putString(LAST_ZOOM, mapView.getZoomLevel() + "");
313341
}
314-
final Intent returningIntent = new Intent();
315-
returningIntent.putExtra(LocationPickerConstants.MAP_CAMERA_POSITION,
316-
new CameraPosition(new LatLng(mapView.getMapCenter().getLatitude(),
317-
mapView.getMapCenter().getLongitude()), 14f, 0, 0));
318-
setResult(AppCompatActivity.RESULT_OK, returningIntent);
342+
343+
if (media == null) {
344+
final Intent returningIntent = new Intent();
345+
returningIntent.putExtra(LocationPickerConstants.MAP_CAMERA_POSITION,
346+
new CameraPosition(new LatLng(mapView.getMapCenter().getLatitude(),
347+
mapView.getMapCenter().getLongitude()), 14f, 0, 0));
348+
setResult(AppCompatActivity.RESULT_OK, returningIntent);
349+
} else {
350+
updateCoordinates(String.valueOf(mapView.getMapCenter().getLatitude()),
351+
String.valueOf(mapView.getMapCenter().getLongitude()),
352+
String.valueOf(0.0f));
353+
}
354+
319355
finish();
320356
}
321357

358+
/**
359+
* Fetched coordinates are replaced with existing coordinates by a POST API call.
360+
* @param Latitude to be added
361+
* @param Longitude to be added
362+
* @param Accuracy to be added
363+
*/
364+
public void updateCoordinates(final String Latitude, final String Longitude,
365+
final String Accuracy) {
366+
if (media == null) {
367+
return;
368+
}
369+
compositeDisposable.add(coordinateEditHelper.makeCoordinatesEdit(getApplicationContext(),media,
370+
Latitude, Longitude, Accuracy)
371+
.subscribeOn(Schedulers.io())
372+
.observeOn(AndroidSchedulers.mainThread())
373+
.subscribe(s -> {
374+
Timber.d("Coordinates are added.");
375+
}));
376+
}
377+
322378
/**
323379
* Center the camera on the last saved ___location
324380
*/
@@ -458,4 +514,22 @@ private void addLocationMarker(GeoPoint geoPoint) {
458514
mapView.getOverlays().add(startMarker);
459515
}
460516

517+
/**
518+
* Saves the state of the activity
519+
* @param outState Bundle
520+
*/
521+
@Override
522+
public void onSaveInstanceState(@NonNull final Bundle outState) {
523+
super.onSaveInstanceState(outState);
524+
if(cameraPosition!=null){
525+
outState.putParcelable(CAMERA_POS, cameraPosition);
526+
}
527+
if(activity!=null){
528+
outState.putString(ACTIVITY, activity);
529+
}
530+
531+
if(media!=null){
532+
outState.putParcelable("sMedia", media);
533+
}
534+
}
461535
}

app/src/main/java/fr/free/nrw/commons/LocationPicker/LocationPickerConstants.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ public final class LocationPickerConstants {
1111
public static final String MAP_CAMERA_POSITION
1212
= "___location.picker.cameraPosition";
1313

14+
public static final String MEDIA
15+
= "___location.picker.media";
16+
1417

1518
private LocationPickerConstants() {
1619
}

app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java

Lines changed: 6 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -938,12 +938,14 @@ private void goToLocationPickerActivity() {
938938
}
939939
}
940940

941-
startActivityForResult(new LocationPicker.IntentBuilder()
941+
942+
startActivity(new LocationPicker.IntentBuilder()
942943
.defaultLocation(new CameraPosition.Builder()
943944
.target(new LatLng(defaultLatitude, defaultLongitude))
944945
.zoom(16).build())
945946
.activityKey("MediaActivity")
946-
.build(getActivity()), REQUEST_CODE);
947+
.media(media)
948+
.build(getActivity()));
947949
}
948950

949951
@OnClick(R.id.description_edit)
@@ -1121,32 +1123,7 @@ public void onActivityResult(final int requestCode, final int resultCode,
11211123
@Nullable final Intent data) {
11221124
super.onActivityResult(requestCode, resultCode, data);
11231125

1124-
if (requestCode == REQUEST_CODE && resultCode == RESULT_OK) {
1125-
1126-
assert data != null;
1127-
final CameraPosition cameraPosition = LocationPicker.getCameraPosition(data);
1128-
1129-
if (cameraPosition != null) {
1130-
1131-
final String latitude = String.valueOf(cameraPosition.target.getLatitude());
1132-
final String longitude = String.valueOf(cameraPosition.target.getLongitude());
1133-
final String accuracy = String.valueOf(cameraPosition.target.getAltitude());
1134-
String currentLatitude = null;
1135-
String currentLongitude = null;
1136-
1137-
if (media.getCoordinates() != null) {
1138-
currentLatitude = String.valueOf(media.getCoordinates().getLatitude());
1139-
currentLongitude = String.valueOf(media.getCoordinates().getLongitude());
1140-
}
1141-
1142-
if (!latitude.equals(currentLatitude) || !longitude.equals(currentLongitude)) {
1143-
updateCoordinates(latitude, longitude, accuracy);
1144-
} else if (media.getCoordinates() == null) {
1145-
updateCoordinates(latitude, longitude, accuracy);
1146-
}
1147-
}
1148-
1149-
} else if (requestCode == REQUEST_CODE_EDIT_DESCRIPTION && resultCode == RESULT_OK) {
1126+
if (requestCode == REQUEST_CODE_EDIT_DESCRIPTION && resultCode == RESULT_OK) {
11501127
final String updatedWikiText = data.getStringExtra(UPDATED_WIKITEXT);
11511128
compositeDisposable.add(descriptionEditHelper.addDescription(getContext(), media,
11521129
updatedWikiText)
@@ -1174,11 +1151,6 @@ public void onActivityResult(final int requestCode, final int resultCode,
11741151
progressBarEditDescription.setVisibility(GONE);
11751152
editDescription.setVisibility(VISIBLE);
11761153

1177-
} else if (requestCode == REQUEST_CODE && resultCode == RESULT_CANCELED) {
1178-
viewUtil.showShortToast(getContext(),
1179-
requireContext()
1180-
.getString(R.string.coordinates_picking_unsuccessful));
1181-
11821154
} else if (requestCode == REQUEST_CODE_EDIT_DESCRIPTION && resultCode == RESULT_CANCELED) {
11831155
progressBarEditDescription.setVisibility(GONE);
11841156
editDescription.setVisibility(VISIBLE);
@@ -1196,24 +1168,6 @@ private void updateCaptions(UploadMediaDetail mediaDetail,
11961168
media.setCaptions(updatedCaptions);
11971169
}
11981170

1199-
/**
1200-
* Fetched coordinates are replaced with existing coordinates by a POST API call.
1201-
* @param Latitude to be added
1202-
* @param Longitude to be added
1203-
* @param Accuracy to be added
1204-
*/
1205-
public void updateCoordinates(final String Latitude, final String Longitude,
1206-
final String Accuracy) {
1207-
compositeDisposable.add(coordinateEditHelper.makeCoordinatesEdit(getContext(), media,
1208-
Latitude, Longitude, Accuracy)
1209-
.subscribeOn(Schedulers.io())
1210-
.observeOn(AndroidSchedulers.mainThread())
1211-
.subscribe(s -> {
1212-
Timber.d("Coordinates are added.");
1213-
coordinates.setText(prettyCoordinates(media));
1214-
}));
1215-
}
1216-
12171171
@SuppressLint("StringFormatInvalid")
12181172
@OnClick(R.id.nominateDeletion)
12191173
public void onDeleteButtonClicked(){
@@ -1598,4 +1552,5 @@ private int getImageBackgroundColor() {
15981552
SharedPreferences imageBackgroundColorPref = this.getImageBackgroundColorPref();
15991553
return imageBackgroundColorPref.getInt(IMAGE_BACKGROUND_COLOR, DEFAULT_IMAGE_BACKGROUND_COLOR);
16001554
}
1555+
16011556
}

app/src/test/kotlin/fr/free/nrw/commons/locationpicker/LocationPickerActivityUnitTests.kt

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package fr.free.nrw.commons.locationpicker
22

33
import android.content.Context
44
import android.os.Looper
5+
import android.util.Log
56
import android.view.View
67
import android.widget.Button
78
import android.widget.ImageView
@@ -20,10 +21,14 @@ import com.nhaarman.mockitokotlin2.times
2021
import com.nhaarman.mockitokotlin2.verify
2122
import com.nhaarman.mockitokotlin2.whenever
2223
import fr.free.nrw.commons.LocationPicker.LocationPickerActivity
24+
import fr.free.nrw.commons.Media
2325
import fr.free.nrw.commons.TestCommonsApplication
26+
import fr.free.nrw.commons.coordinates.CoordinateEditHelper
2427
import fr.free.nrw.commons.kvstore.JsonKvStore
2528
import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.LAST_LOCATION
2629
import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.LAST_ZOOM
30+
import io.reactivex.android.plugins.RxAndroidPlugins
31+
import io.reactivex.schedulers.Schedulers
2732
import org.junit.Assert
2833
import org.junit.Assert.*
2934
import org.junit.Before
@@ -93,6 +98,7 @@ class LocationPickerActivityUnitTests {
9398
MockitoAnnotations.initMocks(this)
9499
context = RuntimeEnvironment.getApplication().applicationContext
95100
activity = Robolectric.buildActivity(LocationPickerActivity::class.java).get()
101+
RxAndroidPlugins.setInitMainThreadSchedulerHandler { Schedulers.trampoline() }
96102

97103
Whitebox.setInternalState(activity, "mapView", mapView)
98104
Whitebox.setInternalState(activity, "applicationKvStore", applicationKvStore)
@@ -165,4 +171,5 @@ class LocationPickerActivityUnitTests {
165171
}
166172

167173

168-
}
174+
175+
}

0 commit comments

Comments
 (0)