Fix p18 issue For an item with P18 item, do not add another one (#3527)

* Add p18value variable to contrib

* set place.pic to candidate contribution

* Add p18 value to contrib

* Passes p18 value to wikidata upload service

* Checks if pic parameter of the wikidata item is empty or not. If not, it does not overrides the existing image, it is just a regular commons upload.

* Make public var private

* Make current tests pass

* Add test case for p18 value is not empty

* Fix wrong log message

* Add nonnul annotation and fix method javadoc

Author: neslihanturan

app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java

@@ -68,6 +68,7 @@ public Contribution[] newArray(int i) {
     public String decimalCoords;
     public boolean isMultiple;
     public String wikiDataEntityId;
+    private String p18Value;
     public Uri contentProviderUri;
     public String dateCreatedSource;
 
@@ -271,6 +272,19 @@ public void setWikiDataEntityId(String wikiDataEntityId) {
         this.wikiDataEntityId = wikiDataEntityId;
     }
 
+    public String getP18Value() {
+        return p18Value;
+    }
+
+    /**
+     * When the corresponding image property of wiki entity is known as in case of nearby uploads,
+     * it can be set using the setter method
+     * @param p18Value p18 value, image property of the wikidata item
+     */
+    public void setP18Value(String p18Value) {
+        this.p18Value = p18Value;
+    }
+
     public void setContentProviderUri(Uri contentProviderUri) {
         this.contentProviderUri = contentProviderUri;
     }

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

@@ -197,6 +197,8 @@ public Observable<Contribution> buildContributions() {
                     CommonsApplication.DEFAULT_EDIT_SUMMARY, item.gpsCoords.getCoords());
             if (item.place != null) {
                 contribution.setWikiDataEntityId(item.place.getWikiDataEntityId());
+                // If item already has an image, we need to know it. We don't want to override existing image later
+                contribution.setP18Value(item.place.pic);
             }
             if (null == selectedCategories) {//Just a fail safe, this should never be null
                 selectedCategories = new ArrayList<>();

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

@@ -278,9 +278,10 @@ private void uploadContribution(Contribution contribution) {
                         showFailedNotification(contribution);
                     } else {
                         String canonicalFilename = "File:" + uploadResult.getFilename();
-                        Timber.d("Contribution upload success. Initiating Wikidata edit for entity id %s",
-                                contribution.getWikiDataEntityId());
-                        wikidataEditService.createClaimWithLogging(contribution.getWikiDataEntityId(), canonicalFilename);
+                        Timber.d("Contribution upload success. Initiating Wikidata edit for"
+                                + " entity id %s if necessary (if P18 is null). P18 value is %s",
+                                contribution.getWikiDataEntityId(), contribution.getP18Value());
+                        wikidataEditService.createClaimWithLogging(contribution.getWikiDataEntityId(), canonicalFilename, contribution.getP18Value());
                         contribution.setFilename(canonicalFilename);
                         contribution.setImageUrl(uploadResult.getImageinfo().getOriginalUrl());
                         contribution.setState(Contribution.STATE_COMPLETED);

app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java

@@ -3,6 +3,7 @@
 import android.annotation.SuppressLint;
 import android.content.Context;
 
+import androidx.annotation.NonNull;
 import java.util.Locale;
 
 import javax.inject.Inject;
@@ -45,10 +46,11 @@ public class WikidataEditService {
 
     /**
      * Create a P18 claim and log the edit with custom tag
-     * @param wikidataEntityId
-     * @param fileName
+     * @param wikidataEntityId a unique id of each Wikidata items
+     * @param fileName name of the file we will upload
+     * @param p18Value pic attribute of Wikidata item
      */
-    public void createClaimWithLogging(String wikidataEntityId, String fileName) {
+    public void createClaimWithLogging(String wikidataEntityId, String fileName, @NonNull String p18Value) {
         if (wikidataEntityId == null) {
             Timber.d("Skipping creation of claim as Wikidata entity ID is null");
             return;
@@ -64,6 +66,11 @@ public void createClaimWithLogging(String wikidataEntityId, String fileName) {
             return;
         }
 
+        if (!p18Value.trim().isEmpty()) {
+            Timber.d("Skipping creation of claim as p18Value is not empty, we won't override existing image");
+            return;
+        }
+
         editWikidataProperty(wikidataEntityId, fileName);
     }
 

app/src/test/kotlin/fr/free/nrw/commons/wikidata/WikidataEditServiceTest.kt

@@ -34,21 +34,27 @@ class WikidataEditServiceTest {
 
     @Test
     fun noClaimsWhenEntityIdIsNull() {
-        wikidataEditService!!.createClaimWithLogging(null, "Test.jpg")
+        wikidataEditService!!.createClaimWithLogging(null, "Test.jpg","")
         verifyZeroInteractions(wikidataClient!!)
     }
 
     @Test
     fun noClaimsWhenFileNameIsNull() {
-        wikidataEditService!!.createClaimWithLogging("Q1", null)
+        wikidataEditService!!.createClaimWithLogging("Q1", null,"")
+        verifyZeroInteractions(wikidataClient!!)
+    }
+
+    @Test
+    fun noClaimsWhenP18IsNotEmpty() {
+        wikidataEditService!!.createClaimWithLogging("Q1", "Test.jpg","Previous.jpg")
         verifyZeroInteractions(wikidataClient!!)
     }
 
     @Test
     fun noClaimsWhenLocationIsNotCorrect() {
         `when`(directKvStore!!.getBoolean("Picture_Has_Correct_Location", true))
                 .thenReturn(false)
-        wikidataEditService!!.createClaimWithLogging("Q1", "Test.jpg")
+        wikidataEditService!!.createClaimWithLogging("Q1", "Test.jpg","")
         verifyZeroInteractions(wikidataClient!!)
     }
 
@@ -60,7 +66,7 @@ class WikidataEditServiceTest {
                 .thenReturn(Observable.just(1L))
         `when`(wikidataClient!!.addEditTag(anyLong(), ArgumentMatchers.anyString(), ArgumentMatchers.anyString()))
                 .thenReturn(Observable.just(mock(AddEditTagResponse::class.java)))
-        wikidataEditService!!.createClaimWithLogging("Q1", "Test.jpg")
+        wikidataEditService!!.createClaimWithLogging("Q1", "Test.jpg","")
         verify(wikidataClient!!, times(1))
                 .createClaim(ArgumentMatchers.anyString(), ArgumentMatchers.anyString())
     }