jenkins-bot has submitted this change and it was merged.
Change subject: Remebmer appeal, make RapidHTML use configurable default
......................................................................
Remebmer appeal, make RapidHTML use configurable default
Treat appeal like a normal data token and store it in session.
Set and sanitize default in DonationData for use in both form types.
TODO: record it someplace so we can actuall a/b test appeals
Bug: T114127
Change-Id: I378a23284ee62e8d546e79a37d8727b1a1fb7caf
---
M gateway_common/DonationData.php
M gateway_common/MessageUtils.php
M gateway_common/gateway.adapter.php
M gateway_forms/Form.php
M gateway_forms/Mustache.php
M gateway_forms/RapidHtml.php
M tests/DonationDataTest.php
M tests/MustacheFormTest.php
8 files changed, 44 insertions(+), 31 deletions(-)
Approvals:
Cdentinger: Looks good to me, approved
jenkins-bot: Verified
diff --git a/gateway_common/DonationData.php b/gateway_common/DonationData.php
index 5a9d943..126a4b6 100644
--- a/gateway_common/DonationData.php
+++ b/gateway_common/DonationData.php
@@ -73,6 +73,7 @@
'amount',
'amountGiven',
'amountOther',
+ 'appeal',
'email',
'emailAdd',
'fname',
@@ -332,6 +333,7 @@
$this->renameCardType();
$this->setEmail();
$this->setCardNum();
+ $this->setAppeal();
if ( $updateCtRequired ) {
$this->saveContributionTrackingData();
@@ -772,6 +774,18 @@
}
/**
+ * Set default appeal if unset, sanitize either way.
+ */
+ protected function setAppeal() {
+ if ( $this->isSomething( 'appeal' ) ) {
+ $appeal = $this->getVal( 'appeal' );
+ } else {
+ $appeal = $this->gateway->getGlobal( 'DefaultAppeal' );
+ }
+ $this->setVal( 'appeal', MessageUtils::makeSafe( $appeal ) );
+ }
+
+ /**
* Clean array of tracking data to contain valid fields
*
* Compares tracking data array to list of valid tracking fields and
diff --git a/gateway_common/MessageUtils.php b/gateway_common/MessageUtils.php
index 0c20f06..77c9fb5 100644
--- a/gateway_common/MessageUtils.php
+++ b/gateway_common/MessageUtils.php
@@ -78,4 +78,19 @@
public static function getCountrySpecificMessage( $key, $country,
$language, $params = array() ) {
return self::languageSpecificFallback( $language, array( $key .
'-' . strtolower( $country ), $key ), $params );
}
+
+ /**
+ * This function limits the possible characters passed as template keys
and
+ * values to letters, numbers, hyphens and underscores. The function
also
+ * performs standard escaping of the passed values.
+ *
+ * @param string $string The unsafe string to escape and check for
invalid characters
+ * @return string Sanitized version of input
+ */
+ public static function makeSafe( $string ) {
+ $stripped = preg_replace( '/[^-_\w]/', '', $string );
+
+ // theoretically this is overkill, but better safe than sorry
+ return wfEscapeWikiText( htmlspecialchars( $stripped ) );
+ }
}
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 7a78c2d..7b16c0d 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -3106,6 +3106,7 @@
$_SESSION['Donor'] = array ( );
$donordata = DonationData::getStompMessageFields();
$donordata[] = 'order_id';
+ $donordata[] = 'appeal';
foreach ( $donordata as $item ) {
$_SESSION['Donor'][$item] =
$this->getData_Unstaged_Escaped( $item );
diff --git a/gateway_forms/Form.php b/gateway_forms/Form.php
index 10d2bc8..aecb07e 100644
--- a/gateway_forms/Form.php
+++ b/gateway_forms/Form.php
@@ -143,21 +143,6 @@
return $this->gateway->getData_Unstaged_Escaped( $key );
}
- /**
- * This function limits the possible characters passed as template keys
and
- * values to letters, numbers, hyphens and underscores. The function
also
- * performs standard escaping of the passed values.
- *
- * @param string $string The unsafe string to escape and check for
invalid characters
- * @return string Sanitized version of input
- */
- function make_safe( $string ) {
- $stripped = preg_replace( '/[^-_\w]/', '', $string );
-
- // theoretically this is overkill, but better safe than sorry
- return wfEscapeWikiText( htmlspecialchars( $stripped ) );
- }
-
public function getResources() {
return array();
}
diff --git a/gateway_forms/Mustache.php b/gateway_forms/Mustache.php
index 83cbe81..37e3f8a 100644
--- a/gateway_forms/Mustache.php
+++ b/gateway_forms/Mustache.php
@@ -87,9 +87,7 @@
$data['has_no_script_redirect'] = isset( $redirect ); // grr
$appealWikiTemplate = $this->gateway->getGlobal(
'AppealWikiTemplate' );
- $defaultAppeal = $this->gateway->getGlobal( 'DefaultAppeal' );
- $appeal = $this->make_safe( $request->getText( 'appeal',
$defaultAppeal ) );
- $appealWikiTemplate = str_replace( '$appeal', $appeal,
$appealWikiTemplate );
+ $appealWikiTemplate = str_replace( '$appeal', $data['appeal'],
$appealWikiTemplate );
$appealWikiTemplate = str_replace( '$language',
$data['language'], $appealWikiTemplate );
$data['appeal_text'] = $output->parse( '{{' .
$appealWikiTemplate . '}}' );
diff --git a/gateway_forms/RapidHtml.php b/gateway_forms/RapidHtml.php
index bcdba59..c7de64f 100644
--- a/gateway_forms/RapidHtml.php
+++ b/gateway_forms/RapidHtml.php
@@ -33,6 +33,7 @@
protected $data_tokens = array(
'@amount', // => $amount,
'@amountOther', // => $wgRequest->getText( 'amountOther' ),
+ '@appeal',
'@emailAdd', //'email' => $wgRequest->getText( 'emailAdd' ),
'@fname', // => $wgRequest->getText( 'fname' ),
'@lname', // => $wgRequest->getText( 'lname' ),
@@ -181,14 +182,6 @@
* This is a hack and should be replaced with something more
performant.
*/
$form = $html;
-
- // handle the appeal and appeal header
- // TODO: determine and set variables for the default templates
- $appeal_title_name = $this->make_safe( $wgRequest->getText(
'appeal-title', 'Appeal-title-default' ) );
- $appeal_name = $this->make_safe( $wgRequest->getText( 'appeal',
'Appeal-default' ) );
-
- $form = str_replace( "@appeal-title", $appeal_title_name, $form
);
- $form = str_replace( "@appeal", $appeal_name, $form );
// handle form action
$form = str_replace( "@action", $this->getNoCacheAction(),
$form );
@@ -339,7 +332,7 @@
$param = $this->getEscapedValue( $matches[2][$i] );
if ( $param && !is_array( $param ) ) {
# get the value of the element and super-escape
- $var = $this->make_safe( $param, 'default' );
+ $var = MessageUtils::makeSafe( $param,
'default' );
}
# oh, and we only allow with the extension .html
diff --git a/tests/DonationDataTest.php b/tests/DonationDataTest.php
index f8e43d3..7c686bb 100644
--- a/tests/DonationDataTest.php
+++ b/tests/DonationDataTest.php
@@ -40,6 +40,7 @@
$this->testData = array(
'amount' => '128.00',
+ 'appeal' => 'JimmyQuote',
'email' => '[email protected]',
'fname' => 'Testocres',
'lname' => 'McTestingyou',
@@ -88,6 +89,7 @@
$returned = $ddObj->getDataEscaped();
$expected = array( 'posted' => '',
'amount' => '0.00',
+ 'appeal' => 'JimmyQuote',
'country' => 'XX',
'payment_method' => '',
'referrer' => '',
@@ -112,6 +114,7 @@
$expected = array (
'amount' => '35.00',
+ 'appeal' => 'JimmyQuote',
'email' => '[email protected]',
'fname' => 'Tester',
'lname' => 'Testington',
@@ -167,6 +170,7 @@
$expected = array (
'amount' => '35.00',
+ 'appeal' => 'JimmyQuote',
'email' => '[email protected]',
'fname' => 'Tester',
'lname' => 'Testington',
diff --git a/tests/MustacheFormTest.php b/tests/MustacheFormTest.php
index 5d50e92..337a492 100644
--- a/tests/MustacheFormTest.php
+++ b/tests/MustacheFormTest.php
@@ -27,6 +27,7 @@
protected $outputPage;
public function setUp() {
+ $this->resetAllEnv();
$this->adapter = new TestingGenericAdapter();
$this->adapter->addRequestData( array(
'amount' => '12',
@@ -125,14 +126,14 @@
'wgDonationInterfaceAppealWikiTemplate' =>
'JimmySezPleeeeeze/$appeal/$language',
) );
- RequestContext::getMain()->getRequest()->setVal( 'appeal',
'differentAppeal' );
+ $this->adapter->addRequestData( array( 'appeal' =>
'differentAppeal' ) );
$this->outputPage->expects( $this->once() )
->method( 'parse' )
->with( $this->equalTo(
'{{JimmySezPleeeeeze/differentAppeal/en}}' ) );
$this->form = new Gateway_Form_Mustache( $this->adapter );
- $html = $this->form->getForm();
+ $this->form->getForm();
}
/**
@@ -145,14 +146,16 @@
'wgDonationInterfaceAppealWikiTemplate' =>
'JimmySezPleeeeeze/$appeal/$language',
) );
- RequestContext::getMain()->getRequest()->setVal( 'appeal',
'}}<script>alert("all your base are belong to us");</script>{{' );
+ $this->adapter->addRequestData( array(
+ 'appeal' => '}}<script>alert("all your base are belong
to us");</script>{{',
+ ) );
$this->outputPage->expects( $this->once() )
->method( 'parse' )
->with( $this->equalTo(
'{{JimmySezPleeeeeze/scriptalertallyourbasearebelongtousscript/en}}' ) );
$this->form = new Gateway_Form_Mustache( $this->adapter );
- $html = $this->form->getForm();
+ $this->form->getForm();
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/246894
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I378a23284ee62e8d546e79a37d8727b1a1fb7caf
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
Gerrit-Reviewer: AndyRussG <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
Gerrit-Reviewer: Cdentinger <[email protected]>
Gerrit-Reviewer: Ssmith <[email protected]>
Gerrit-Reviewer: XenoRyet <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
