From e5d5a7af92f75318b3355333f571cb43a1a9ea2d Mon Sep 17 00:00:00 2001 From: neslihanturan Date: Tue, 17 Mar 2020 12:35:21 +0200 Subject: [PATCH] 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 --- .../nrw/commons/contributions/Contribution.java | 14 ++++++++++++++ .../fr/free/nrw/commons/upload/UploadModel.java | 2 ++ .../fr/free/nrw/commons/upload/UploadService.java | 7 ++++--- .../nrw/commons/wikidata/WikidataEditService.java | 13 ++++++++++--- .../commons/wikidata/WikidataEditServiceTest.kt | 14 ++++++++++---- 5 files changed, 40 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java b/app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java index 6a97c938d..f71df7d8d 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/Contribution.java @@ -68,6 +68,7 @@ public class Contribution extends Media { public String decimalCoords; public boolean isMultiple; public String wikiDataEntityId; + private String p18Value; public Uri contentProviderUri; public String dateCreatedSource; @@ -271,6 +272,19 @@ public class Contribution extends Media { 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; } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java index 172d7a360..f92f5bfa6 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java @@ -197,6 +197,8 @@ public class UploadModel { 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<>(); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java index b37dfd011..11f4f1419 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java @@ -278,9 +278,10 @@ public class UploadService extends HandlerService { 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); diff --git a/app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java b/app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java index e486d1150..abc72478d 100644 --- a/app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java +++ b/app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java @@ -3,6 +3,7 @@ package fr.free.nrw.commons.wikidata; 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 class WikidataEditService { 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); } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/wikidata/WikidataEditServiceTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/wikidata/WikidataEditServiceTest.kt index f8e24b6dc..e937952ad 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/wikidata/WikidataEditServiceTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/wikidata/WikidataEditServiceTest.kt @@ -34,13 +34,19 @@ 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!!) } @@ -48,7 +54,7 @@ class WikidataEditServiceTest { 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()) }