Fixes commons-app#4281 "Wrong language pre-selected in Nearby upload" (commons-app#4285)

* location added

* tests

* changes requested

* comments

* Test fixed, minor improvement

Author: 4D17Y4

app/src/main/java/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationsDao.java

@@ -157,6 +157,7 @@ Place fromCursor(Cursor cursor) {
         builder.setCommonsLink(cursor.getString(cursor.getColumnIndex(Table.COLUMN_COMMONS_LINK)));
 
         return new Place(
+                cursor.getString(cursor.getColumnIndex(Table.COLUMN_LANGUAGE)),
                 cursor.getString(cursor.getColumnIndex(Table.COLUMN_NAME)),
                 Label.fromText((cursor.getString(cursor.getColumnIndex(Table.COLUMN_LABEL_TEXT)))),
                 cursor.getString(cursor.getColumnIndex(Table.COLUMN_DESCRIPTION)),
@@ -171,6 +172,7 @@ Place fromCursor(Cursor cursor) {
     private ContentValues toContentValues(Place bookmarkLocation) {
         ContentValues cv = new ContentValues();
         cv.put(BookmarkLocationsDao.Table.COLUMN_NAME, bookmarkLocation.getName());
+        cv.put(BookmarkLocationsDao.Table.COLUMN_LANGUAGE, bookmarkLocation.getLanguage());
         cv.put(BookmarkLocationsDao.Table.COLUMN_DESCRIPTION, bookmarkLocation.getLongDescription());
         cv.put(BookmarkLocationsDao.Table.COLUMN_CATEGORY, bookmarkLocation.getCategory());
         cv.put(BookmarkLocationsDao.Table.COLUMN_LABEL_TEXT, bookmarkLocation.getLabel().getText());
@@ -189,6 +191,7 @@ public static class Table {
         public static final String TABLE_NAME = "bookmarksLocations";
 
         static final String COLUMN_NAME = "location_name";
+        static final String COLUMN_LANGUAGE = "location_lang";
         static final String COLUMN_DESCRIPTION = "location_description";
         static final String COLUMN_LAT = "location_lat";
         static final String COLUMN_LONG = "location_long";
@@ -205,6 +208,7 @@ public static class Table {
         // NOTE! KEEP IN SAME ORDER AS THEY ARE DEFINED UP THERE. HELPS HARD CODE COLUMN INDICES.
         public static final String[] ALL_FIELDS = {
                 COLUMN_NAME,
+                COLUMN_LANGUAGE,
                 COLUMN_DESCRIPTION,
                 COLUMN_CATEGORY,
                 COLUMN_LABEL_TEXT,
@@ -223,6 +227,7 @@ public static class Table {
 
         static final String CREATE_TABLE_STATEMENT = "CREATE TABLE " + TABLE_NAME + " ("
                 + COLUMN_NAME + " STRING PRIMARY KEY,"
+                + COLUMN_LANGUAGE + " STRING,"
                 + COLUMN_DESCRIPTION + " STRING,"
                 + COLUMN_CATEGORY + " STRING,"
                 + COLUMN_LABEL_TEXT + " STRING,"
@@ -287,6 +292,13 @@ public static void onUpdate(SQLiteDatabase db, int from, int to) {
                     Timber.e(exception);
                 }
             }
+            if (from == 13){
+                try {
+                    db.execSQL("ALTER TABLE bookmarksLocations ADD COLUMN location_lang STRING;");
+                } catch (SQLiteException exception){
+                    Timber.e(exception);
+                }
+            }
         }
     }
 }

app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.java

@@ -13,7 +13,7 @@
 public class DBOpenHelper  extends SQLiteOpenHelper {
 
     private static final String DATABASE_NAME = "commons.db";
-    private static final int DATABASE_VERSION = 13;
+    private static final int DATABASE_VERSION = 14;
     public static final String CONTRIBUTIONS_TABLE = "contributions";
     private final String DROP_TABLE_STATEMENT="DROP TABLE IF EXISTS %s";
 

app/src/main/java/fr/free/nrw/commons/nearby/Place.java

@@ -18,6 +18,7 @@
  */
 public class Place implements Parcelable {
 
+    public final String language;
     public final String name;
     private final Label label;
     private final String longDescription;
@@ -30,7 +31,8 @@ public class Place implements Parcelable {
     public final Sitelinks siteLinks;
 
 
-    public Place(String name, Label label, String longDescription, LatLng location, String category, Sitelinks siteLinks, String pic, String destroyed) {
+    public Place(String language,String name, Label label, String longDescription, LatLng location, String category, Sitelinks siteLinks, String pic, String destroyed) {
+        this.language = language;
         this.name = name;
         this.label = label;
         this.longDescription = longDescription;
@@ -40,8 +42,8 @@ public Place(String name, Label label, String longDescription, LatLng location,
         this.pic = (pic == null) ? "":pic;
         this.destroyed = (destroyed == null) ? "":destroyed;
     }
-
     public Place(Parcel in) {
+        this.language = in.readString();
         this.name = in.readString();
         this.label = (Label) in.readSerializable();
         this.longDescription = in.readString();
@@ -53,14 +55,14 @@ public Place(Parcel in) {
         String destroyedString = in.readString();
         this.destroyed = (destroyedString == null) ? "":destroyedString;
     }
-
     public static Place from(NearbyResultItem item) {
         String itemClass = item.getClassName().getValue();
         String classEntityId = "";
         if(!StringUtils.isBlank(itemClass)) {
             classEntityId = itemClass.replace("http://www.wikidata.org/entity/", "");
         }
         return new Place(
+                item.getLabel().getLanguage(),
                 item.getLabel().getValue(),
                 Label.fromText(classEntityId), // list
                 item.getClassLabel().getValue(), // details
@@ -75,6 +77,14 @@ public static Place from(NearbyResultItem item) {
                 item.getDestroyed().getValue());
     }
 
+    /**
+     * Gets the language of the caption ie name.
+     * @return language
+     */
+    public String getLanguage() {
+        return language;
+    }
+
     /**
      * Gets the name of the place
      * @return name
@@ -176,6 +186,7 @@ public int hashCode() {
     public String toString() {
         return "Place{" +
                 "name='" + name + '\'' +
+                ", lang='" + language + '\'' +
                 ", label='" + label + '\'' +
                 ", longDescription='" + longDescription + '\'' +
                 ", location='" + location + '\'' +
@@ -194,6 +205,7 @@ public int describeContents() {
 
     @Override
     public void writeToParcel(Parcel dest, int flags) {
+        dest.writeString(language);
         dest.writeString(name);
         dest.writeSerializable(label);
         dest.writeString(longDescription);

app/src/main/java/fr/free/nrw/commons/nearby/model/ResultTuple.kt

@@ -1,15 +1,21 @@
 package fr.free.nrw.commons.nearby.model
 
+import com.google.gson.annotations.SerializedName
+
 class ResultTuple {
+    @SerializedName("xml:lang")
+    val language: String
     val type: String
     val value: String
 
-    constructor(type: String, value: String) {
+    constructor(lang: String, type: String, value: String) {
+        this.language = lang
         this.type = type
         this.value = value
     }
 
     constructor() {
+        language = ""
         type = ""
         value = ""
     }

app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetail.kt

@@ -1,7 +1,6 @@
 package fr.free.nrw.commons.upload
 
 import fr.free.nrw.commons.nearby.Place
-import java.util.*
 
 /**
  * Holds a description of an item being uploaded by [UploadActivity]
@@ -20,7 +19,7 @@ data class UploadMediaDetail constructor(
     fun javaCopy() = copy()
 
     constructor(place: Place) : this(
-        Locale.getDefault().language,
+        place.language,
         place.longDescription,
         place.name
     )

app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetailAdapter.java

@@ -174,31 +174,46 @@ public void onNothingSelected(AdapterView<?> adapterView) {
                 }
             });
 
-            if (description.getSelectedLanguageIndex() == -1) {
-                if (!TextUtils.isEmpty(savedLanguageValue)) {
-                    // If user has chosen a default language from settings activity savedLanguageValue is not null
-                    spinnerDescriptionLanguages.setSelection(languagesAdapter.getIndexOfLanguageCode(savedLanguageValue));
-                } else {
-                    //Checking whether Language Code attribute is null or not.
-                    if(uploadMediaDetails.get(position).getLanguageCode() != null){
-                        //If it is not null that means it is fetching details from the previous upload (i.e. when user has pressed copy previous caption & description)
-                        //hence providing same language code for the current upload.
-                        spinnerDescriptionLanguages.setSelection(languagesAdapter
-                            .getIndexOfLanguageCode(uploadMediaDetails.get(position).getLanguageCode()), true);
+
+            if(description.getCaptionText().isEmpty() == false
+                && languagesAdapter.getIndexOfLanguageCode(description.getLanguageCode()) != -1){
+                // If the user selects a nearby pin or location bookmark to upload a picture and language is present in spinner we set the language.
+                spinnerDescriptionLanguages.setSelection(languagesAdapter.getIndexOfLanguageCode(description.getLanguageCode()));
+            }
+            else {
+                // This is a contribution upload or the language from description is not present in spinner.
+                if (description.getSelectedLanguageIndex() == -1) {
+                    if (!TextUtils.isEmpty(savedLanguageValue)) {
+                        // If user has chosen a default language from settings activity savedLanguageValue is not null
+                        spinnerDescriptionLanguages
+                            .setSelection(
+                                languagesAdapter.getIndexOfLanguageCode(savedLanguageValue));
                     } else {
-                        if (position == 0) {
-                            int defaultLocaleIndex = languagesAdapter
-                                .getIndexOfUserDefaultLocale(spinnerDescriptionLanguages.getContext());
-                            spinnerDescriptionLanguages.setSelection(defaultLocaleIndex, true);
+                        //Checking whether Language Code attribute is null or not.
+                        if (uploadMediaDetails.get(position).getLanguageCode() != null) {
+                            //If it is not null that means it is fetching details from the previous upload (i.e. when user has pressed copy previous caption & description)
+                            //hence providing same language code for the current upload.
+                            spinnerDescriptionLanguages.setSelection(languagesAdapter
+                                .getIndexOfLanguageCode(
+                                    uploadMediaDetails.get(position).getLanguageCode()), true);
                         } else {
-                            spinnerDescriptionLanguages.setSelection(0,true);
+                            if (position == 0) {
+                                int defaultLocaleIndex = languagesAdapter
+                                    .getIndexOfUserDefaultLocale(
+                                        spinnerDescriptionLanguages.getContext());
+                                spinnerDescriptionLanguages.setSelection(defaultLocaleIndex, true);
+                            } else {
+                                spinnerDescriptionLanguages.setSelection(0, true);
+                            }
                         }
                     }
-                }
 
-            } else {
-                spinnerDescriptionLanguages.setSelection(description.getSelectedLanguageIndex());
-                selectedLanguages.put(spinnerDescriptionLanguages, description.getLanguageCode());
+                } else {
+                    spinnerDescriptionLanguages
+                        .setSelection(description.getSelectedLanguageIndex());
+                    selectedLanguages
+                        .put(spinnerDescriptionLanguages, description.getLanguageCode());
+                }
             }
         }
     }

app/src/test/kotlin/ModelFunctions.kt

@@ -76,6 +76,7 @@ fun depictSearchItem(
 
 fun place(
     name: String = "name",
+    lang: String = "en",
     label: Label? = null,
     longDescription: String = "longDescription",
     latLng: LatLng? = null,
@@ -84,7 +85,7 @@ fun place(
     pic: String = "pic",
     destroyed: String = "destroyed"
 ): Place {
-    return Place(name, label, longDescription, latLng, category, siteLinks, pic, destroyed)
+    return Place(lang, name, label, longDescription, latLng, category, siteLinks, pic, destroyed)
 }
 
 fun entityId(wikiBaseEntityValue: WikiBaseEntityValue = wikiBaseEntityValue()) =

app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookMarkLocationDaoTest.kt

@@ -26,6 +26,7 @@ import org.robolectric.annotation.Config
 @Config(sdk = [21], application = TestCommonsApplication::class)
 class BookMarkLocationDaoTest {
     private val columns = arrayOf(COLUMN_NAME,
+            COLUMN_LANGUAGE,
             COLUMN_DESCRIPTION,
             COLUMN_CATEGORY,
             COLUMN_LABEL_TEXT,
@@ -61,7 +62,7 @@ class BookMarkLocationDaoTest {
         builder.setCommonsLink("commonsLink")
 
 
-        examplePlaceBookmark = Place("placeName", exampleLabel, "placeDescription"
+        examplePlaceBookmark = Place("en", "placeName", exampleLabel, "placeDescription"
                 , exampleLocation, "placeCategory", builder.build(),"picName","placeDestroyed")
         testObject = BookmarkLocationsDao { client }
     }
@@ -86,6 +87,7 @@ class BookMarkLocationDaoTest {
         createCursor(1).let { cursor ->
             cursor.moveToFirst()
             testObject.fromCursor(cursor).let {
+                assertEquals("en", it.language)
                 assertEquals("placeName", it.name)
                 assertEquals(Label.FOREST, it.label)
                 assertEquals("placeDescription", it.longDescription)
@@ -149,8 +151,9 @@ class BookMarkLocationDaoTest {
         assertTrue(testObject.updateBookmarkLocation(examplePlaceBookmark))
         verify(client).insert(eq(BASE_URI), captor.capture())
         captor.firstValue.let { cv ->
-            assertEquals(12, cv.size())
+            assertEquals(13, cv.size())
             assertEquals(examplePlaceBookmark.name, cv.getAsString(COLUMN_NAME))
+            assertEquals(examplePlaceBookmark.language, cv.getAsString(COLUMN_LANGUAGE))
             assertEquals(examplePlaceBookmark.longDescription, cv.getAsString(COLUMN_DESCRIPTION))
             assertEquals(examplePlaceBookmark.label.text, cv.getAsString(COLUMN_LABEL_TEXT))
             assertEquals(examplePlaceBookmark.category, cv.getAsString(COLUMN_CATEGORY))
@@ -262,10 +265,17 @@ class BookMarkLocationDaoTest {
         verify(database).execSQL("ALTER TABLE bookmarksLocations ADD COLUMN location_destroyed STRING;")
     }
 
+    @Test
+    fun migrateTableVersionFrom_v13_to_v14() {
+        onUpdate(database, 13, 14)
+        verify(database).execSQL("ALTER TABLE bookmarksLocations ADD COLUMN location_lang STRING;")
+    }
+
+
     private fun createCursor(rowCount: Int) = MatrixCursor(columns, rowCount).apply {
 
         for (i in 0 until rowCount) {
-            addRow(listOf("placeName", "placeDescription","placeCategory", exampleLabel.text, exampleLabel.icon,
+            addRow(listOf("placeName", "en", "placeDescription", "placeCategory", exampleLabel.text, exampleLabel.icon,
                     exampleUri, builder.build().wikipediaLink, builder.build().wikidataLink, builder.build().commonsLink,
                     exampleLocation.latitude, exampleLocation.longitude, "picName", "placeDestroyed"))
         }

app/src/test/kotlin/fr/free/nrw/commons/bookmarks/locations/BookmarkLocationControllerTest.kt

@@ -33,8 +33,8 @@ class BookmarkLocationControllerTest {
     private val mockBookmarkList: List<Place>
         private get() {
             val list = ArrayList<Place>()
-            list.add(Place("a place",null,"a description",null,"a cat",null,null,null))
-            list.add(Place("another place",null,"another description",null,"another cat",null,null,null))
+            list.add(Place("en","a place",null,"a description",null,"a cat",null,null,null))
+            list.add(Place("en","another place",null,"another description",null,"another cat",null,null,null))
             return list
         }