Code cleanups

app/src/main/java/fr/free/nrw/commons/nearby/mvp/contract/NearbyParentFragmentContract.java

@@ -27,7 +27,6 @@ public interface NearbyParentFragmentContract {
         void setFABRecenterAction(android.view.View.OnClickListener onClickListener);
         void animateFABs();
         void recenterMap(LatLng curLatLng);
-        void initViewPositions();
         void hideBottomSheet();
         void displayBottomSheetWithInfo(Marker marker);
         void addOnCameraMoveListener(MapboxMap.OnCameraMoveListener onCameraMoveListener);
@@ -45,7 +44,7 @@ public interface NearbyParentFragmentContract {
 
     interface UserActions {
         void onTabSelected();
-        void initializeNearbyOperations();
+        void checkForPermission();
         void updateMapAndList(LocationServiceManager.LocationChangeType locationChangeType, LatLng cameraTarget);
         void lockUnlockNearby(boolean isNearbyLocked);
         void setActionListeners(JsonKvStore applicationKvStore);

app/src/main/java/fr/free/nrw/commons/nearby/mvp/fragments/NearbyMapFragment.java

@@ -339,7 +339,6 @@ public class NearbyMapFragment extends CommonsDaggerSupportFragment
     public void addNearbyMarkersToMapBoxMap(@Nullable List<NearbyBaseMarker> baseMarkerList
                                                         , Marker selectedMarker
                                                         , NearbyParentFragmentPresenter nearbyParentFragmentPresenter) {
-        Log.d("denemeTest","add markers to map");
         mapboxMap.addMarkers(baseMarkerList);
         map.getMapAsync(mapboxMap -> {
             mapboxMap.addMarkers(baseMarkerList);

app/src/main/java/fr/free/nrw/commons/nearby/mvp/fragments/NearbyParentFragment.java

@@ -44,8 +44,10 @@ import javax.inject.Named;
 
 import butterknife.BindView;
 import butterknife.ButterKnife;
+import fr.free.nrw.commons.CommonsApplication;
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.Utils;
+import fr.free.nrw.commons.auth.LoginActivity;
 import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao;
 import fr.free.nrw.commons.contributions.ContributionController;
 import fr.free.nrw.commons.contributions.MainActivity;
@@ -71,6 +73,7 @@ import timber.log.Timber;
 import static fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType.LOCATION_SIGNIFICANTLY_CHANGED;
 import static fr.free.nrw.commons.contributions.MainActivity.CONTRIBUTIONS_TAB_POSITION;
 import static fr.free.nrw.commons.location.LocationServiceManager.LocationChangeType.MAP_UPDATED;
+import static fr.free.nrw.commons.wikidata.WikidataConstants.PLACE_OBJECT;
 
 
 public class NearbyParentFragment extends CommonsDaggerSupportFragment
@@ -436,11 +439,11 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
 
     @Override
     public void populatePlaces(fr.free.nrw.commons.location.LatLng curlatLng, fr.free.nrw.commons.location.LatLng searchLatLng) {
-        boolean checkingAroundCurretLocation;
+        boolean checkingAroundCurrentLocation;
         if (curlatLng.equals(searchLatLng)) { // Means we are checking around current location
-            checkingAroundCurretLocation = true;
+            checkingAroundCurrentLocation = true;
             compositeDisposable.add(Observable.fromCallable(() -> nearbyController
-                    .loadAttractionsFromLocation(curlatLng, searchLatLng, false, checkingAroundCurretLocation))
+                    .loadAttractionsFromLocation(curlatLng, searchLatLng, false, checkingAroundCurrentLocation))
                     .subscribeOn(Schedulers.io())
                     .observeOn(AndroidSchedulers.mainThread())
                     .subscribe(this::updateMapMarkers,
@@ -451,9 +454,9 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
                                 nearbyParentFragmentPresenter.lockUnlockNearby(false);
                             }));
         } else {
-            checkingAroundCurretLocation = false;
+            checkingAroundCurrentLocation = false;
             compositeDisposable.add(Observable.fromCallable(() -> nearbyController
-                    .loadAttractionsFromLocation(curlatLng, searchLatLng, false, checkingAroundCurretLocation))
+                    .loadAttractionsFromLocation(curlatLng, searchLatLng, false, checkingAroundCurrentLocation))
                     .subscribeOn(Schedulers.io())
                     .observeOn(AndroidSchedulers.mainThread())
                     .subscribe(this::updateMapMarkersForCustomLocation,
@@ -615,16 +618,29 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
                     .setMessage(R.string.login_alert_message)
                     .setPositiveButton(R.string.login, (dialog, which) -> {
                         // logout of the app
-                        //TODO:
+                        BaseLogoutListener logoutListener = new BaseLogoutListener();
-                        // ((NavigationBaseActivity)getActivity()).BaseLogoutListener logoutListener = new ((NavigationBaseActivity)getActivity()).BaseLogoutListener();
+                        CommonsApplication app = (CommonsApplication) getActivity().getApplication();
-                        // CommonsApplication app = (CommonsApplication) getActivity().getApplication();
+                        app.clearApplicationData(getContext(), logoutListener);
-                        // app.clearApplicationData(getContext(), logoutListener);
-
                     })
                     .show();
         }
     }
 
+    /**
+     * onLogoutComplete is called after shared preferences and data stored in local database are cleared.
+     */
+    private class BaseLogoutListener implements CommonsApplication.LogoutListener {
+        @Override
+        public void onLogoutComplete() {
+            Timber.d("Logout complete callback received.");
+            Intent nearbyIntent = new Intent( getActivity(), LoginActivity.class);
+            nearbyIntent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK);
+            nearbyIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+            startActivity(nearbyIntent);
+            getActivity().finish();
+        }
+    }
+
     @Override
     public void setFABPlusAction(View.OnClickListener onClickListener) {
         fabPlus.setOnClickListener(onClickListener);
@@ -664,11 +680,6 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
         nearbyMapFragment.getMapboxMap().animateCamera(CameraUpdateFactory.newCameraPosition(position), 1000);
     }
 
-    @Override
-    public void initViewPositions() {
-
-    }
-
     @Override
     public void hideBottomSheet() {
         bottomSheetListBehavior.setState(BottomSheetBehavior.STATE_HIDDEN);
@@ -688,7 +699,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
      * If nearby details bottom sheet state is collapsed: show fab plus
      * If nearby details bottom sheet state is expanded: show fab plus
      * If nearby details bottom sheet state is hidden: hide all fabs
-     * @param bottomSheetState
+     * @param bottomSheetState see bottom sheet states
      */
     public void prepareViewsForSheetPosition(int bottomSheetState) {
 
@@ -729,8 +740,6 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
             nearbyMapFragment.updateMarker(isBookmarked, this.selectedPlace, locationManager.getLastLocation());
         });
 
-
-        //TODO move all this buttons into a custom bottom sheet
         wikipediaButton.setVisibility(place.hasWikipediaLink()?View.VISIBLE:View.GONE);
         wikipediaButton.setOnClickListener(view -> Utils.handleWebUrl(getContext(), this.selectedPlace.siteLinks.getWikipediaLink()));
 
@@ -751,7 +760,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
         fabCamera.setOnClickListener(view -> {
             if (fabCamera.isShown()) {
                 Timber.d("Camera button tapped. Place: %s", this.selectedPlace.toString());
-                // TODO storeSharedPrefs();
+                storeSharedPrefs(selectedPlace);
                 controller.initiateCameraPick(getActivity());
             }
         });
@@ -759,12 +768,17 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
         fabGallery.setOnClickListener(view -> {
             if (fabGallery.isShown()) {
                 Timber.d("Gallery button tapped. Place: %s", this.selectedPlace.toString());
-                //TODO storeSharedPrefs();
+                storeSharedPrefs(selectedPlace);
                 controller.initiateGalleryPick(getActivity(), false);
             }
         });
     }
 
+    private void storeSharedPrefs(Place selectedPlace) {
+        Timber.d("Store place object %s", selectedPlace.toString());
+        applicationKvStore.putJson(PLACE_OBJECT, selectedPlace);
+    }
+
     private void updateBookmarkButtonImage(Place place) {
         int bookmarkIcon;
         if (bookmarkLocationDao.findBookmarkLocation(place)) {
@@ -820,6 +834,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
     }
 
     private void showErrorMessage(String message) {
+        // TODO
         ViewUtil.showLongToast(getActivity(), message);
     }
 

app/src/main/java/fr/free/nrw/commons/nearby/mvp/presenter/NearbyParentFragmentPresenter.java

@@ -37,17 +37,13 @@ public class NearbyParentFragmentPresenter
     private boolean isNearbyLocked;
     private LatLng curLatLng;
 
-    boolean nearbyViewsAreReady;
+    private boolean nearbyViewsAreReady;
-    boolean onTabSelected;
+    private boolean onTabSelected;
-    boolean searchingThisArea;
+    private boolean mapInitialized;
-    boolean nearbyMapViewReady;
-    boolean nearbyOperationsInitialized;
-    boolean mapInitialized; // TODO reset this on fragment destroyed
 
-    Place placeToCenter;
+    private Place placeToCenter;
-    boolean isPortraitMode;
+    private boolean isPortraitMode;
-    boolean willMapBeCentered;
+    private boolean placesLoadedOnce;
-    boolean placesLoadedOnce;
 
 
     private LocationServiceManager locationServiceManager;
@@ -70,11 +66,10 @@ public class NearbyParentFragmentPresenter
     @Override
     public void onTabSelected() {
         Timber.d("Nearby tab selected");
-        Log.d("denemeTest","Nearby tab selected");
         onTabSelected = true;
         // The condition for initialize operations is both having views ready and tab is selected
         if (nearbyViewsAreReady && !mapInitialized) {
-            initializeNearbyOperations();
+            checkForPermission();
         }
     }
 
@@ -85,64 +80,40 @@ public class NearbyParentFragmentPresenter
     @Override
     public void nearbyFragmentsAreReady() {
         Timber.d("Nearby fragments are ready to be used by presenter");
-        Log.d("denemeTest","nearbyFragmentsAreReady");
         nearbyViewsAreReady = true;
         // The condition for initialize operations is both having views ready and tab is selected
         if (onTabSelected) {
-            initializeNearbyOperations();
+            checkForPermission();
         }
     }
 
     /**
      * Initializes nearby operations by following these steps:
-     * - Add this location listener to location manager
+     * -Checks for permission and perform if given
-     * - Registers location updates with parent fragment, this methods also checks permissions
-     * Note: Highly context dependent methods are handled in view and triggered from presenter
      */
     @Override
-    public void initializeNearbyOperations() {
+    public void checkForPermission() {
-        // TODO: do not update every time user selected tab
+        Timber.d("checking for permission");
-        Timber.d("initializing nearby operations started");
-        // Add location listener to be notified about location changes
-        //locationServiceManager.addLocationListener(this);
         nearbyParentFragmentView.checkPermissionsAndPerformAction(this::performNearbyOperationsIfPermissionGiven);
-
     }
 
+    /**
+     * - Adds search this area button action
+     * - Adds camera move action listener
+     * - Initializes nearby operations, registers listeners, broadcast receivers etc.
+     */
     public void performNearbyOperationsIfPermissionGiven() {
-        Timber.d("performNearbyOperationsIfPermissionGiven");
+        Timber.d("Permission is given, performing actions");
-        //nearbyParentFragmentView.registerLocationUpdates(locationServiceManager);
-        // Nearby buttons should be active, they should be inactive only during update
-        // This will start a consequence to check GPS depending on different API
-        //nearbyParentFragmentView.checkGps(locationServiceManager);
-        // We will know when we went offline and online again
-        //nearbyParentFragmentView.addNetworkBroadcastReceiver();
-        //nearbyMapFragmentView.setupMapView(null);
-        //nearbyOperationsInitialized();
         this.nearbyParentFragmentView.addSearchThisAreaButtonAction();
         this.nearbyParentFragmentView.addOnCameraMoveListener(onCameraMove(getMapboxMap()));
         initializeMapOperations();
     }
 
-    public void nearbyOperationsInitialized() {
-        Log.d("deneme2","nearbyOperationsinitialized");
-        nearbyOperationsInitialized = true;
-        if (nearbyMapViewReady) {
-            initializeMapOperations();
-        }
-    }
-
     public void initializeMapOperations() {
-        Log.d("denemeTest","initializeMapOperations");
-        nearbyParentFragmentView.initViewPositions();
-
         lockUnlockNearby(false);
         registerUnregisterLocationListener(false);
         nearbyParentFragmentView.addNetworkBroadcastReceiver();
-
-        Timber.d("Nearby map view is created and ready");
         updateMapAndList(LOCATION_SIGNIFICANTLY_CHANGED, null);
-        // TODO: document this prpoblem, if updateMapAndList is not called at checkGPS then this method never called, setup map view never ends
         this.nearbyParentFragmentView.addSearchThisAreaButtonAction();
         this.nearbyMapFragmentView.addOnCameraMoveListener(onCameraMove(getMapboxMap()));
         mapInitialized = true;
@@ -205,11 +176,11 @@ public class NearbyParentFragmentPresenter
         if (removeLocationListener) {
             locationServiceManager.unregisterLocationManager();
             locationServiceManager.removeLocationListener(this);
-            Timber.d("Nearby locked");
+            Timber.d("Location service manager unregistered and removed");
         } else {
             locationServiceManager.addLocationListener(this);
             nearbyParentFragmentView.registerLocationUpdates(locationServiceManager);
-            Timber.d("Nearby unlocked");
+            Timber.d("Location service manager added and registered");
         }
     }
 
@@ -229,29 +200,21 @@ public class NearbyParentFragmentPresenter
      */
     @Override
     public void updateMapAndList(LocationServiceManager.LocationChangeType locationChangeType, LatLng cameraTarget) {
-        Log.d("denemeTest","updateMapAndList");
+        Timber.d("Presenter updates map and list");
         if (isNearbyLocked) {
-            Log.d("denemeTest","isNearbyLocked");
             Timber.d("Nearby is locked, so updateMapAndList returns");
             return;
         }
 
         if (!nearbyParentFragmentView.isNetworkConnectionEstablished()) {
             Timber.d("Network connection is not established");
-            Log.d("denemeTest","!nearbyParentFragmentView.isNetworkConnectionEstablished()");
             return;
         }
 
         LatLng lastLocation = locationServiceManager.getLastLocation();
-
-        if (curLatLng != null) {
-            // TODO figure out what is happening here about orientation change
-        }
-
         curLatLng = lastLocation;
 
         if (curLatLng == null) {
-
             Timber.d("Skipping update of nearby places as location is unavailable");
             return;
         }
@@ -262,27 +225,21 @@ public class NearbyParentFragmentPresenter
          */
         if (locationChangeType.equals(LOCATION_SIGNIFICANTLY_CHANGED)
                 || locationChangeType.equals(MAP_UPDATED)) {
-            Log.d("denemeTest","1");
+            Timber.d("LOCATION_SIGNIFICANTLY_CHANGED");
             lockUnlockNearby(true);
             nearbyParentFragmentView.setProgressBarVisibility(true);
             nearbyParentFragmentView.populatePlaces(lastLocation, lastLocation);
-            //nearbyMapFragmentView.updateMapToTrackPosition(curLatLng);
-            // TODO: when unneeded populate places call problem is solved, open commented out line and remove it from update map markers method
-            // TODO dont forget map updated state after an wikidata item is updated
 
         } else if (locationChangeType.equals(SEARCH_CUSTOM_AREA)) {
-            Log.d("denemeTest","2");
+            Timber.d("SEARCH_CUSTOM_AREA");
             lockUnlockNearby(true);
             nearbyParentFragmentView.setProgressBarVisibility(true);
             nearbyParentFragmentView.populatePlaces(lastLocation, cameraTarget);
-            searchingThisArea = false;
-        } else { // Means location changed slightly, ie user is walking or driving.
-            Log.d("denemeTest","3");
-            nearbyMapFragmentView.updateMapToTrackPosition(curLatLng);
-            searchingThisArea = false;
-        }
 
-        // TODO: update camera angle accordingly here, 1- search this area mode, 2- following current location, 3- list sheet expanded, 4- landcaped
+        } else { // Means location changed slightly, ie user is walking or driving.
+            Timber.d("Means location changed slightly");
+            nearbyMapFragmentView.updateMapToTrackPosition(curLatLng);
+        }
     }
 
     /**
@@ -314,6 +271,10 @@ public class NearbyParentFragmentPresenter
         handleCenteringTaskIfAny();
     }
 
+    /**
+     * Some centering task may need to wait for map to be ready, if they are requested before
+     * map is ready. So we will remember it when the map is ready
+     */
     private void handleCenteringTaskIfAny() {
         if (!placesLoadedOnce) {
             placesLoadedOnce = true;
@@ -323,14 +284,13 @@ public class NearbyParentFragmentPresenter
 
     @Override
     public void onWikidataEditSuccessful() {
-
+        // TODO
     }
 
     @Override
     public void onLocationChangedSignificantly(LatLng latLng) {
         Timber.d("Location significantly changed");
         updateMapAndList(LOCATION_SIGNIFICANTLY_CHANGED, null);
-
     }
 
     @Override
@@ -346,23 +306,15 @@ public class NearbyParentFragmentPresenter
 
     @Override
     public MapboxMap.OnCameraMoveListener onCameraMove(MapboxMap mapboxMap) {
-        Log.d("denemeTestt","cameramoving1");
-
         return () -> {
-            Log.d("denemeTestt","cameramoving2");
-
             // If our nearby markers are calculated at least once
             if (NearbyController.currentLocation != null) {
-                Log.d("denemeTestt","NearbyController.currentLocation != null");
                double distance = mapboxMap.getCameraPosition().target.distanceTo
-                        //TODO: test this distances
                         (LocationUtils.commonsLatLngToMapBoxLatLng(NearbyController.latestSearchLocation));
                 if (nearbyParentFragmentView.isNetworkConnectionEstablished()) {
                     if (distance > NearbyController.latestSearchRadius) {
-                        Log.d("denemeTestt","distance > NearbyController.latestSearchRadius");
                         nearbyParentFragmentView.setSearchThisAreaButtonVisibility(true);
                     } else {
-                        Log.d("denemeTestt","distance < NearbyController.latestSearchRadius");
                         nearbyParentFragmentView.setSearchThisAreaButtonVisibility(false);
                     }
                 }
@@ -373,22 +325,16 @@ public class NearbyParentFragmentPresenter
     }
 
     public View.OnClickListener onSearchThisAreaClicked() {
-        return new View.OnClickListener() {
+        return v -> {
-            @Override
+            // Lock map operations during search this area operation
-            public void onClick(View v) {
+            nearbyParentFragmentView.setSearchThisAreaButtonVisibility(false);
-                Log.d("denemeTestt","onSearchThisAreaClicked");
-                // Lock map operations during search this area operation
-                nearbyParentFragmentView.setSearchThisAreaButtonVisibility(false);
 
-                if (searchCloseToCurrentLocation()){
+            if (searchCloseToCurrentLocation()){
-                    Log.d("denemeTestt","searchCloseToCurrentLocation()");
+                updateMapAndList(LOCATION_SIGNIFICANTLY_CHANGED,
-                    updateMapAndList(LOCATION_SIGNIFICANTLY_CHANGED,
+                        null);
-                            null);
+            } else {
-                } else {
+                updateMapAndList(SEARCH_CUSTOM_AREA,
-                    Log.d("denemeTestt","!searchCloseToCurrentLocation()+"+getCameraTarget());
+                        getCameraTarget());
-                    updateMapAndList(SEARCH_CUSTOM_AREA,
-                            getCameraTarget());
-                }
             }
         };
     }
@@ -399,7 +345,6 @@ public class NearbyParentFragmentPresenter
      * @return Returns true if search this area button is used around our current location
      */
     public boolean searchCloseToCurrentLocation() {
-        Log.d("denemeTestt","searchCloseToCurrentLocation method");
         double distance = LocationUtils.commonsLatLngToMapBoxLatLng(getCameraTarget())
                 .distanceTo(new com.mapbox.mapboxsdk.geometry.LatLng(NearbyController.currentLocation.getLatitude()
                         , NearbyController.currentLocation.getLongitude()));
@@ -418,7 +363,6 @@ public class NearbyParentFragmentPresenter
         if (placesLoadedOnce) {
             nearbyMapFragmentView.centerMapToPlace(place, isPortraitMode);
         } else {
-            willMapBeCentered = true;
             this.isPortraitMode = isPortraitMode;
             this.placeToCenter = place;
         }