jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/353852 )
Change subject: Improve violation messages for Item and Target required claim
......................................................................
Improve violation messages for Item and Target required claim
The previously separate messages for “no items” and “one or more items”
are merged; message translations can distinguish between the two cases
using the PLURAL magic word.
Bug: T164354
Change-Id: Ic6f4c99019a6d9d7180718127e7e7c0d7cfd9569
---
M i18n/en.json
M i18n/qqq.json
M includes/ConstraintCheck/Checker/ItemChecker.php
M includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
M includes/ConstraintReportFactory.php
M tests/phpunit/Checker/ConnectionChecker/ItemCheckerTest.php
M tests/phpunit/Checker/ConnectionChecker/TargetRequiredClaimCheckerTest.php
7 files changed, 84 insertions(+), 25 deletions(-)
Approvals:
Jonas Kress (WMDE): Looks good to me, approved
jenkins-bot: Verified
diff --git a/i18n/en.json b/i18n/en.json
index 68cb4b4..9678c0c 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -66,8 +66,7 @@
"wbqc-violation-message-diff-within-range-must-have-equal-types": "The
property defined in the parameters must have a value of the same type as this
property.",
"wbqc-violation-message-format": "The property's value must match the
pattern defined in the parameters.",
"wbqc-violation-message-inverse": "$1 should also have the inverse
statement $2 $3.",
- "wbqc-violation-message-item-property": "This property must only be
used when there is another statement using the property defined in the
parameters.",
- "wbqc-violation-message-item-claim": "This property must only be used
when there is another statement using the property with one of the values
defined in the parameters.",
+ "wbqc-violation-message-item": "An entity with $1 should also have
{{PLURAL:$3|0=a statement $2.|1=a statement $2 $5.|a statement for $2 with one
of the following values:$4}}",
"wbqc-violation-message-mandatory-qualifiers": "All of the properties
defined in the parameters have to be used as qualifiers on this statement.",
"wbqc-violation-message-multi-value": "This property must have multiple
values. That is, there must be more than one claim using this property.",
"wbqc-violation-message-one-of": "The value for $1 should be
{{PLURAL:$2|1=$4.|2=either $4 or $5.|one of the following:$3}}",
@@ -82,6 +81,5 @@
"wbqc-violation-message-type-subclass": "Entities using the $1 property
should be subclasses of {{PLURAL:$3|1=$5|2=$5 or $6|one of the following
classes}} (or of {{PLURAL:$3|1=a subclass of it|2=a subclass of them|one of
their subclasses}}), but $2 currently {{PLURAL:$3|1=isn't.|2=isn't.|isn't:
$4}}",
"wbqc-violation-message-valueType-instance": "Values of $1 statements
should be instances of {{PLURAL:$3|1=$5|2=$5 or $6|one of the following
classes}} (or of {{PLURAL:$3|1=a subclass of it|2=a subclass of them|one of
their subclasses}}), but $2 currently {{PLURAL:$3|1=isn't.|2=isn't.|isn't:
$4}}",
"wbqc-violation-message-valueType-subclass": "Values of $1 statements
should be subclasses of {{PLURAL:$3|1=$5|2=$5 or $6|one of the following
classes}} (or of {{PLURAL:$3|1=a subclass of it|2=a subclass of them|one of
their subclasses}}), but $2 currently {{PLURAL:$3|1=isn't.|2=isn't.|isn't:
$4}}",
- "wbqc-violation-message-target-required-claim-property": "This property
must only be used when there is a statement on its value entity using the
property defined in the parameters.",
- "wbqc-violation-message-target-required-claim-claim": "This property
must only be used when there is a statement on its value entity using the
property with one of the values defined in the parameters."
+ "wbqc-violation-message-target-required-claim": "$1 should have
{{PLURAL:$3|0=a statement $2.|1=a statement $2 $5.|a statement for $2 with one
of the following values:$4}}"
}
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 3d9a0e6..88f578d 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -61,8 +61,7 @@
"wbqc-violation-message-diff-within-range-must-have-equal-types":
"Message for violation of Diff within range constraint. When this and the given
property differ in value type.",
"wbqc-violation-message-format": "Message for violation of format
constraint. When string does not match given pattern.",
"wbqc-violation-message-inverse": "Message for a violation of the
“Inverse” constraint, when the inverse statement of a statement does not exist.
$1, $2 and $3 contain the expected subject entity, property, and target entity
of the missing inverse statement.",
- "wbqc-violation-message-item-property": "Message for violation of Item
constraint. When only a property is given.",
- "wbqc-violation-message-item-claim": "Message for violation of Item
constraint. When property and value are given.",
+ "wbqc-violation-message-item": "Message for a violation of the “Item”
constraint, when the subject entity of a statement is missing another
statement. Parameters:\n* $1 is the property of the statement that has the
constraint.\n* $2 is the property of the missing statement.\n* $3 is the number
of values permitted for the missing statement (or 0, in which case the
constraint only specifies that there should be a statement but not the values
it should have).\n* $4 is an HTML list of all values permitted for the missing
statement.\n* $5, $6 etc. are the individual values permitted for the missing
statement.\n{{Related|wbqc-violation-message-target-required-claim}}",
"wbqc-violation-message-mandatory-qualifiers": "Message for violation
of Mandatory qualifiers constraint. When a qualifier is missing.",
"wbqc-violation-message-multi-value": "Message for violation of Multi
value constraint. When no more than one value exists.",
"wbqc-violation-message-one-of": "Message for a violation of the “One
of” constraint, when the value is not one of the allowed values. $1 is the
property, $2 the number of allowed values (note that certain constraints have
only a single allowed value), $3 an HTML list of all allowed values, and $4, $5
etc. are the individual allowed values.",
@@ -77,6 +76,5 @@
"wbqc-violation-message-type-subclass": "Message for a violation of the
“Type” constraint, when the subject of a statement should have be a subclass of
a certain type but isn't. $1 is the property of the statement, $2 is the
subject of the statement, $3 is the number of classes, $4 is an HTML list of
all classes, and $5, $6 etc. are the individual
classes.{{Related|wbqc-violation-message-type-instance}}",
"wbqc-violation-message-valueType-instance": "Message for a violation
of the “Value type” constraint, when the value of a statement should have be an
instance of a certain type but isn't. $1 is the property of the statement, $2
is the value of the statement, $3 is the number of classes, $4 is an HTML list
of all classes, and $5, $6 etc. are the individual
classes.{{Related|wbqc-violation-message-valueType-subclass}}",
"wbqc-violation-message-valueType-subclass": "Message for a violation
of the “Value type” constraint, when the value of a statement should have be a
subclass of a certain type but isn't. $1 is the property of the statement, $2
is the value of the statement, $3 is the number of classes, $4 is an HTML list
of all classes, and $5, $6 etc. are the individual
classes.{{Related|wbqc-violation-message-valueType-instance}}",
- "wbqc-violation-message-target-required-claim-property": "Message for
violation of Target required claim constraint. When only a property is given.",
- "wbqc-violation-message-target-required-claim-claim": "Message for
violation of Target required claim constraint. When property and value are
given."
+ "wbqc-violation-message-target-required-claim": "Message for a
violation of the “Target required claim” constraint, when the target entity of
a statement is missing an expected statement. Parameters:\n* $1 is the subject
entity of the missing statement, i. e. the target entity of the statement that
has the constraint.\n* $2 is the property of the missing statement.\n* $3 is
the number of values permitted for the missing statement (or 0, in which case
the constraint only specifies that there should be a statement but not the
values it should have).\n* $4 is an HTML list of all values permitted for the
missing statement.\n* $5, $6 etc. are the individual values permitted for the
missing statement.\n{{Related|wbqc-violation-message-item}}"
}
diff --git a/includes/ConstraintCheck/Checker/ItemChecker.php
b/includes/ConstraintCheck/Checker/ItemChecker.php
index 28a1c77..2ee8282 100644
--- a/includes/ConstraintCheck/Checker/ItemChecker.php
+++ b/includes/ConstraintCheck/Checker/ItemChecker.php
@@ -10,6 +10,7 @@
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConnectionCheckerHelper;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
+use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
use Wikibase\DataModel\Statement\Statement;
/**
@@ -35,14 +36,26 @@
private $connectionCheckerHelper;
/**
+ * @var ConstraintParameterRenderer
+ */
+ private $constraintParameterRenderer;
+
+ /**
* @param EntityLookup $lookup
* @param ConstraintParameterParser $helper
* @param ConnectionCheckerHelper $connectionCheckerHelper
+ * @param ConstraintParameterRenderer $constraintParameterRenderer
*/
- public function __construct( EntityLookup $lookup,
ConstraintParameterParser $helper, ConnectionCheckerHelper
$connectionCheckerHelper ) {
+ public function __construct(
+ EntityLookup $lookup,
+ ConstraintParameterParser $helper,
+ ConnectionCheckerHelper $connectionCheckerHelper,
+ ConstraintParameterRenderer $constraintParameterRenderer
+ ) {
$this->entityLookup = $lookup;
$this->constraintParameterParser = $helper;
$this->connectionCheckerHelper = $connectionCheckerHelper;
+ $this->constraintParameterRenderer =
$constraintParameterRenderer;
}
/**
@@ -64,7 +77,7 @@
$parameters['property'] =
$this->constraintParameterParser->parseSingleParameter( $property );
}
- $items = false;
+ $items = [];
if ( array_key_exists( 'item', $constraintParameters ) ) {
$items = explode( ',', $constraintParameters['item'] );
$parameters['item'] =
$this->constraintParameterParser->parseParameterArray( $items );
@@ -86,22 +99,31 @@
*/
if ( !$items ) {
if ( $this->connectionCheckerHelper->hasProperty(
$entity->getStatements(), $property ) ) {
- $message = '';
$status = CheckResult::STATUS_COMPLIANCE;
} else {
- $message = wfMessage(
"wbqc-violation-message-item-property" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
}
} else {
if ( $this->connectionCheckerHelper->findStatement(
$entity->getStatements(), $property, $items ) !== null ) {
- $message = '';
$status = CheckResult::STATUS_COMPLIANCE;
} else {
- $message = wfMessage(
"wbqc-violation-message-item-claim" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
}
}
+ if ( $status == CheckResult::STATUS_COMPLIANCE ) {
+ $message = '';
+ } else {
+ $message = wfMessage( 'wbqc-violation-message-item' );
+ $message->rawParams(
+
$this->constraintParameterRenderer->formatEntityId( $statement->getPropertyId()
),
+
$this->constraintParameterRenderer->formatPropertyId( $property )
+ );
+ $message->numParams( count( $items ) );
+ $message->rawParams(
$this->constraintParameterRenderer->formatItemIdList( $items ) );
+ $message = $message->escaped();
+ }
+
return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, $status, $message );
}
diff --git a/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
b/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
index 1cc882c..bce95ec 100644
--- a/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
+++ b/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
@@ -12,6 +12,7 @@
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConnectionCheckerHelper;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
+use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
use Wikibase\DataModel\Statement\Statement;
/**
@@ -37,14 +38,26 @@
private $connectionCheckerHelper;
/**
+ * @var ConstraintParameterRenderer
+ */
+ private $constraintParameterRenderer;
+
+ /**
* @param EntityLookup $lookup
* @param ConstraintParameterParser $helper
* @param ConnectionCheckerHelper $connectionCheckerHelper
+ * @param ConstraintParameterRenderer $constraintParameterRenderer
*/
- public function __construct( EntityLookup $lookup,
ConstraintParameterParser $helper, ConnectionCheckerHelper
$connectionCheckerHelper ) {
+ public function __construct(
+ EntityLookup $lookup,
+ ConstraintParameterParser $helper,
+ ConnectionCheckerHelper $connectionCheckerHelper,
+ ConstraintParameterRenderer $constraintParameterRenderer
+ ) {
$this->entityLookup = $lookup;
$this->constraintParameterParser = $helper;
$this->connectionCheckerHelper = $connectionCheckerHelper;
+ $this->constraintParameterRenderer =
$constraintParameterRenderer;
}
/**
@@ -66,7 +79,7 @@
$parameters['property'] =
$this->constraintParameterParser->parseSingleParameter( $property );
}
- $items = false;
+ $items = [];
if ( array_key_exists( 'item', $constraintParameters ) ) {
$items = explode( ',', $constraintParameters['item'] );
$parameters['item'] =
$this->constraintParameterParser->parseParameterArray( $items );
@@ -105,7 +118,8 @@
return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $message );
}
- $targetEntity = $this->entityLookup->getEntity(
$dataValue->getEntityId() );
+ $targetEntityId = $dataValue->getEntityId();
+ $targetEntity = $this->entityLookup->getEntity( $targetEntityId
);
if ( $targetEntity === null ) {
$message = wfMessage(
"wbqc-violation-message-target-entity-must-exist" )->escaped();
return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $message );
@@ -119,22 +133,31 @@
*/
if ( !$items ) {
if ( $this->connectionCheckerHelper->hasProperty(
$targetEntityStatementList, $property ) ) {
- $message = '';
$status = CheckResult::STATUS_COMPLIANCE;
} else {
- $message = wfMessage(
"wbqc-violation-message-target-required-claim-property" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
}
} else {
if ( $this->connectionCheckerHelper->findStatement(
$targetEntityStatementList, $property, $items ) !== null ) {
- $message = '';
$status = CheckResult::STATUS_COMPLIANCE;
} else {
- $message = wfMessage(
"wbqc-violation-message-target-required-claim-claim" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
}
}
+ if ( $status == CheckResult::STATUS_COMPLIANCE ) {
+ $message = '';
+ } else {
+ $message = wfMessage(
'wbqc-violation-message-target-required-claim' );
+ $message->rawParams(
+
$this->constraintParameterRenderer->formatItemId( $targetEntityId ),
+
$this->constraintParameterRenderer->formatPropertyId( $property )
+ );
+ $message->numParams( count( $items ) );
+ $message->rawParams(
$this->constraintParameterRenderer->formatItemIdList( $items ) );
+ $message = $message->escaped();
+ }
+
return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, $status, $message );
}
diff --git a/includes/ConstraintReportFactory.php
b/includes/ConstraintReportFactory.php
index 04be725..1036d2f 100644
--- a/includes/ConstraintReportFactory.php
+++ b/includes/ConstraintReportFactory.php
@@ -148,8 +148,8 @@
$this->constraintCheckerMap = [
'Conflicts with' => new ConflictsWithChecker(
$this->lookup, $constraintParameterParser, $connectionCheckerHelper,
$this->constraintParameterRenderer ),
- 'Item' => new ItemChecker( $this->lookup,
$constraintParameterParser, $connectionCheckerHelper ),
- 'Target required claim' => new
TargetRequiredClaimChecker( $this->lookup, $constraintParameterParser,
$connectionCheckerHelper ),
+ 'Item' => new ItemChecker( $this->lookup,
$constraintParameterParser, $connectionCheckerHelper,
$this->constraintParameterRenderer ),
+ 'Target required claim' => new
TargetRequiredClaimChecker( $this->lookup, $constraintParameterParser,
$connectionCheckerHelper, $this->constraintParameterRenderer ),
'Symmetric' => new SymmetricChecker(
$this->lookup, $constraintParameterParser, $connectionCheckerHelper,
$this->constraintParameterRenderer ),
'Inverse' => new InverseChecker( $this->lookup,
$constraintParameterParser, $connectionCheckerHelper,
$this->constraintParameterRenderer ),
'Qualifier' => new QualifierChecker(
$constraintParameterParser ),
diff --git a/tests/phpunit/Checker/ConnectionChecker/ItemCheckerTest.php
b/tests/phpunit/Checker/ConnectionChecker/ItemCheckerTest.php
index 95131c2..bec8bb3 100644
--- a/tests/phpunit/Checker/ConnectionChecker/ItemCheckerTest.php
+++ b/tests/phpunit/Checker/ConnectionChecker/ItemCheckerTest.php
@@ -2,7 +2,9 @@
namespace WikibaseQuality\ConstraintReport\Test\ConnectionChecker;
+use ValueFormatters\ValueFormatter;
use Wikibase\DataModel\Statement\Statement;
+use Wikibase\DataModel\Services\EntityId\PlainEntityIdFormatter;
use Wikibase\DataModel\Snak\PropertyValueSnak;
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Entity\ItemId;
@@ -11,6 +13,7 @@
use WikibaseQuality\ConstraintReport\ConstraintCheck\Checker\ItemChecker;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConnectionCheckerHelper;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
+use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
use WikibaseQuality\Tests\Helper\JsonFileEntityLookup;
/**
@@ -51,7 +54,13 @@
$this->lookup = new JsonFileEntityLookup( __DIR__ );
$this->helper = new ConstraintParameterParser();
$this->connectionCheckerHelper = new ConnectionCheckerHelper();
- $this->checker = new ItemChecker( $this->lookup, $this->helper,
$this->connectionCheckerHelper );
+ $valueFormatter = $this->getMock( ValueFormatter::class );
+ $valueFormatter->method( 'format' )->willReturn( '' );
+ $constraintParameterRenderer = new ConstraintParameterRenderer(
+ new PlainEntityIdFormatter(),
+ $valueFormatter
+ );
+ $this->checker = new ItemChecker( $this->lookup, $this->helper,
$this->connectionCheckerHelper, $constraintParameterRenderer );
}
protected function tearDown() {
diff --git
a/tests/phpunit/Checker/ConnectionChecker/TargetRequiredClaimCheckerTest.php
b/tests/phpunit/Checker/ConnectionChecker/TargetRequiredClaimCheckerTest.php
index 6e866ba..b3b13b9 100644
--- a/tests/phpunit/Checker/ConnectionChecker/TargetRequiredClaimCheckerTest.php
+++ b/tests/phpunit/Checker/ConnectionChecker/TargetRequiredClaimCheckerTest.php
@@ -2,7 +2,9 @@
namespace WikibaseQuality\ConstraintReport\Test\ConnectionChecker;
+use ValueFormatters\ValueFormatter;
use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\DataModel\Services\EntityId\PlainEntityIdFormatter;
use Wikibase\DataModel\Snak\PropertyNoValueSnak;
use Wikibase\DataModel\Statement\Statement;
use Wikibase\DataModel\Snak\PropertyValueSnak;
@@ -15,6 +17,7 @@
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Checker\TargetRequiredClaimChecker;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConnectionCheckerHelper;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
+use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
use WikibaseQuality\Tests\Helper\JsonFileEntityLookup;
/**
@@ -55,7 +58,13 @@
$this->lookup = new JsonFileEntityLookup( __DIR__ );
$this->helper = new ConstraintParameterParser();
$this->connectionCheckerHelper = new ConnectionCheckerHelper();
- $this->checker = new TargetRequiredClaimChecker( $this->lookup,
$this->helper, $this->connectionCheckerHelper );
+ $valueFormatter = $this->getMock( ValueFormatter::class );
+ $valueFormatter->method( 'format' )->willReturn( '' );
+ $constraintParameterRenderer = new ConstraintParameterRenderer(
+ new PlainEntityIdFormatter(),
+ $valueFormatter
+ );
+ $this->checker = new TargetRequiredClaimChecker( $this->lookup,
$this->helper, $this->connectionCheckerHelper, $constraintParameterRenderer );
}
protected function tearDown() {
--
To view, visit https://gerrit.wikimedia.org/r/353852
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic6f4c99019a6d9d7180718127e7e7c0d7cfd9569
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <lucas.werkmeis...@wikimedia.de>
Gerrit-Reviewer: Abián <davidab...@wikimedia.es>
Gerrit-Reviewer: Jonas Kress (WMDE) <jonas.kr...@wikimedia.de>
Gerrit-Reviewer: Lucas Werkmeister (WMDE) <lucas.werkmeis...@wikimedia.de>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
