Awight has uploaded a new change for review.
https://gerrit.wikimedia.org/r/150884
Change subject: (FR #1845) Assert no unconsumed tokens while rendering
thank-yous
......................................................................
(FR #1845) Assert no unconsumed tokens while rendering thank-yous
This improves the check by intercepting content before it's written to
disk, making it much harder to accidentally commit bad templates.
Deployment:
* drush rr
Change-Id: Ic269f66a8623e63411c7dc612762c2018f8f6c65
---
D sites/all/modules/thank_you/FindUnconsumedTokens.php
M sites/all/modules/thank_you/find_unconsumed_tokens.drush.inc
A sites/all/modules/thank_you/generators/FindUnconsumedTokens.php
M sites/all/modules/thank_you/generators/RenderTranslatedPage.php
D sites/all/modules/thank_you/generators/ThankYou2012.php
M sites/all/modules/thank_you/make_thank_you.drush.inc
M sites/all/modules/thank_you/thank_you.info
M sites/all/modules/wmf_communication/Templating.php
8 files changed, 130 insertions(+), 104 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/crm
refs/changes/84/150884/1
diff --git a/sites/all/modules/thank_you/FindUnconsumedTokens.php
b/sites/all/modules/thank_you/FindUnconsumedTokens.php
deleted file mode 100644
index 3f3f8e8..0000000
--- a/sites/all/modules/thank_you/FindUnconsumedTokens.php
+++ /dev/null
@@ -1,55 +0,0 @@
-<?php
-
-class FindUnconsumedTokens {
- static function searchTokens() {
- watchdog( 'thank_you', "Searching for unconsumed tokens in thank-you
pages..." );
- $ret = 0;
- $languages = thank_you_get_languages();
- foreach ( $languages as $lang ) {
- $err = FindUnconsumedTokens::searchToken( $lang );
- $ret = $err || $ret;
- }
- if ( $ret ) {
- throw new Exception( "Bad news, you have errors." );
- }
- }
-
- static function searchToken( $lang ) {
- // Turn on all the lights
- $params = array(
- // FIXME: name should be run through both branches
- 'first_name' => 'fix',
- 'last_name' => 'me',
- 'recurring' => true,
- 'RecurringRestarted' => true,
-
- 'receive_date' => time(),
-
- 'locale' => $lang,
- );
- $buf = thank_you_render( $params );
-
- $bad_punctuation_re = '/
- # Square brackets, but not a numbered footnote
- \[ (?: (?! \d+ \] ) . )+ \]
- |
- # Other exotic punctuation that indicates a failed token
- (
- [${}]+
- |
- # This is a hash, but is not part of an URL fragment?
- (?: (?! https?:\/\/ ) . )+ \K [#]
- )
- # Token name if we are lucky
- (?: [-a-zA-Z0-9_]+ )?
- /x';
- if ( $count = preg_match_all( $bad_punctuation_re, $buf, $matches ) ) {
- $bad_punc = implode( ', ', $matches[0] );
- watchdog( 'thank_you',
- "Found {$count} likely tokens [{$bad_punc}] in thank-you
translation [{$lang}].",
- null, WATCHDOG_ERROR
- );
- return -1;
- }
- }
-}
diff --git a/sites/all/modules/thank_you/find_unconsumed_tokens.drush.inc
b/sites/all/modules/thank_you/find_unconsumed_tokens.drush.inc
index 82e9783..fdf3c12 100644
--- a/sites/all/modules/thank_you/find_unconsumed_tokens.drush.inc
+++ b/sites/all/modules/thank_you/find_unconsumed_tokens.drush.inc
@@ -1,5 +1,7 @@
<?php
+use thank_you\generators\FindUnconsumedTokens;
+
function find_unconsumed_tokens_drush_command() {
$items = array();
@@ -11,5 +13,5 @@
}
function drush_find_unconsumed_tokens() {
- FindUnconsumedTokens::searchTokens();
+ FindUnconsumedTokens::findAllTokens();
}
diff --git a/sites/all/modules/thank_you/generators/FindUnconsumedTokens.php
b/sites/all/modules/thank_you/generators/FindUnconsumedTokens.php
new file mode 100644
index 0000000..3c40110
--- /dev/null
+++ b/sites/all/modules/thank_you/generators/FindUnconsumedTokens.php
@@ -0,0 +1,95 @@
+<?php
+namespace thank_you\generators;
+
+use wmf_communication\Templating;
+
+class FindUnconsumedTokens {
+ /**
+ * Search all locally translated languages for unconsumed tokens
+ * @throws UnconsumedTokenException
+ */
+ static function findAllTokens() {
+ watchdog( 'thank_you',
+ "Searching for unconsumed tokens in thank-you pages...", NULL,
WATCHDOG_INFO );
+ $failure = false;
+ $languages = thank_you_get_languages();
+ foreach ( $languages as $lang ) {
+ try {
+ $err = FindUnconsumedTokens::findTokensInLanguage( $lang );
+ } catch ( UnconsumedTokenException $ex ) {
+ watchdog( 'thank_you', $ex->getMessage(), null, WATCHDOG_ERROR
);
+ $failure = true;
+ }
+ }
+ if ( $failure ) {
+ throw new UnconsumedTokenException( "Bad news, see errors above."
);
+ }
+ }
+
+ static protected function getRandomTemplateParams( $locale ) {
+ // Turn on all the lights
+ $params = array(
+ // FIXME: name should be run through both branches
+ 'first_name' => 'fix',
+ 'last_name' => 'me',
+ 'recurring' => true,
+ 'RecurringRestarted' => true,
+
+ 'receive_date' => time(),
+
+ 'locale' => $locale,
+ );
+ return $params;
+ }
+
+ // TODO: refactor out of this class
+ static function renderLanguage( $lang ) {
+ $params = FindUnconsumedTokens::getRandomTemplateParams( $lang );
+ $buf = thank_you_render( $params );
+ return $buf;
+ }
+
+ /**
+ * @throws UnconsumedTokenException
+ */
+ static function findTokensInLanguage( $lang ) {
+ $buf = FindUnconsumedTokens::renderLanguage( $lang );
+ FindUnconsumedTokens::findTokens( $buf );
+ }
+
+ /**
+ * @throws UnconsumedTokenException
+ */
+ static function renderAndFindTokens( $template, $locale ) {
+ $rendered = Templating::renderStringTemplate( $template,
FindUnconsumedTokens::getRandomTemplateParams( $locale ) );
+ FindUnconsumedTokens::findTokens( $buf );
+ }
+
+ /**
+ * Search a buffer for unconsumed tokens
+ * @throws UnconsumedTokenException
+ */
+ static function findTokens( $buf ) {
+ $bad_punctuation_re = '/
+ # Square brackets, but not a numbered footnote
+ \[ (?: (?! \d+ \] ) . )+ \]
+ |
+ # Other exotic punctuation that indicates a failed token
+ (
+ [${}]+
+ |
+ # This is a hash, but is not part of an URL fragment?
+ (?: (?! https?:\/\/ ) . )+ \K [#]
+ )
+ # Grab the token name for debugging, if available
+ (?: [-a-zA-Z0-9_]+ )?
+ /x';
+ if ( $count = preg_match_all( $bad_punctuation_re, $buf, $matches ) ) {
+ $bad_punc = implode( ', ', $matches[0] );
+ throw new UnconsumedTokenException(
+ "Found {$count} likely tokens [{$bad_punc}] in rendered
thank-you translation." );
+ }
+ }
+}
+
+class UnconsumedTokenException extends TranslationException {}
diff --git a/sites/all/modules/thank_you/generators/RenderTranslatedPage.php
b/sites/all/modules/thank_you/generators/RenderTranslatedPage.php
index c90ee72..318b0ee 100644
--- a/sites/all/modules/thank_you/generators/RenderTranslatedPage.php
+++ b/sites/all/modules/thank_you/generators/RenderTranslatedPage.php
@@ -24,16 +24,6 @@
protected $review_history;
- /**
- * @var string[] Translation states that this module will accept as
acceptable
- * to include in a composite message.
- */
- protected $valid_translation_states = array(
- 'ready',
- 'proofread',
- 'translated', // what exactly is this state?
- );
-
public function execute($wantedLangs = array()) {
watchdog(
'make-thank-you',
@@ -58,6 +48,9 @@
$page_content = str_replace( '|</p>|',
"</p>\n", $page_content );
$file = str_replace( '$1', $lang,
$this->proto_file );
+
+ // Assert no garbage
+ FindUnconsumedTokens::renderAndFindTokens(
$page_content, $lang );
if (file_put_contents( $file, $page_content )) {
watchdog( 'make-thank-you', "$lang --
Wrote translation into $file", null, WATCHDOG_INFO );
@@ -315,6 +308,7 @@
foreach( $this->substitutions as $k => $v ) {
$content = preg_replace( $k, $v, $content );
}
+
return $content;
}
}
diff --git a/sites/all/modules/thank_you/generators/ThankYou2012.php
b/sites/all/modules/thank_you/generators/ThankYou2012.php
deleted file mode 100644
index f0d8b2f..0000000
--- a/sites/all/modules/thank_you/generators/ThankYou2012.php
+++ /dev/null
@@ -1,34 +0,0 @@
-<?php namespace thank_you\generators;
-
-class ThankYou2012 extends RenderTranslatedPage {
- function __construct() {
- $this->title = 'Fundraising_2012/Translation/Thank_you_letter';
- $this->proto_file = __DIR__ .
'/../templates/html/thank_you.$1.html';
-
- $this->substitutions = array(
- '/\[given name\]/' => '{{ first_name }}',
- '/\[date\]/' => '{{ receive_date }}',
- '/\[amount\]/' => '{{ (currency ~ " " ~ amount) |
l10n_currency(locale) }}',
- '/\[contributionId\]/' => '{{ transaction_id }}',
-
- '/\[ifRecurring\]/' => '{% if recurring %}',
- '/\[endifRecurring\]/' => '{% endif %}',
-
- '/\[#twitter ((?:(?!\]).)*)\]/' => '<a
href="https://twitter.com/Wikipedia";>$1</a>',
- '/\[#identica ((?:(?!\]).)*)\]/' => '<a
href="https://identi.ca/wikipedia";>$1</a>',
- '/\[#google ((?:(?!\]).)*)\]/' => '<a
href="https://plus.google.com/+Wikipedia/posts";>$1</a>',
- '/\[#facebook ((?:(?!\]).)*)\]/' => '<a
href="https://www.facebook.com/wikipedia";>$1</a>',
- '/\[#blog ((?:(?!\]).)*)\]/' => '<a
href="https://blog.wikimedia.org";>$1</a>',
- // TODO: DO WE HAVE TRANSLATIONS FOR THE ANNUAL REPORT
- '/\[#annual ((?:(?!\]).)*)\]/' => '<a
href="https://meta.wikimedia.org/wiki/Special:MyLanguage/Wikimedia_Foundation/Annual_Report/2012-2013/Front";>$1</a>',
- // TODO: DO WE HAVE TRANSLATIONS FOR THE ANNUAL PLAN
- '/\[#plan ((?:(?!\]).)*)\]/' => '<a
href="http://upload.wikimedia.org/wikipedia/foundation/4/4f/2012-13_Wikimedia_Foundation_Plan_FINAL_FOR_WEBSITE.pdf";>$1</a>',
- // TODO: DO WE HAVE TRANSLATIONS FOR THE 5-YEAR,
STRATEGIC PLAN
- '/\[#strategic ((?:(?!\]).)*)\]/' => '<a
href="https://wikimediafoundation.org/wiki/Wikimedia_Movement_Strategic_Plan_Summary";>$1</a>',
- '/\[#shop ((?:(?!\]).)*)\]/' => '<a
href="https://shop.wikimedia.org";>$1</a>',
- '/\[#unsubscribe ((?:(?!\]).)*)\]/' => '<a href="{{
unsubscribe_link | raw }}">$1</a>',
- '/\[#recurringCancel ((?:(?!\]).)*)\]/' => '<a
href="https://wikimediafoundation.org/wiki/Special:LandingCheck?landing_page=Cancel_or_change_recurring_payments&basic=true&language={{
locale }}">$1</a>',
- '/\[#translate ((?:(?!\]).)*)\]/' => '<a
href="https://meta.wikimedia.org/w/index.php?title=Special:Translate&group=page-Fundraising+2012%2FTranslation%2FThank+you+letter";>$1</a>',
- );
- }
-}
diff --git a/sites/all/modules/thank_you/make_thank_you.drush.inc
b/sites/all/modules/thank_you/make_thank_you.drush.inc
index dfc0146..7e3370a 100644
--- a/sites/all/modules/thank_you/make_thank_you.drush.inc
+++ b/sites/all/modules/thank_you/make_thank_you.drush.inc
@@ -35,8 +35,6 @@
$generator = new $generators[$genname]();
$generator->execute($langs);
}
-
- FindUnconsumedTokens::searchTokens();
}
/**
diff --git a/sites/all/modules/thank_you/thank_you.info
b/sites/all/modules/thank_you/thank_you.info
index c0d8ca2..1ed382e 100644
--- a/sites/all/modules/thank_you/thank_you.info
+++ b/sites/all/modules/thank_you/thank_you.info
@@ -6,8 +6,7 @@
dependencies[] = wmf_communication
package = Wikimedia
configure = admin/config/thank_you
-files[] = FindUnconsumedTokens.php
files[] = Html2Text.php
+files[] = generators/FindUnconsumedTokens.php
files[] = generators/RenderTranslatedPage.php
-files[] = generators/ThankYou2012.php
files[] = generators/ThankYou.php
diff --git a/sites/all/modules/wmf_communication/Templating.php
b/sites/all/modules/wmf_communication/Templating.php
index 27f6fad..9dbf10d 100644
--- a/sites/all/modules/wmf_communication/Templating.php
+++ b/sites/all/modules/wmf_communication/Templating.php
@@ -2,6 +2,12 @@
use \Exception;
+// FIXME: encapsulate here, not in wmf_common
+use \Twig_Environment;
+use \Twig_Loader_String;
+
+use \TwigLocalization;
+
/**
* Single-use template.
*/
@@ -132,4 +138,25 @@
}
return null;
}
+
+ /**
+ * Evaluate a template passed as string literal
+ *
+ * TODO: clean up interface
+ */
+ static function renderStringTemplate( $template, $params ) {
+ // TODO: autoload instead
+ if ( !class_exists( 'Twig_Loader_String' ) ) {
+ module_load_include( 'inc', 'wmf_common', 'twig' );
+
+ // FIXME: throwaway call to initialize
+ wmf_common_get_twig( __DIR__ );
+ }
+
+ $loader = new Twig_Loader_String();
+ $twig = new Twig_Environment( $loader );
+ $twig->addExtension( new TwigLocalization() );
+
+ return $twig->render( $template, $params );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/150884
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic269f66a8623e63411c7dc612762c2018f8f6c65
Gerrit-PatchSet: 1
Gerrit-Project: wikimedia/fundraising/crm
Gerrit-Branch: master
Gerrit-Owner: Awight <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
