Jonaskeutel has uploaded a new change for review.
https://gerrit.wikimedia.org/r/212537
Change subject: save parameters in violation
......................................................................
save parameters in violation
enabling us to display nice messages and distinguish bad bad and very bad
violations for the UI
Change-Id: Ib07d4f38acdc92c68671c746a28eb71810f22f64
---
M includes/ConstraintCheck/Checker/CommonsLinkChecker.php
M includes/ConstraintCheck/Checker/ConflictsWithChecker.php
M includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
M includes/ConstraintCheck/Checker/FormatChecker.php
M includes/ConstraintCheck/Checker/InverseChecker.php
M includes/ConstraintCheck/Checker/ItemChecker.php
M includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php
M includes/ConstraintCheck/Checker/MultiValueChecker.php
M includes/ConstraintCheck/Checker/OneOfChecker.php
M includes/ConstraintCheck/Checker/QualifierChecker.php
M includes/ConstraintCheck/Checker/QualifiersChecker.php
M includes/ConstraintCheck/Checker/RangeChecker.php
M includes/ConstraintCheck/Checker/SingleValueChecker.php
M includes/ConstraintCheck/Checker/SymmetricChecker.php
M includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
M includes/ConstraintCheck/Checker/TypeChecker.php
M includes/ConstraintCheck/Checker/UniqueValueChecker.php
M includes/ConstraintCheck/Checker/ValueTypeChecker.php
M includes/ConstraintCheck/Result/CheckResultToViolationTranslator.php
M tests/phpunit/Result/CheckResultToViolationTranslatorTest.php
20 files changed, 103 insertions(+), 7 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikidataQualityConstraints
refs/changes/37/212537/1
diff --git a/includes/ConstraintCheck/Checker/CommonsLinkChecker.php
b/includes/ConstraintCheck/Checker/CommonsLinkChecker.php
index 33a57fb..3ca6d8e 100755
--- a/includes/ConstraintCheck/Checker/CommonsLinkChecker.php
+++ b/includes/ConstraintCheck/Checker/CommonsLinkChecker.php
@@ -47,7 +47,13 @@
public function checkConstraint( Statement $statement, Constraint
$constraint, Entity $entity = null ) {
$parameters = array ();
$constraintParameters = $constraint->getConstraintParameters();
- $parameters[ 'namespace' ] =
$this->helper->parseSingleParameter( $constraintParameters['namespace'], true );
+ if ( array_key_exists( 'namespace', $constraintParameters ) ) {
+ $parameters[ 'namespace' ] =
$this->helper->parseSingleParameter( $constraintParameters['namespace'], true );
+ }
+
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
$mainSnak = $statement->getClaim()->getMainSnak();
diff --git a/includes/ConstraintCheck/Checker/ConflictsWithChecker.php
b/includes/ConstraintCheck/Checker/ConflictsWithChecker.php
index a39027d..883a2cc 100755
--- a/includes/ConstraintCheck/Checker/ConflictsWithChecker.php
+++ b/includes/ConstraintCheck/Checker/ConflictsWithChecker.php
@@ -77,6 +77,10 @@
$parameters[ 'item' ] =
$this->constraintReportHelper->parseParameterArray( explode( ',',
$constraintParameters[ 'item' ] ) );
};
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
/*
* 'Conflicts with' can be defined with
* a) a property only
diff --git a/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
b/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
index 50c8083..23417a7 100755
--- a/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
+++ b/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
@@ -59,6 +59,10 @@
$parameters['property'] =
$this->constraintReportHelper->parseSingleParameter(
$constraintParameters['property'], 'PropertyId' );
}
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$mainSnak = $statement->getClaim()->getMainSnak();
/*
diff --git a/includes/ConstraintCheck/Checker/FormatChecker.php
b/includes/ConstraintCheck/Checker/FormatChecker.php
index 279cb6d..78e3d40 100755
--- a/includes/ConstraintCheck/Checker/FormatChecker.php
+++ b/includes/ConstraintCheck/Checker/FormatChecker.php
@@ -55,6 +55,10 @@
return new CheckResult( $statement,
$constraint->getConstraintTypeQid(), $parameters,
CheckResult::STATUS_VIOLATION, $message );
}
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$mainSnak = $statement->getClaim()->getMainSnak();
/*
diff --git a/includes/ConstraintCheck/Checker/InverseChecker.php
b/includes/ConstraintCheck/Checker/InverseChecker.php
index 820e931..3adaa49 100755
--- a/includes/ConstraintCheck/Checker/InverseChecker.php
+++ b/includes/ConstraintCheck/Checker/InverseChecker.php
@@ -70,6 +70,10 @@
$parameters['property'] =
$this->constraintReportHelper->parseSingleParameter(
$constraintParameters['property'] );
};
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$mainSnak = $statement->getClaim()->getMainSnak();
/*
diff --git a/includes/ConstraintCheck/Checker/ItemChecker.php
b/includes/ConstraintCheck/Checker/ItemChecker.php
index c1caa7d..9103d2c 100755
--- a/includes/ConstraintCheck/Checker/ItemChecker.php
+++ b/includes/ConstraintCheck/Checker/ItemChecker.php
@@ -76,6 +76,10 @@
$parameters['item'] =
$this->constraintReportHelper->parseParameterArray( $items );
}
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
/*
* error handling:
* parameter $property must not be null
diff --git a/includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php
b/includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php
index 350a7dc..c55a8f7 100755
--- a/includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php
+++ b/includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php
@@ -50,6 +50,11 @@
if ( array_key_exists( 'property', $constraintParameters ) ) {
$properties = explode( ',',
$constraintParameters['property'] );
}
+
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$parameters[ 'property' ] = $this->helper->parseParameterArray(
$properties );
$qualifiersList = $statement->getQualifiers();
$qualifiers = array ();
diff --git a/includes/ConstraintCheck/Checker/MultiValueChecker.php
b/includes/ConstraintCheck/Checker/MultiValueChecker.php
index adb753e..fe351ac 100755
--- a/includes/ConstraintCheck/Checker/MultiValueChecker.php
+++ b/includes/ConstraintCheck/Checker/MultiValueChecker.php
@@ -42,6 +42,11 @@
$parameters = array ();
+ $constraintParameters = $constraint->getConstraintParameters();
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$propertyCountArray =
$this->valueCountCheckerHelper->getPropertyCount( $entity->getStatements() );
if ( $propertyCountArray[ $propertyId->getNumericId() ] <= 1 ) {
diff --git a/includes/ConstraintCheck/Checker/OneOfChecker.php
b/includes/ConstraintCheck/Checker/OneOfChecker.php
index 395a7b8..1100400 100755
--- a/includes/ConstraintCheck/Checker/OneOfChecker.php
+++ b/includes/ConstraintCheck/Checker/OneOfChecker.php
@@ -51,6 +51,10 @@
$parameters['item'] =
$this->helper->parseParameterArray( $items );
}
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$mainSnak = $statement->getClaim()->getMainSnak();
/*
diff --git a/includes/ConstraintCheck/Checker/QualifierChecker.php
b/includes/ConstraintCheck/Checker/QualifierChecker.php
index 936de80..cb5abd5 100755
--- a/includes/ConstraintCheck/Checker/QualifierChecker.php
+++ b/includes/ConstraintCheck/Checker/QualifierChecker.php
@@ -44,6 +44,12 @@
* @return CheckResult
*/
public function checkConstraint( Statement $statement, Constraint
$constraint, Entity $entity = null ) {
+
+ $constraintParameters = $constraint->getConstraintParameters();
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$message = 'The property must only be used as a qualifier.';
return new CheckResult( $statement,
$constraint->getConstraintTypeQid(), array (), CheckResult::STATUS_VIOLATION,
$message );
}
diff --git a/includes/ConstraintCheck/Checker/QualifiersChecker.php
b/includes/ConstraintCheck/Checker/QualifiersChecker.php
index a226d6a..664d059 100755
--- a/includes/ConstraintCheck/Checker/QualifiersChecker.php
+++ b/includes/ConstraintCheck/Checker/QualifiersChecker.php
@@ -46,6 +46,11 @@
$parameters = array ();
$constraintParameters = $constraint->getConstraintParameters();
+
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$parameters[ 'property' ] = $this->helper->parseParameterArray(
explode( ',', $constraintParameters['property'] ) );
/*
diff --git a/includes/ConstraintCheck/Checker/RangeChecker.php
b/includes/ConstraintCheck/Checker/RangeChecker.php
index ca2301c..229804c 100755
--- a/includes/ConstraintCheck/Checker/RangeChecker.php
+++ b/includes/ConstraintCheck/Checker/RangeChecker.php
@@ -56,6 +56,10 @@
$parameters = array ();
$constraintParameters = $constraint->getConstraintParameters();
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$mainSnak = $statement->getClaim()->getMainSnak();
/*
diff --git a/includes/ConstraintCheck/Checker/SingleValueChecker.php
b/includes/ConstraintCheck/Checker/SingleValueChecker.php
index 8718772..1c1ae3a 100755
--- a/includes/ConstraintCheck/Checker/SingleValueChecker.php
+++ b/includes/ConstraintCheck/Checker/SingleValueChecker.php
@@ -42,6 +42,11 @@
$parameters = array ();
+ $constraintParameters = $constraint->getConstraintParameters();
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$propertyCountArray =
$this->valueCountCheckerHelper->getPropertyCount( $entity->getStatements() );
if ( $propertyCountArray[ $propertyId->getNumericId() ] > 1 ) {
diff --git a/includes/ConstraintCheck/Checker/SymmetricChecker.php
b/includes/ConstraintCheck/Checker/SymmetricChecker.php
index 704179d..49925f5 100755
--- a/includes/ConstraintCheck/Checker/SymmetricChecker.php
+++ b/includes/ConstraintCheck/Checker/SymmetricChecker.php
@@ -64,6 +64,11 @@
public function checkConstraint( Statement $statement, Constraint
$constraint, Entity $entity = null ) {
$parameters = array ();
+ $constraintParameters = $constraint->getConstraintParameters();
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$mainSnak = $statement->getClaim()->getMainSnak();
$propertyId = $statement->getClaim()->getPropertyId();
diff --git a/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
b/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
index 9c6af48..b6cb41f 100755
--- a/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
+++ b/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
@@ -79,6 +79,10 @@
$parameters['item'] =
$this->constraintReportHelper->parseParameterArray( $items );
}
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$mainSnak = $statement->getClaim()->getMainSnak();
/*
diff --git a/includes/ConstraintCheck/Checker/TypeChecker.php
b/includes/ConstraintCheck/Checker/TypeChecker.php
index 4b02e40..1f80631 100755
--- a/includes/ConstraintCheck/Checker/TypeChecker.php
+++ b/includes/ConstraintCheck/Checker/TypeChecker.php
@@ -78,6 +78,10 @@
$parameters['relation'] =
$this->helper->parseSingleParameter( $relation, true );
}
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
/*
* error handling:
* parameter $constraintParameters['class'] must not be null
diff --git a/includes/ConstraintCheck/Checker/UniqueValueChecker.php
b/includes/ConstraintCheck/Checker/UniqueValueChecker.php
index 95e2460..ff8964d 100755
--- a/includes/ConstraintCheck/Checker/UniqueValueChecker.php
+++ b/includes/ConstraintCheck/Checker/UniqueValueChecker.php
@@ -42,6 +42,11 @@
public function checkConstraint( Statement $statement, Constraint
$constraint, Entity $entity = null ) {
$parameters = array ();
+ $constraintParameters = $constraint->getConstraintParameters();
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$message = 'For technical reasons, the check for this
constraint has not yet been implemented.';
return new CheckResult( $statement,
$constraint->getConstraintTypeQid(), $parameters, CheckResult::STATUS_TODO,
$message );
}
diff --git a/includes/ConstraintCheck/Checker/ValueTypeChecker.php
b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
index fe6c007..a3087b1 100755
--- a/includes/ConstraintCheck/Checker/ValueTypeChecker.php
+++ b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
@@ -78,6 +78,11 @@
$relation = $constraintParameters['relation'];
$parameters['relation'] =
$this->helper->parseSingleParameter( $relation, true );
}
+
+ if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
+ $parameters[ 'constraint_status' ] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
+ }
+
$mainSnak = $statement->getClaim()->getMainSnak();
/*
diff --git
a/includes/ConstraintCheck/Result/CheckResultToViolationTranslator.php
b/includes/ConstraintCheck/Result/CheckResultToViolationTranslator.php
index 07ff873..32171cc 100755
--- a/includes/ConstraintCheck/Result/CheckResultToViolationTranslator.php
+++ b/includes/ConstraintCheck/Result/CheckResultToViolationTranslator.php
@@ -23,6 +23,12 @@
$this->entityRevisionLookup = $entityRevisionLookup;
}
+ /**
+ * @param Entity $entity
+ * @param CheckResult[] $checkResultOrArray
+ *
+ * @return array
+ */
public function translateToViolation( Entity $entity,
$checkResultOrArray ) {
$checkResultArray = $this->setCheckResultArray( $checkResultOrArray
);
@@ -42,8 +48,8 @@
$constraintId = $this->setConstraintId( $checkResult, $statement,
$constraintTypeEntityId );
$revisionId =
$this->entityRevisionLookup->getLatestRevisionId( $entityId );
$status = CheckResult::STATUS_VIOLATION;
-
- $violationArray[ ] = new Violation( $entityId,
$propertyId, $claimGuid, $constraintId, $constraintTypeEntityId, $revisionId,
$status );
+ $parameters = json_encode( $checkResult->getParameters() );
+ $violationArray[ ] = new Violation( $entityId,
$propertyId, $claimGuid, $constraintId, $constraintTypeEntityId, $revisionId,
$status, $parameters );
}
return $violationArray;
diff --git a/tests/phpunit/Result/CheckResultToViolationTranslatorTest.php
b/tests/phpunit/Result/CheckResultToViolationTranslatorTest.php
index 007ec9f..76a9190 100644
--- a/tests/phpunit/Result/CheckResultToViolationTranslatorTest.php
+++ b/tests/phpunit/Result/CheckResultToViolationTranslatorTest.php
@@ -27,7 +27,7 @@
* @author BP2014N1
* @license GNU GPL v2+
*/
-class CheckResultTestToViolationTranslator extends \MediaWikiTestCase {
+class CheckResultTestToViolationTranslatorTest extends \MediaWikiTestCase {
/**
* @var CheckResultToViolationTranslator
@@ -81,7 +81,7 @@
$this->propertyId = new PropertyId( 'P1' );
$this->claimGuid = 'P1$aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee';
$this->statement->setGuid(
'P1$aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' );
- $this->constraintName = 'Range';
+ $this->constraintName = 'Single value';
$this->parameters = array ();
$this->message = 'All right';
$this->entity = new Item();
@@ -106,7 +106,7 @@
$this->assertEquals( array (), $violations );
}
- public function testSingleViolationResult() {
+ public function testSingleViolationResultWithoutParameter() {
$checkResult = new CheckResult( $this->statement,
$this->constraintName, $this->parameters, 'violation', $this->message );
$violations = $this->translator->translateToViolation(
$this->entity, $checkResult );
$this->assertEquals( 1, sizeof( $violations ) );
@@ -117,8 +117,15 @@
$this->assertEquals( $this->statement->getGuid(),
$violation->getClaimGuid() );
$this->assertEquals( md5( $this->statement->getGuid() .
$checkResult->getConstraintName() ), $violation->getConstraintId() );
$this->assertEquals( $checkResult->getConstraintName(),
$violation->getConstraintTypeEntityId() );
- $this->assertEquals( 42, $violation->getRevisionId() );
+ $this->assertEquals( 42, $violation->getRevisionId() );
+ $this->assertEquals( '[]', $violation->getAdditionalInfo() );
+ }
+ public function testSingleViolationResultWithParameter() {
+ $checkResult = new CheckResult( $this->statement,
$this->constraintName, array( array( 'constraint_status' => 'mandatory' ) ),
'violation', $this->message );
+ $violations = $this->translator->translateToViolation(
$this->entity, $checkResult );
+ $violation = $violations[0];
+ $this->assertEquals( '[{"constraint_status":"mandatory"}]',
$violation->getAdditionalInfo() );
}
public function testMultipleCheckResults() {
--
To view, visit https://gerrit.wikimedia.org/r/212537
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib07d4f38acdc92c68671c746a28eb71810f22f64
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikidataQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Jonaskeutel <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
