jenkins-bot has submitted this change and it was merged.
Change subject: Model internal and external surveys
......................................................................
Model internal and external surveys
* Add the Survey abstract base class, and the InternalSurvey and
ExternalSurvey classes, which model internal and external surveys
respectively.
* Add the SurveyFactory class that creates an instance of a model based
on some specification or throws an exception - with an informative
message - if it is invalid
* Replace most of the logic in the ResourceLoaderGetConfigVars and
ResourceLoaderRegisterModules handlers with simple manipulations of
models
Bug: T110196
Change-Id: I90b3ce5c7fb0ef06802cd9e0abe429e67bc742cc
---
M extension.json
A includes/ExternalSurvey.php
A includes/InternalSurvey.php
M includes/QuickSurveys.hooks.php
A includes/Survey.php
A includes/SurveyFactory.php
M tests/browser/LocalSettings.php
A tests/phpunit/SurveyFactoryTest.php
8 files changed, 501 insertions(+), 50 deletions(-)
Approvals:
Jdlrobson: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index 072d0aa..306de24 100644
--- a/extension.json
+++ b/extension.json
@@ -74,7 +74,11 @@
}
},
"AutoloadClasses": {
- "QuickSurveys\\Hooks": "includes/QuickSurveys.hooks.php"
+ "QuickSurveys\\Hooks": "includes/QuickSurveys.hooks.php",
+ "QuickSurveys\\SurveyFactory": "includes/SurveyFactory.php",
+ "QuickSurveys\\Survey": "includes/Survey.php",
+ "QuickSurveys\\InternalSurvey": "includes/InternalSurvey.php",
+ "QuickSurveys\\ExternalSurvey": "includes/ExternalSurvey.php"
},
"manifest_version": 1,
"Hooks": {
@@ -101,7 +105,7 @@
"@question": "survey question message key",
"question":
"ext-quicksurveys-example-internal-survey-question",
"@description": "The message key of the
description of the survey. Displayed immediately below the survey question.",
- "description": "",
+ "description":
"ext-quicksurveys-example-internal-survey-description",
"@answers": "possible answer message keys for
positive, neutral, and negative",
"answers": {
"positive":
"ext-quicksurveys-example-internal-survey-answer-positive",
@@ -113,7 +117,7 @@
"@enabled": "whether the survey is enabled",
"enabled": false,
"@coverage": "percentage of users that will see
the survey",
- "coverage": "50",
+ "coverage": 0.5,
"@platform": "for each platform (desktop,
mobile), which version of it is targeted (stable, beta, alpha)",
"platform": {
"desktop": ["stable"],
@@ -135,7 +139,7 @@
"@enabled": "whether the survey is enabled",
"enabled": false,
"@coverage": "percentage of users that will see
the survey",
- "coverage": "50",
+ "coverage": 0.5,
"@platform": "for each platform (desktop,
mobile), which version of it is targeted (stable, beta, alpha)",
"platform": {
"desktop": ["stable"],
diff --git a/includes/ExternalSurvey.php b/includes/ExternalSurvey.php
new file mode 100644
index 0000000..568de8f
--- /dev/null
+++ b/includes/ExternalSurvey.php
@@ -0,0 +1,43 @@
+<?php
+
+namespace QuickSurveys;
+
+class ExternalSurvey extends Survey
+{
+ /**
+ * @var string The URL of the external survey.
+ */
+ private $link;
+
+ /**
+ * @var string The description of the privacy policy of the website
that hosts the external survey.
+ */
+ private $privacyPolicy;
+
+ public function __construct(
+ $name,
+ $question,
+ $description,
+ $isEnabled,
+ $coverage,
+ $link,
+ $privacyPolicy
+ ) {
+ parent::__construct( $name, $question, $description,
$isEnabled, $coverage );
+
+ $this->link = $link;
+ $this->privacyPolicy = $privacyPolicy;
+ }
+
+ public function getMessages() {
+ return array_merge( parent::getMessages(), array(
$this->privacyPolicy ) );
+ }
+
+ public function toArray() {
+ return parent::toArray() + array(
+ 'type' => 'external',
+ 'link' => $this->link,
+ 'privacyPolicy' => $this->privacyPolicy,
+ );
+ }
+}
diff --git a/includes/InternalSurvey.php b/includes/InternalSurvey.php
new file mode 100644
index 0000000..5ce74d6
--- /dev/null
+++ b/includes/InternalSurvey.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace QuickSurveys;
+
+class InternalSurvey extends Survey
+{
+ /**
+ * @var array A map of internal key, e.g. "positive", to a i18n message
key
+ */
+ private $answers;
+
+ public function __construct(
+ $name,
+ $question,
+ $description,
+ $isEnabled,
+ $coverage,
+ array $answers
+ ) {
+ parent::__construct( $name, $question, $description,
$isEnabled, $coverage );
+
+ $this->answers = $answers;
+ }
+
+ public function getMessages() {
+ return array_merge( parent::getMessages(), array_values(
$this->answers ) );
+ }
+
+ public function toArray() {
+ return parent::toArray() + array(
+ 'type' => 'internal',
+ 'answers' => $this->answers,
+ );
+ }
+}
diff --git a/includes/QuickSurveys.hooks.php b/includes/QuickSurveys.hooks.php
index 190219c..dbde460 100644
--- a/includes/QuickSurveys.hooks.php
+++ b/includes/QuickSurveys.hooks.php
@@ -20,7 +20,11 @@
* @return boolean
*/
public static function onResourceLoaderGetConfigVars( &$vars ) {
- $vars['wgEnabledQuickSurveys'] = self::getEnabledSurveys();
+ $surveys = self::getEnabledSurveys();
+ $vars['wgEnabledQuickSurveys']= array_map( function ( Survey
$survey ) {
+ return $survey->toArray();
+ }, $surveys );
+
return true;
}
@@ -49,34 +53,17 @@
*/
public static function onResourceLoaderRegisterModules( ResourceLoader
&$resourceLoader ) {
$enabledSurveys = self::getEnabledSurveys();
- // Register enabled surveys as their own modules
+
foreach ( $enabledSurveys as $survey ) {
- $messages = array();
+ $moduleName = $survey->getResourceLoaderModuleName();
+ $module = array(
+ $moduleName => array(
+ 'messages' => $survey->getMessages(),
+ 'targets' => array( 'desktop', 'mobile'
),
+ ),
+ );
- // All surveys have a question and description
- $messages[] = $survey['question'];
- $messages[] = $survey['description'];
-
- // Add messages that are specific the survey type
- if ( $survey['type'] === 'internal' ) {
- $messages[] = $survey['answers']['positive'];
- $messages[] = $survey['answers']['neutral'];
- $messages[] = $survey['answers']['negative'];
-
- } elseif ( $survey['type'] === 'external' ) {
- $messages[] = $survey['link'];
- // camelCase because the key will be a property
of a JavaScript object.
- // Also this is not the key of the i18n
message, it's the key that denotes the key of
- // the i18n message. Basically, key of a PHP
array.
- $messages[] = $survey['privacyPolicy'];
- }
-
- $surveyModule = array( $survey['module'] => array(
- 'messages' => $messages,
- 'targets' => array( 'desktop', 'mobile' ),
- ) );
-
- $resourceLoader->register( $surveyModule );
+ $resourceLoader->register( $module );
}
return true;
@@ -89,28 +76,15 @@
*/
private static function getEnabledSurveys() {
$config = ConfigFactory::getDefaultInstance()->makeConfig(
'quicksurveys' );
- // Get configured surveys
$configuredSurveys = $config->has( 'QuickSurveysConfig' )
? $config->get( 'QuickSurveysConfig' )
: array();
- $enabledQuickSurveys = array();
- // Make enabled surveys available to the browser
- foreach ( $configuredSurveys as $survey ) {
- if ( $survey['enabled'] === true ) {
- $survey['module'] = self::getSurveyModuleName(
$survey );
- $enabledQuickSurveys[] = $survey;
- }
- }
- return $enabledQuickSurveys;
- }
+ $surveys = array_map( '\\QuickSurveys\\SurveyFactory::factory',
$configuredSurveys );
+ $enabledSurveys = array_filter( $surveys, function ( Survey
$survey ) {
+ return $survey->isEnabled();
+ } );
- /**
- * Returns the name of the specified survey's module
- *
- * @return string Survey's ResourceLoader module name
- */
- private static function getSurveyModuleName( $survey ) {
- return 'ext.quicksurveys.survey.' . str_replace( ' ', '.',
$survey['name'] );
+ return array_values( $enabledSurveys );
}
/**
@@ -132,4 +106,20 @@
return true;
}
+
+ /**
+ * UnitTestsList hook handler.
+ *
+ * Adds the path to the QuickSurveys PHPUnit tests to the set of enabled
+ * extension's test suites.
+ *
+ * @param array $paths The set of paths to other extension's PHPUnit
test
+ * suites
+ * @return bool Always true
+ */
+ public static function onUnitTestsList( array &$paths ) {
+ $paths[] = __DIR__ . '/../tests/phpunit';
+
+ return true;
+ }
}
diff --git a/includes/Survey.php b/includes/Survey.php
new file mode 100644
index 0000000..354fca7
--- /dev/null
+++ b/includes/Survey.php
@@ -0,0 +1,93 @@
+<?php
+
+namespace QuickSurveys;
+
+abstract class Survey
+{
+ /**
+ * @var string The friendly name of the survey
+ */
+ private $name;
+
+ /**
+ * @var string The question that the survey is posing to the user
+ */
+ private $question;
+
+ /**
+ * @var string A user-friendly description of, or introduction to, the
question
+ */
+ private $description;
+
+ /**
+ * @var bool Whether the survey is enabled
+ */
+ private $isEnabled;
+
+ /**
+ * @var float The percentage of users that will see the survey
expressed as a fraction
+ */
+ private $coverage;
+
+ public function __construct(
+ $name,
+ $question,
+ $description,
+ $isEnabled,
+ $coverage
+ ) {
+ $this->name = $name;
+ $this->question = $question;
+ $this->description = $description;
+ $this->isEnabled = $isEnabled;
+ $this->coverage = $coverage;
+ }
+
+ // --------
+ /**
+ * Returns the name of the ResourceLoader module
+ *
+ * @return string
+ */
+ public function getResourceLoaderModuleName() {
+ return 'ext.quicksurveys.survey.' . str_replace( ' ', '.',
$this->name );
+ }
+
+ /**
+ * Gets the list of i18n message keys that the survey uses
+ *
+ * @return [string]
+ */
+ public function getMessages() {
+ return array(
+ $this->question,
+
+ // FIXME: Should description be optional?
+ $this->description,
+ );
+ }
+ // --------
+
+ /**
+ * Returns the JSON-encodable, minimal representation of the survey
+ *
+ * @return array
+ */
+ public function toArray() {
+ return array(
+ 'question' => $this->question,
+ 'description' => $this->description,
+ 'module' => $this->getResourceLoaderModuleName(),
+ 'coverage' => $this->coverage,
+ );
+ }
+
+ /**
+ * Gets whether the survey is enabled
+ *
+ * @return bool
+ */
+ public function isEnabled() {
+ return $this->isEnabled;
+ }
+}
diff --git a/includes/SurveyFactory.php b/includes/SurveyFactory.php
new file mode 100644
index 0000000..a7d0ac9
--- /dev/null
+++ b/includes/SurveyFactory.php
@@ -0,0 +1,105 @@
+<?php
+
+namespace QuickSurveys;
+
+use InvalidArgumentException;
+
+class SurveyFactory
+{
+ /**
+ * Creates an instance of either the InternalSurvey or ExternalSurvey
class
+ * given a specification.
+ *
+ * An exception is thrown if any of the following conditions aren't met:
+ *
+ * <ul>
+ * <li>A survey must have a question</li>
+ * <li>A survey must have a description</li>
+ * <li>A survey's type must be either "internal" or "external"</li>
+ * <li>A survey must have a coverage</li>
+ * <li>An internal survey must have a set of questions</li>
+ * <li>An external survey must have a link</li>
+ * <li>An external survey must have a privacy policy</li>
+ * </ul>
+ *
+ * @throws InvalidArgumentException If the configuration is invalid
+ * @return Survey
+ */
+ public static function factory( array $spec ) {
+ $name = $spec['name'];
+
+ if ( !isset( $spec['question'] ) ) {
+ throw new InvalidArgumentException( "The \"{$name}\"
survey doesn't have a question." );
+ }
+
+ if ( !isset( $spec['description'] ) ) {
+ throw new InvalidArgumentException( "The \"{$name}\"
survey doesn't have a description." );
+ }
+
+ if (
+ !isset( $spec['type'] )
+ || ( $spec['type'] !== 'internal' && $spec['type'] !==
'external' )
+ ) {
+ throw new InvalidArgumentException(
+ "The \"{$name}\" survey isn't marked as
internal or external."
+ );
+ }
+
+ if ( !isset( $spec['coverage'] ) ) {
+ throw new InvalidArgumentException( "The \"{$name}\"
survey doesn't have a coverage." );
+ }
+
+ if ( !isset( $spec['enabled' ] ) ) {
+ $spec['enabled'] = false;
+ }
+
+ if ( $spec['type'] === 'internal' ) {
+ return self::factoryInternal( $spec );
+ }
+
+ return self::factoryExternal( $spec );
+ }
+
+ private static function factoryExternal( $spec ) {
+ $name = $spec['name'];
+
+ if ( !isset( $spec['link'] ) ) {
+ throw new InvalidArgumentException( "The \"{$name}\"
external survey doesn't have a link." );
+ }
+
+ if ( !isset( $spec['privacyPolicy'] ) ) {
+ throw new InvalidArgumentException(
+ "The \"{$name}\" external survey doesn't have a
privacy policy."
+ );
+ }
+
+ return new ExternalSurvey(
+ $spec['name'],
+ $spec['question'],
+ $spec['description'],
+ $spec['enabled'],
+ $spec['coverage'],
+ $spec['link'],
+ $spec['privacyPolicy']
+ );
+ }
+
+ private static function factoryInternal( $spec ) {
+ $name = $spec['name'];
+
+ if ( !isset( $spec['answers'] ) ) {
+ throw new InvalidArgumentException(
+ "The \"{$name}\" internal survey doesn't have
any answers."
+ );
+ }
+
+ return new InternalSurvey(
+ $spec['name'],
+ $spec['question'],
+ $spec['description'],
+ $spec['enabled'],
+ $spec['coverage'],
+ $spec['answers']
+ );
+ }
+}
diff --git a/tests/browser/LocalSettings.php b/tests/browser/LocalSettings.php
index 98d3451..346a91e 100644
--- a/tests/browser/LocalSettings.php
+++ b/tests/browser/LocalSettings.php
@@ -11,7 +11,7 @@
),
"schema" => "QuickSurveysResponses",
"enabled" => true,
- "coverage" => "100",
+ "coverage" => 1,
"description" => "yo",
"platform" => array(
"desktop" => array( "stable" ),
diff --git a/tests/phpunit/SurveyFactoryTest.php
b/tests/phpunit/SurveyFactoryTest.php
new file mode 100644
index 0000000..9838076
--- /dev/null
+++ b/tests/phpunit/SurveyFactoryTest.php
@@ -0,0 +1,181 @@
+<?php
+
+namespace Tests\QuickSurveys;
+
+use PHPUnit_Framework_TestCase;
+use QuickSurveys\SurveyFactory;
+use QuickSurveys\InternalSurvey;
+use QuickSurveys\ExternalSurvey;
+
+class SurveyFactoryTest extends PHPUnit_Framework_TestCase
+{
+ /**
+ * @expectedException InvalidArgumentException
+ * @expectedExceptionMessage The "test" survey doesn't have a question.
+ */
+ public function testItShouldThrowWhenThereIsNoQuestion() {
+ SurveyFactory::factory( array(
+ 'name' => 'test',
+ ) );
+ }
+
+
+ /**
+ * @expectedException InvalidArgumentException
+ * @expectedExceptionMessage The "test" survey doesn't have a
description.
+ */
+ public function testItShouldThrowWhenThereIsNoDescription() {
+ SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'question' => 'Do you like writing unit tests?',
+ ) );
+ }
+
+ /**
+ * @expectedException InvalidArgumentException
+ * @expectedExceptionMessage The "test" survey isn't marked as internal
or external.
+ */
+ public function testItShouldThrowWhenThereIsNoType() {
+ SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'question' => 'Do you like writing unit tests?',
+ 'description' => 'A survey for (potential) developers
on the QuickSurveys project.',
+ ) );
+ }
+
+ /**
+ * @expectedException InvalidArgumentException
+ * @expectedExceptionMessage The "test" external survey doesn't have a
link.
+ */
+ public function testItShouldThrowWhenThereIsNoLink() {
+ SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'type' => 'external',
+ 'question' => 'Do you like writing unit tests?',
+ 'description' => 'A survey for (potential) developers
on the QuickSurveys project.',
+ 'coverage' => 1,
+ ) );
+ }
+
+ /**
+ * @expectedException InvalidArgumentException
+ * @expectedExceptionMessage The "test" external survey doesn't have a
privacy policy.
+ */
+ public function testItShouldThrowWhenThereIsNoPrivacyPolicy() {
+ SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'type' => 'external',
+ 'question' => 'Do you like writing unit tests?',
+ 'description' => 'A survey for (potential) developers
on the QuickSurveys project.',
+ 'coverage' => 1,
+ 'link' => '//example.org/test-external-survey',
+ ) );
+ }
+
+ public function testItShouldFactoryAnExternalSurvey() {
+ $expected = new ExternalSurvey(
+ 'test',
+ 'Do you like writing unit tests?',
+ 'A survey for (potential) developers of the
QuickSurveys extension.',
+ true,
+ 1,
+ '//example.org/test-external-survey',
+ 'ext-quicksurveys-test-external-survey-privacy-policy'
+ );
+
+ $actual = SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'type' => 'external',
+ 'question' => 'Do you like writing unit tests?',
+ 'description' => 'A survey for (potential) developers
of the QuickSurveys extension.',
+ 'enabled' => true,
+ 'coverage' => 1,
+ 'link' => '//example.org/test-external-survey',
+ 'privacyPolicy' =>
'ext-quicksurveys-test-external-survey-privacy-policy',
+ ) );
+
+ $this->assertEquals( $actual, $expected );
+ }
+
+ /**
+ * @expectedException InvalidArgumentException
+ * @expectedExceptionMessage The "test" internal survey doesn't have
any answers.
+ */
+ public function testItShouldThrowWhenThereAreNoAnswers() {
+ SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'type' => 'internal',
+ 'question' => 'Do you like writing unit tests?',
+ 'description' => 'A survey for (potential) developers
on the QuickSurveys project.',
+ 'coverage' => 1,
+ ) );
+ }
+
+ public function testItShouldFactoryAnInternalSurvey() {
+ $expected = new InternalSurvey(
+ 'test',
+ 'Do you like writing unit tests?',
+ 'A survey for (potential) developers of the
QuickSurveys extension.',
+ true,
+ 1,
+ array(
+ 'positive' =>
'ext-quicksurveys-test-internal-survey-positive',
+ )
+ );
+
+ $actual = SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'type' => 'internal',
+ 'question' => 'Do you like writing unit tests?',
+ 'description' => 'A survey for (potential) developers
of the QuickSurveys extension.',
+ 'enabled' => true,
+ 'coverage' => 1,
+ 'answers' => array(
+ 'positive' =>
'ext-quicksurveys-test-internal-survey-positive',
+ ),
+ ) );
+
+ $this->assertEquals( $actual, $expected );
+ }
+
+ /**
+ * @expectedException InvalidArgumentException
+ * @expectedExceptionMessage The "test" survey isn't marked as internal
or external.
+ */
+ public function testItShouldThrowIfTheTypeIsNotRecognized() {
+ SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'type' => 'ixternal',
+ 'question' => 'Do you like writing unit tests?',
+ 'description' => 'A survey for (potential) developers
of the QuickSurveys extension.',
+ ) );
+ }
+
+ public function testItShouldMarkTheSurveyAsDisabledByDefault() {
+ $survey = SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'type' => 'internal',
+ 'question' => 'Do you like writing unit tests?',
+ 'description' => 'A survey for (potential) developers
of the QuickSurveys extension.',
+ 'coverage' => 1,
+ 'answers' => array(
+ 'positive' =>
'ext-quicksurveys-test-internal-survey-positive',
+ ),
+ ) );
+
+ $this->assertFalse( $survey->isEnabled() );
+ }
+
+ /**
+ * @expectedException InvalidArgumentException
+ * @expectedExceptionMessage The "test" survey doesn't have a coverage.
+ */
+ public function testItShouldThrowWhenThereIsNoCoverage() {
+ SurveyFactory::factory( array(
+ 'name' => 'test',
+ 'type' => 'internal',
+ 'question' => 'Do you like writing unit tests?',
+ 'description' => 'A survey for (potential) developers
of the QuickSurveys extension.',
+ ) );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/234267
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I90b3ce5c7fb0ef06802cd9e0abe429e67bc742cc
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/QuickSurveys
Gerrit-Branch: dev
Gerrit-Owner: Phuedx <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Phuedx <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
