Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 17, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import android.annotation.SuppressLint;
import android.content.Context;

import androidx.annotation.NonNull;
import java.util.Locale;

import javax.inject.Inject;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!!)
}

Expand All @@ -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())
}
Expand Down