From e2aca87dc8bba0fd1282cdbce12777d41ebf5883 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Tue, 10 Sep 2019 13:33:04 +0200 Subject: [PATCH 01/34] Add amos_subscription database table --- db/install.xml | 14 +++++++++++++- db/upgrade.php | 25 +++++++++++++++++++++++++ version.php | 2 +- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/db/install.xml b/db/install.xml index 9691551..4db058f 100644 --- a/db/install.xml +++ b/db/install.xml @@ -1,5 +1,5 @@ - @@ -206,5 +206,17 @@ + + + + + + + + + + + +
\ No newline at end of file diff --git a/db/upgrade.php b/db/upgrade.php index 9c50fe4..b3891ef 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -345,5 +345,30 @@ function xmldb_local_amos_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2019040901, 'local', 'amos'); } + if ($oldversion < 2019090900) { + + // Define table amos_subscription to be created. + $table = new xmldb_table('amos_subscription'); + + // Adding fields to table amos_subscription. + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); + $table->add_field('userid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + $table->add_field('component', XMLDB_TYPE_CHAR, '100', null, XMLDB_NOTNULL, null, null); + $table->add_field('lang', XMLDB_TYPE_CHAR, '20', null, XMLDB_NOTNULL, null, null); + + // Adding keys to table amos_subscription. + $table->add_key('primary', XMLDB_KEY_PRIMARY, ['id']); + $table->add_key('fk_user', XMLDB_KEY_FOREIGN, ['userid'], 'user', ['id']); + + // Conditionally launch create table for amos_subscription. + if (!$dbman->table_exists($table)) { + $dbman->create_table($table); + } + + // Amos savepoint reached. + upgrade_plugin_savepoint(true, 2019090900, 'local', 'amos'); + } + + return $result; } diff --git a/version.php b/version.php index 0c037b6..df9e75c 100644 --- a/version.php +++ b/version.php @@ -26,7 +26,7 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'local_amos'; -$plugin->version = 2019040901; +$plugin->version = 2019090900; $plugin->release = '3.6.3'; $plugin->maturity = MATURITY_STABLE; $plugin->requires = 2018120300; From a634ca0502258cdb5f90348240658c161ae3bbf3 Mon Sep 17 00:00:00 2001 From: Jan Eberhardt Date: Tue, 10 Sep 2019 18:42:21 +0200 Subject: [PATCH 02/34] Started UI Bended original local_amos_filter class for our output Changes to be committed: modified: lang/en/local_amos.php modified: lib.php modified: locallib.php modified: renderer.php new file: subscription.php modified: yui/filter/filter.js --- lang/en/local_amos.php | 1 + lib.php | 1 + locallib.php | 10 ++++++ renderer.php | 67 +++++++++++++++++++++++++++++++------- subscription.php | 50 ++++++++++++++++++++++++++++ yui/filter/filter.js | 74 +++++++++++++++++++++++------------------- 6 files changed, 158 insertions(+), 45 deletions(-) create mode 100644 subscription.php diff --git a/lang/en/local_amos.php b/lang/en/local_amos.php index 96f95a0..fa6a2c0 100644 --- a/lang/en/local_amos.php +++ b/lang/en/local_amos.php @@ -451,6 +451,7 @@ $string['strings'] = 'Strings'; $string['submitting'] = 'Submitting a contribution'; $string['submitting_help'] = 'This will send translated strings to official language maintainers. They will be able to apply your work into their stage, review it and eventually commit. Please provide a message for them describing your work and why you would like to see your contribution included.'; +$string['subscription'] = 'Subscription'; $string['targetversion'] = 'Target version'; $string['timeline'] = 'timeline'; $string['translatortool'] = 'Translator'; diff --git a/lib.php b/lib.php index be26d3e..8669fb4 100644 --- a/lib.php +++ b/lib.php @@ -44,6 +44,7 @@ function local_amos_extend_navigation(global_navigation $navigation) { $amos->add(get_string('contributions', 'local_amos'), new moodle_url('/local/amos/contrib.php'), navigation_node::TYPE_CUSTOM, null, 'contributions'); } $amos->add(get_string('log', 'local_amos'), new moodle_url('/local/amos/log.php'), navigation_node::TYPE_CUSTOM, null, 'log'); + $amos->add(get_string('subscription', 'local_amos'), new moodle_url('/local/amos/subscription.php'), navigation_node::TYPE_CUSTOM, 'subscription'); $amos->add(get_string('creditstitleshort', 'local_amos'), new moodle_url('/local/amos/credits.php'), navigation_node::TYPE_CUSTOM, null, 'credits'); if (has_capability('local/amos:manage', context_system::instance())) { $admin = $amos->add(get_string('administration')); diff --git a/locallib.php b/locallib.php index d3e718e..71e896c 100644 --- a/locallib.php +++ b/locallib.php @@ -1767,3 +1767,13 @@ function local_amos_simplediff(array $old, array $new) { array_slice($new, $nmax, $maxlen), local_amos_simplediff(array_slice($old, $omax + $maxlen), array_slice($new, $nmax + $maxlen))); } + +class local_amos_subscription_filter extends local_amos_filter { + + public function __construct(moodle_url $handler) + { + parent::__construct($handler); + $this->fields = ['component', 'language']; + } + +} diff --git a/renderer.php b/renderer.php index a14ad75..e443102 100644 --- a/renderer.php +++ b/renderer.php @@ -30,21 +30,46 @@ */ class local_amos_renderer extends plugin_renderer_base { + protected function render_local_amos_subscription_filter(local_amos_subscription_filter $filter) { + // initialize + $output = ''; + $formattributes = [ + 'action' => $filter->handler, + 'method' => 'post', + 'id' => 'amosfilter_form' + ]; + + // start form + $output .= html_writer::start_div('filterwrapper'); + $output .= html_writer::start_tag('form', $formattributes); + $output .= html_writer::start_tag('fieldset', ['id' => 'amosfilter']); + + $alerts = []; + $defaults = (object) ['language' => [], 'component' => [], 'last' => true]; + $output .= $this->render_local_amos_filter_basic($defaults, $alerts, false, false); + + // close form + $output .= html_writer::end_tag('fieldset'); + $button = ''; + $output .= html_writer::div($button, 'form-actions'); + $output .= html_writer::end_tag('form'); + $output .= html_writer::end_div(); + + return $output; + } + /** - * Renders the filter form + * Renders basic filter of the filter form * - * @param local_amos_filter $filter + * @param object $filterdata * @return string */ - protected function render_local_amos_filter(local_amos_filter $filter) { + protected function render_local_amos_filter_basic($filterdata, &$alerts, $showappfilterbutton = true, $showselectionerror = true) { $output = ''; - $alerts = array(); - - $filterdata = $filter->get_data(); // language selector $current = $filterdata->language; - $someselected = false; + $someselected = !$showselectionerror; $options = mlang_tools::list_languages(false); foreach ($options as $langcode => $langname) { if (!$someselected and in_array($langcode, $current)) { @@ -68,7 +93,7 @@ protected function render_local_amos_filter(local_amos_filter $filter) { $output .= html_writer::start_tag('div', array('class' => 'controls')); $output .= html_writer::select($options, 'flng[]', $current, '', - array('id' => 'amosfilter_flng', 'multiple' => 'multiple', 'size' => 3)); + array('id' => 'amosfilter_flng', 'multiple' => 'multiple', 'size' => 3)); $radio = html_writer::empty_tag('input', array('type' => 'radio', 'name' => 'amosfilter_flng_actions', 'autocomplete' => 'off')); @@ -129,7 +154,7 @@ protected function render_local_amos_filter(local_amos_filter $filter) { asort($optionscontrib); $current = $filterdata->component; - $someselected = false; + $someselected = !$showselectionerror; $app_plugins = local_amos_app_plugins(); @@ -203,8 +228,10 @@ protected function render_local_amos_filter(local_amos_filter $filter) { $radio = html_writer::empty_tag('input', array('type' => 'radio', 'name' => 'amosfilter_fcmp_actions', 'autocomplete' => 'off')); $fcmpactions = html_writer::tag('label', $radio.' '.get_string('componentsstandard', 'local_amos'), array('id' => 'amosfilter_fcmp_actions_allstandard', 'class' => 'btn btn-light amosfilter_fcmp_actions_select')); - $radio = html_writer::empty_tag('input', array('type' => 'radio', 'name' => 'amosfilter_fcmp_actions', 'autocomplete' => 'off')); - $fcmpactions .= html_writer::tag('label', $radio.' '.get_string('componentsapp', 'local_amos'), array('id' => 'amosfilter_fcmp_actions_allapp', 'class' => 'btn btn-light amosfilter_fcmp_actions_select')); + if ($showappfilterbutton) { + $radio = html_writer::empty_tag('input', array('type' => 'radio', 'name' => 'amosfilter_fcmp_actions', 'autocomplete' => 'off')); + $fcmpactions .= html_writer::tag('label', $radio . ' ' . get_string('componentsapp', 'local_amos'), array('id' => 'amosfilter_fcmp_actions_allapp', 'class' => 'btn btn-light amosfilter_fcmp_actions_select')); + } $radio = html_writer::empty_tag('input', array('type' => 'radio', 'name' => 'amosfilter_fcmp_actions', 'autocomplete' => 'off')); $fcmpactions .= html_writer::tag('label', $radio.' '.get_string('componentsall', 'local_amos'), array('id' => 'amosfilter_fcmp_actions_all', 'class' => 'btn btn-light amosfilter_fcmp_actions_select')); @@ -226,6 +253,24 @@ protected function render_local_amos_filter(local_amos_filter $filter) { $output .= html_writer::end_tag('div'); // .controls $output .= html_writer::end_tag('div'); // .control-group + return $output; + } + + /** + * Renders the filter form + * + * @param local_amos_filter $filter + * @return string + */ + protected function render_local_amos_filter(local_amos_filter $filter) { + $output = ''; + $alerts = array(); + + $filterdata = $filter->get_data(); + + // render language and component filter + $output .= $this->render_local_amos_filter_basic($filterdata, $alerts); + // version checkboxes $current = $filterdata->version; $someselected = false; diff --git a/subscription.php b/subscription.php new file mode 100644 index 0000000..826ea6d --- /dev/null +++ b/subscription.php @@ -0,0 +1,50 @@ +. + +/** + * View your subscription and change settings + * + * @package local-amos + * @copyright 2019 Tobias Reischmann + * @copyright 2019 Martin Gauk + * @copyright 2019 Jan Eberhardt + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +require_once(dirname(dirname(dirname(__FILE__))).'/config.php'); +require_once(dirname(__FILE__).'/locallib.php'); +require_once(dirname(__FILE__).'/mlanglib.php'); + +require_login(SITEID, false); + +#$name = optional_param('name', null, PARAM_RAW); // stash name +$mode = optional_param('m', null, PARAM_ALPHA); + +$PAGE->set_pagelayout('standard'); +$PAGE->set_url('/local/amos/subscription.php'); +$PAGE->set_title('AMOS ' . get_string('subscription', 'local_amos')); +$PAGE->set_heading('AMOS ' . get_string('subscription', 'local_amos')); +$PAGE->requires->strings_for_js(array('processing', 'googletranslate'), 'local_amos'); +$PAGE->requires->yui_module('moodle-local_amos-filter', 'M.local_amos.init_filter', null, null, true); +#$PAGE->requires->yui_module('moodle-local_amos-translator', 'M.local_amos.init_translator', null, null, true); +$PAGE->requires->yui_module('moodle-local_amos-timeline', 'M.local_amos.init_timeline', null, null, true); + +$filter = new local_amos_subscription_filter($PAGE->url); +$output = $PAGE->get_renderer('local_amos'); +echo $output->header(); +echo $output->render($filter); +echo $output->footer(); \ No newline at end of file diff --git a/yui/filter/filter.js b/yui/filter/filter.js index e9d9eda..8756fe4 100644 --- a/yui/filter/filter.js +++ b/yui/filter/filter.js @@ -73,19 +73,21 @@ YUI.add('moodle-local_amos-filter', function(Y) { fcmp.all('tr.standard:not(.hidden) input').set('checked', true); }); var fcmpselectallapp = filter.one('#amosfilter_fcmp_actions_allapp'); - fcmpselectallapp.on('click', function(e) { - // Check all displayed standard components. - e.preventDefault(); - filter.all('.amosfilter_fcmp_actions_select').removeClass('active'); - fcmp.all('tr:not(.hidden) input').set('checked', false); - fcmp.all('tr.app:not(.hidden) input').set('checked', true); - filter.one('#fapp input').set('checked', true); - filter.one('#amosfilter_fmis_collapse').removeClass('collapse'); - filter.one('#fapp').ancestor().removeClass('collapse'); - filter.all('#amosfilter_fver .fver').set('disabled', true); - filter.one('#amosfilter_fver_versions').addClass('hidden'); - filter.one('#amosfilter_fver #flast').set('checked', true); - }); + if (fcmpselectallapp) { + fcmpselectallapp.on('click', function (e) { + // Check all displayed standard components. + e.preventDefault(); + filter.all('.amosfilter_fcmp_actions_select').removeClass('active'); + fcmp.all('tr:not(.hidden) input').set('checked', false); + fcmp.all('tr.app:not(.hidden) input').set('checked', true); + filter.one('#fapp input').set('checked', true); + filter.one('#amosfilter_fmis_collapse').removeClass('collapse'); + filter.one('#fapp').ancestor().removeClass('collapse'); + filter.all('#amosfilter_fver .fver').set('disabled', true); + filter.one('#amosfilter_fver_versions').addClass('hidden'); + filter.one('#amosfilter_fver #flast').set('checked', true); + }); + } var fcmpselectall = filter.one('#amosfilter_fcmp_actions_all'); fcmpselectall.on('click', function(e) { // Check all displayed components. @@ -106,17 +108,19 @@ YUI.add('moodle-local_amos-filter', function(Y) { }); var flast = filter.one('#flast'); - flast.on('change', function(e) { - e.preventDefault(); - filter.all('.fver').set('disabled', e.currentTarget.get('checked')); - if (e.currentTarget.get('checked')) { - filter.one('#amosfilter_fver_versions').addClass('hidden'); - filter.all('#amosfilter_fcmp').addClass('hiddenversions'); - } else { - filter.one('#amosfilter_fver_versions').removeClass('hidden'); - filter.all('#amosfilter_fcmp').removeClass('hiddenversions'); - } - }); + if (flast) { + flast.on('change', function (e) { + e.preventDefault(); + filter.all('.fver').set('disabled', e.currentTarget.get('checked')); + if (e.currentTarget.get('checked')) { + filter.one('#amosfilter_fver_versions').addClass('hidden'); + filter.all('#amosfilter_fcmp').addClass('hiddenversions'); + } else { + filter.one('#amosfilter_fver_versions').removeClass('hidden'); + filter.all('#amosfilter_fcmp').removeClass('hiddenversions'); + } + }); + } // search for components var fcmpsearch = filter.one('#amosfilter_fcmp_actions_search'); @@ -138,16 +142,18 @@ YUI.add('moodle-local_amos-filter', function(Y) { // make greylist related checkboxed mutally exclusive var fglo = filter.one('#amosfilter_fglo'); var fwog = filter.one('#amosfilter_fwog'); - fglo.on('change', function(e) { - if (fglo.get('checked')) { - fwog.set('checked', false); - } - }); - fwog.on('change', function(e) { - if (fwog.get('checked')) { - fglo.set('checked', false); - } - }); + if (fglo && fwog) { + fglo.on('change', function (e) { + if (fglo.get('checked')) { + fwog.set('checked', false); + } + }); + fwog.on('change', function (e) { + if (fwog.get('checked')) { + fglo.set('checked', false); + } + }); + } // display the "loading" icon after the filter button is pressed var fform = Y.one('#amosfilter_form'); From d62a6b49fe505e4ebb70dc3e2e6b6203d3af7c11 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Tue, 10 Sep 2019 14:47:08 +0200 Subject: [PATCH 03/34] Add cron task for sending notifications to subscribers --- classes/task/notify_subscribers.php | 54 +++++++++++++++++++++++++++++ db/tasks.php | 9 +++++ lang/en/local_amos.php | 1 + version.php | 2 +- 4 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 classes/task/notify_subscribers.php diff --git a/classes/task/notify_subscribers.php b/classes/task/notify_subscribers.php new file mode 100644 index 0000000..a907279 --- /dev/null +++ b/classes/task/notify_subscribers.php @@ -0,0 +1,54 @@ +. + +/** + * Provides the {@link \local_amos\task\notify_subscribers} class. + * + * @package local_amos + * @category task + * @copyright 2019 Tobias Reischmann + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace local_amos\task; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Sends notifications about changes of the language packs to the respective subsrcibers. + * + * @copyright 2019 Tobias Reischmann + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class notify_subscribers extends \core\task\scheduled_task { + + /** + * Return the task name. + * + * @return string + */ + public function get_name() { + return get_string('tasknotifysubscribers', 'local_amos'); + } + + /** + * Execute the task. + */ + public function execute() { + + } + +} diff --git a/db/tasks.php b/db/tasks.php index efa2316..f1dd498 100644 --- a/db/tasks.php +++ b/db/tasks.php @@ -44,4 +44,13 @@ 'month' => '*', 'dayofweek' => '*', ], + [ + 'classname' => '\local_amos\task\notify_subscribers', + 'blocking' => 0, + 'minute' => '0', + 'hour' => '0', + 'day' => '*', + 'month' => '*', + 'dayofweek' => '*', + ], ]; diff --git a/lang/en/local_amos.php b/lang/en/local_amos.php index fa6a2c0..bfbfb01 100644 --- a/lang/en/local_amos.php +++ b/lang/en/local_amos.php @@ -453,6 +453,7 @@ $string['submitting_help'] = 'This will send translated strings to official language maintainers. They will be able to apply your work into their stage, review it and eventually commit. Please provide a message for them describing your work and why you would like to see your contribution included.'; $string['subscription'] = 'Subscription'; $string['targetversion'] = 'Target version'; +$string['tasknotifysubscribers'] = 'Send Notifications to Subscribers'; $string['timeline'] = 'timeline'; $string['translatortool'] = 'Translator'; $string['translatortoolopen'] = 'Open AMOS translator'; diff --git a/version.php b/version.php index df9e75c..a56e39f 100644 --- a/version.php +++ b/version.php @@ -26,7 +26,7 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'local_amos'; -$plugin->version = 2019090900; +$plugin->version = 2019090901; $plugin->release = '3.6.3'; $plugin->maturity = MATURITY_STABLE; $plugin->requires = 2018120300; From 643baf60ed444039b102f31dc4c9230b91f1f50a Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Tue, 10 Sep 2019 18:43:22 +0200 Subject: [PATCH 04/34] Added initial version of notify subscriber task Including phpunit tests, mail templates and first sql queries for the changes of lang strings --- classes/task/notify_subscribers.php | 24 ++++ locallib.php | 37 ++++++ renderer.php | 12 ++ templates/subscription_notification.mustache | 49 ++++++++ tests/notify_subscribers_test.php | 113 +++++++++++++++++++ 5 files changed, 235 insertions(+) create mode 100644 templates/subscription_notification.mustache create mode 100644 tests/notify_subscribers_test.php diff --git a/classes/task/notify_subscribers.php b/classes/task/notify_subscribers.php index a907279..1e1c242 100644 --- a/classes/task/notify_subscribers.php +++ b/classes/task/notify_subscribers.php @@ -27,6 +27,8 @@ defined('MOODLE_INTERNAL') || die(); +require_once($CFG->dirroot . '/local/amos/locallib.php'); + /** * Sends notifications about changes of the language packs to the respective subsrcibers. * @@ -48,7 +50,29 @@ public function get_name() { * Execute the task. */ public function execute() { + global $DB, $PAGE; + $subject = 'Subscription Notification'; + $content = 'Subscription Content'; + $lasttimerun = get_config('local_amos', 'timesubnotified'); + $getsql = "SELECT distinct s.userid + FROM {amos_commits} c + JOIN {amos_repository} r ON (c.id = r.commitid) + JOIN {amos_texts} t ON (r.textid = t.id) + JOIN {amos_subscription} s ON (s.lang = r.lang AND s.component = r.component) + WHERE timecommitted > $lasttimerun"; + + $users = $DB->get_records_sql($getsql); + $output = $PAGE->get_renderer('local_amos'); + foreach ($users as $user) { + $user = \core_user::get_user($user->userid); + if ($user) { + $notification = new \local_amos_sub_notification($user); + $content = $output->render($notification); + email_to_user($user, \core_user::get_noreply_user(), $subject, $content); + } + } + set_config('timesubnotified', time(), 'local_amos'); } } diff --git a/locallib.php b/locallib.php index 71e896c..adcdb3d 100644 --- a/locallib.php +++ b/locallib.php @@ -1527,6 +1527,43 @@ public function __construct(stdClass $info, stdClass $author=null, stdClass $ass } } +/** + * Represents a collection changes to be sent to subscribers + */ +class local_amos_sub_notification implements renderable, templatable { + + /** @var array of changes to be included in the mail*/ + public $changes = array(); + + /** @var stdClass user */ + public $user; + + /** + * Fetches the required commits from the repository + * + * @param stdClass $user id of the subscriber + */ + public function __construct($user) { + global $DB; + + $time = time() - 86400; + $getsql = "SELECT t.text, r.stringid, r.component, r.lang + FROM {amos_commits} co + JOIN {amos_repository} r ON (co.id = r.commitid) + JOIN {amos_texts} t ON (r.textid = t.id) + JOIN {amos_subscription} s ON (s.lang = r.lang AND s.component = r.component) + WHERE timecommitted > $time"; + $records = $DB->get_records_sql($getsql); + $this->changes = array_values($records); + + $this->user = $user; + } + + public function export_for_template(renderer_base $output) { + return $this; + } +} + /** * Returns the list of standard components * diff --git a/renderer.php b/renderer.php index e443102..fbb784a 100644 --- a/renderer.php +++ b/renderer.php @@ -1632,6 +1632,18 @@ protected function render_local_amos_contribution(local_amos_contribution $contr return $output; } + /** + * Render repository records + * + * @param param local_amos_sub_notification $records of stdclass amos changes + * @return string HTML + */ + protected function render_local_amos_sub_notification(local_amos_sub_notification $notification) { + global $OUTPUT; + return $OUTPUT->render_from_template('local_amos/subscription_notification', + $notification->export_for_template($OUTPUT)); + } + /** * Renders the AMOS credits page * diff --git a/templates/subscription_notification.mustache b/templates/subscription_notification.mustache new file mode 100644 index 0000000..1a4d1cb --- /dev/null +++ b/templates/subscription_notification.mustache @@ -0,0 +1,49 @@ +{{! + This file is part of Moodle - http://moodle.org/ + + Moodle is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Moodle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Moodle. If not, see . +}} +{{! + @template local_amos/subscription_notification + + Template for the notification of langpack changes to a subscriber. + + Context variables required for this template: + * user + * changes - list of changes + + Example context (json): + { "user": { + "firstname": "Mr", + "lastname": "T", + }, + "changes": [ + { + "component": "admin", + "stringid": "accounts", + "lang": "de", + "text": "Accounts" + } + ] + } +}} + +Dear {{user.firstname}} {{user.lastname}}, + +the following changes were made in AMOS: + +{{# changes }} +{{component}} {{stringid}} {{lang}} {{text}} +{{/ changes }} + diff --git a/tests/notify_subscribers_test.php b/tests/notify_subscribers_test.php new file mode 100644 index 0000000..07a1a01 --- /dev/null +++ b/tests/notify_subscribers_test.php @@ -0,0 +1,113 @@ +. + +/** + * Provides the {@link local_amos_notify_subscribers_testcase} class. + * + * @package local_amos + * @category test + * @copyright 2019 Tobias Reischmann + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +require_once($CFG->dirroot . '/local/amos/mlanglib.php'); + +/** + * Unit tests for the AMOS notify_subscribers_tasks. + * + * @copyright 2019 Tobias Reischmann + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class local_amos_notify_subscribers_testcase extends advanced_testcase { + + /** + * Test the permission check for the update_strings_file external function. + */ + public function test_notifications() { + global $DB; + $this->resetAfterTest(true); + + $generator = self::getDataGenerator(); + + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + $user3 = $generator->create_user(); + + $component = new mlang_component('langconfig', 'cs', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('thislanguage', 'Čeština')); + $component->add_string(new mlang_string('thislanguageint', 'Czech')); + $stage = new mlang_stage(); + $stage->add($component); + $stage->commit('Registering the Czech language', array('source' => 'bot'), true); + $component->clear(); + + $component = new mlang_component('langconfig', 'de', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('thislanguage', 'German')); + $component->add_string(new mlang_string('thislanguageint', 'DE')); + $stage = new mlang_stage(); + $stage->add($component); + $stage->commit('Registering the German language', array('source' => 'amos'), true); + $component->clear(); + + $component = new mlang_component('admin', 'de', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('accounts', 'Kontos')); + $stage = new mlang_stage(); + $stage->add($component); + $stage->commit('Registering the German string form accounts', array('source' => 'amos'), true); + $component->clear(); + + // Add a subscription. + $record = new stdClass(); + $record->userid = $user1->id; + $record->lang = 'cs'; + $record->component = 'langconfig'; + $DB->insert_record('amos_subscription', $record); + + // Add a subscription. + $record = new stdClass(); + $record->userid = $user1->id; + $record->lang = 'de'; + $record->component = 'langconfig'; + $DB->insert_record('amos_subscription', $record); + + // Add a subscription. + $record = new stdClass(); + $record->userid = $user2->id; + $record->lang = 'de'; + $record->component = 'admin'; + $DB->insert_record('amos_subscription', $record); + + // Add a subscription, which has not changes. + $record = new stdClass(); + $record->userid = $user3->id; + $record->lang = 'cs'; + $record->component = 'admin'; + $DB->insert_record('amos_subscription', $record); + + set_config('timesubnotified', time() - 86400, 'local_amos'); + + $sink = $this->redirectEmails(); + $task = new \local_amos\task\notify_subscribers(); + $task->execute(); + $this->assertCount(2, $sink->get_messages()); + $this->assertContains("thislanguage", $sink->get_messages()[0]->body); + $sink->close(); + } +} From 284fc0b2a625ca87be44147f3550c1c6ebd87e4e Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 11:18:31 +0200 Subject: [PATCH 05/34] Move notifiaction subject to lang strings --- classes/task/notify_subscribers.php | 3 +-- lang/en/local_amos.php | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/task/notify_subscribers.php b/classes/task/notify_subscribers.php index 1e1c242..71b873d 100644 --- a/classes/task/notify_subscribers.php +++ b/classes/task/notify_subscribers.php @@ -51,8 +51,7 @@ public function get_name() { */ public function execute() { global $DB, $PAGE; - $subject = 'Subscription Notification'; - $content = 'Subscription Content'; + $subject = get_string('subscription_mail_subject', 'local_amos'); $lasttimerun = get_config('local_amos', 'timesubnotified'); $getsql = "SELECT distinct s.userid FROM {amos_commits} c diff --git a/lang/en/local_amos.php b/lang/en/local_amos.php index bfbfb01..955bbee 100644 --- a/lang/en/local_amos.php +++ b/lang/en/local_amos.php @@ -452,6 +452,7 @@ $string['submitting'] = 'Submitting a contribution'; $string['submitting_help'] = 'This will send translated strings to official language maintainers. They will be able to apply your work into their stage, review it and eventually commit. Please provide a message for them describing your work and why you would like to see your contribution included.'; $string['subscription'] = 'Subscription'; +$string['subscription_mail_subject'] = 'AMOS Notification - New language pack changes'; $string['targetversion'] = 'Target version'; $string['tasknotifysubscribers'] = 'Send Notifications to Subscribers'; $string['timeline'] = 'timeline'; From f611ec8c5b090a43c32b428522063bc171729f9c Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 11:19:14 +0200 Subject: [PATCH 06/34] Alter notification query and template --- locallib.php | 75 ++++++++++++++++++-- templates/subscription_notification.mustache | 23 ++++-- 2 files changed, 86 insertions(+), 12 deletions(-) diff --git a/locallib.php b/locallib.php index adcdb3d..7373ee5 100644 --- a/locallib.php +++ b/locallib.php @@ -1532,8 +1532,8 @@ public function __construct(stdClass $info, stdClass $author=null, stdClass $ass */ class local_amos_sub_notification implements renderable, templatable { - /** @var array of changes to be included in the mail*/ - public $changes = array(); + /** @var array of $components including the changes made for string od them*/ + public $components = array(); /** @var stdClass user */ public $user; @@ -1547,14 +1547,77 @@ public function __construct($user) { global $DB; $time = time() - 86400; - $getsql = "SELECT t.text, r.stringid, r.component, r.lang + + $getsqlrelevantstrings = "SELECT r.stringid, r.component, r.lang FROM {amos_commits} co JOIN {amos_repository} r ON (co.id = r.commitid) JOIN {amos_texts} t ON (r.textid = t.id) JOIN {amos_subscription} s ON (s.lang = r.lang AND s.component = r.component) - WHERE timecommitted > $time"; - $records = $DB->get_records_sql($getsql); - $this->changes = array_values($records); + WHERE r.timemodified > ? + GROUP BY r.stringid, r.component, r.lang"; + + $getsqlfortextids = "SELECT relevantstrings.stringid, + relevantstrings.component, + relevantstrings.lang, + max(newtexts.id) newrid, + max(oldtexts.id) oldrid + FROM ($getsqlrelevantstrings) as relevantstrings JOIN + {amos_repository} newtexts ON + relevantstrings.stringid = newtexts.stringid AND + relevantstrings.component = newtexts.component AND + relevantstrings.lang = newtexts.lang AND + newtexts.timemodified > ? LEFT JOIN + {amos_repository} oldtexts ON + relevantstrings.stringid = oldtexts.stringid AND + relevantstrings.component = oldtexts.component AND + relevantstrings.lang = oldtexts.lang AND + oldtexts.timemodified < ? + GROUP BY relevantstrings.stringid, + relevantstrings.component, + relevantstrings.lang"; + + $getsql = "SELECT newrepo.id, innersql.stringid, + innersql.component, + innersql.lang, + newtext.text newtext, + oldtext.text oldtext FROM + ($getsqlfortextids) innersql JOIN + {amos_repository} newrepo ON + innersql.newrid = newrepo.id JOIN + {amos_texts} newtext ON + newrepo.textid = newtext.id LEFT JOIN + {amos_repository} oldrepo ON + innersql.oldrid = oldrepo.id LEFT JOIN + {amos_texts} oldtext ON + oldrepo.textid = oldtext.id"; + $records = $DB->get_records_sql($getsql, array($time,$time,$time)); + + foreach ($records as $record) { + if (!array_key_exists($record->component, $this->components)) { + $component = new stdClass(); + $component->name = $record->component; + $component->langs = []; + $this->components[$record->component] = $component; + } + $component = $this->components[$record->component]; + if (!array_key_exists($record->lang, $component->langs)) { + $lang = new stdClass(); + $lang->name = $record->lang; + $lang->changes = []; + $component->langs[$record->lang] = $lang; + } + $lang = $component->langs[$record->lang]; + $change = new stdClass(); + $change->newtext = $record->newtext; + $change->oldtext = $record->oldtext; + $change->stringid = $record->stringid; + $lang->changes [] = $change; + } + + foreach ($this->components as $component) { + $component->langs = array_values($component->langs); + } + $this->components = array_values($this->components); $this->user = $user; } diff --git a/templates/subscription_notification.mustache b/templates/subscription_notification.mustache index 1a4d1cb..53f033e 100644 --- a/templates/subscription_notification.mustache +++ b/templates/subscription_notification.mustache @@ -28,12 +28,17 @@ "firstname": "Mr", "lastname": "T", }, - "changes": [ + "components": [ { - "component": "admin", - "stringid": "accounts", - "lang": "de", - "text": "Accounts" + "name": "admin", + "langs": [ + name: "de" + changes: [ + branch: "3.7" + oldtext: "Admin" + newtext: "Admin Plugin" + ] + ], } ] } @@ -43,7 +48,13 @@ Dear {{user.firstname}} {{user.lastname}}, the following changes were made in AMOS: +{{# components }} +For the component {{name}} +{{# langs }} +For the lang {{name}} {{# changes }} -{{component}} {{stringid}} {{lang}} {{text}} +[{{branch}}/{{stringid}}]: {{oldtext}} => {{newtext}} {{/ changes }} +{{/ langs}} +{{/ components }} From 6cdefb7d32ae657752729eed5603a0a09325a151 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 12:02:25 +0200 Subject: [PATCH 07/34] Add branch to notify subscriber template and add comments --- locallib.php | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/locallib.php b/locallib.php index 7373ee5..a0096be 100644 --- a/locallib.php +++ b/locallib.php @@ -1548,37 +1548,46 @@ public function __construct($user) { $time = time() - 86400; - $getsqlrelevantstrings = "SELECT r.stringid, r.component, r.lang + // Get all unique combinations of stringid, component, lang and branch. + $getsqlrelevantstrings = "SELECT r.stringid, r.component, r.lang, r.branch FROM {amos_commits} co JOIN {amos_repository} r ON (co.id = r.commitid) JOIN {amos_texts} t ON (r.textid = t.id) JOIN {amos_subscription} s ON (s.lang = r.lang AND s.component = r.component) WHERE r.timemodified > ? - GROUP BY r.stringid, r.component, r.lang"; + GROUP BY r.stringid, r.component, r.lang, r.branch"; + // Query the newest and oldest repository ids for the respective relevant string combinations. $getsqlfortextids = "SELECT relevantstrings.stringid, relevantstrings.component, relevantstrings.lang, + relevantstrings.branch, max(newtexts.id) newrid, max(oldtexts.id) oldrid FROM ($getsqlrelevantstrings) as relevantstrings JOIN {amos_repository} newtexts ON relevantstrings.stringid = newtexts.stringid AND relevantstrings.component = newtexts.component AND - relevantstrings.lang = newtexts.lang AND + relevantstrings.lang = newtexts.lang AND + relevantstrings.branch = newtexts.branch AND newtexts.timemodified > ? LEFT JOIN {amos_repository} oldtexts ON relevantstrings.stringid = oldtexts.stringid AND relevantstrings.component = oldtexts.component AND relevantstrings.lang = oldtexts.lang AND + relevantstrings.branch = oldtexts.branch AND oldtexts.timemodified < ? GROUP BY relevantstrings.stringid, relevantstrings.component, - relevantstrings.lang"; + relevantstrings.lang, + relevantstrings.branch"; + // Actually query the result containing the identifiers of the unique combinaiton. + // And also the old and the new texts. $getsql = "SELECT newrepo.id, innersql.stringid, innersql.component, innersql.lang, + innersql.branch, newtext.text newtext, oldtext.text oldtext FROM ($getsqlfortextids) innersql JOIN @@ -1592,6 +1601,7 @@ public function __construct($user) { oldrepo.textid = oldtext.id"; $records = $DB->get_records_sql($getsql, array($time,$time,$time)); + // Build the data format for the mustach template. foreach ($records as $record) { if (!array_key_exists($record->component, $this->components)) { $component = new stdClass(); @@ -1611,9 +1621,11 @@ public function __construct($user) { $change->newtext = $record->newtext; $change->oldtext = $record->oldtext; $change->stringid = $record->stringid; + $change->branch = mlang_version::by_code($record->branch)->label; $lang->changes [] = $change; } + // Remove the text keys and replace them by random ints in order for mustach to work. foreach ($this->components as $component) { $component->langs = array_values($component->langs); } @@ -1622,6 +1634,9 @@ public function __construct($user) { $this->user = $user; } + /** + * The class already stores the data in the correct structure, so it returns itself. + */ public function export_for_template(renderer_base $output) { return $this; } From 4ea95852b2ec07671e217b55c9a7c50289c138ea Mon Sep 17 00:00:00 2001 From: Kathrin Osswald Date: Wed, 11 Sep 2019 12:19:35 +0200 Subject: [PATCH 08/34] Enhanced mail with HTML tags --- templates/subscription_notification.mustache | 33 ++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/templates/subscription_notification.mustache b/templates/subscription_notification.mustache index 53f033e..c4b302b 100644 --- a/templates/subscription_notification.mustache +++ b/templates/subscription_notification.mustache @@ -44,9 +44,38 @@ } }} -Dear {{user.firstname}} {{user.lastname}}, +

Dear {{user.firstname}} {{user.lastname}},
+ the following string changes were made in AMOS for your subscripted plugins:

-the following changes were made in AMOS: +
+ {{#components}} +

{{{name}}} {{{lang}}}:

+ + + + + + + + {{#changes}} + + + + + + + {{/changes}} +
String identifierVersionOld valueNew value
+ {{{ stringid }}} + + {{{ branch }}} + + {{{ oldtext }}} + + {{{ newtext }}} +
+ {{/components}} +
{{# components }} For the component {{name}} From 6233903ef1fe07b575e90fd4ded1a9334c468f91 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 12:18:06 +0200 Subject: [PATCH 09/34] Remove one sublevel of mustache templates We now have the language on the same level as we have the component name. However, we might have multiple entries for the same component, but only one for each component-lang combination. --- locallib.php | 22 +++++++------------- templates/subscription_notification.mustache | 12 +++++------ 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/locallib.php b/locallib.php index a0096be..c4c2d93 100644 --- a/locallib.php +++ b/locallib.php @@ -1603,32 +1603,24 @@ public function __construct($user) { // Build the data format for the mustach template. foreach ($records as $record) { - if (!array_key_exists($record->component, $this->components)) { + $identifier = $record->component . '#' . $record->lang; + if (!array_key_exists($identifier, $this->components)) { $component = new stdClass(); $component->name = $record->component; - $component->langs = []; - $this->components[$record->component] = $component; + $component->lang = $record->lang; + $component->changes + $this->components[$identifier] = $component; } - $component = $this->components[$record->component]; - if (!array_key_exists($record->lang, $component->langs)) { - $lang = new stdClass(); - $lang->name = $record->lang; - $lang->changes = []; - $component->langs[$record->lang] = $lang; - } - $lang = $component->langs[$record->lang]; + $component = $this->components[$identifier]; $change = new stdClass(); $change->newtext = $record->newtext; $change->oldtext = $record->oldtext; $change->stringid = $record->stringid; $change->branch = mlang_version::by_code($record->branch)->label; - $lang->changes [] = $change; + $component->changes [] = $change; } // Remove the text keys and replace them by random ints in order for mustach to work. - foreach ($this->components as $component) { - $component->langs = array_values($component->langs); - } $this->components = array_values($this->components); $this->user = $user; diff --git a/templates/subscription_notification.mustache b/templates/subscription_notification.mustache index c4b302b..2c06366 100644 --- a/templates/subscription_notification.mustache +++ b/templates/subscription_notification.mustache @@ -31,13 +31,11 @@ "components": [ { "name": "admin", - "langs": [ - name: "de" - changes: [ - branch: "3.7" - oldtext: "Admin" - newtext: "Admin Plugin" - ] + "lang": "de", + changes: [ + branch: "3.7" + oldtext: "Admin" + newtext: "Admin Plugin" ], } ] From 202a2c843583250b9b2534cdaf8fca08c242014d Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 12:22:27 +0200 Subject: [PATCH 10/34] Fixed missing array initializiation --- locallib.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/locallib.php b/locallib.php index c4c2d93..3c3898f 100644 --- a/locallib.php +++ b/locallib.php @@ -1608,7 +1608,7 @@ public function __construct($user) { $component = new stdClass(); $component->name = $record->component; $component->lang = $record->lang; - $component->changes + $component->changes = []; $this->components[$identifier] = $component; } $component = $this->components[$identifier]; From e02200aaa87d55a62d84d48a70444ca02996d8e7 Mon Sep 17 00:00:00 2001 From: Martin Gauk Date: Wed, 11 Sep 2019 12:25:34 +0200 Subject: [PATCH 11/34] Add subscription manager class and test --- classes/subscription_manager.php | 187 ++++++++++++++++++++++++++++ tests/subscription_manager_test.php | 120 ++++++++++++++++++ 2 files changed, 307 insertions(+) create mode 100644 classes/subscription_manager.php create mode 100644 tests/subscription_manager_test.php diff --git a/classes/subscription_manager.php b/classes/subscription_manager.php new file mode 100644 index 0000000..75774e4 --- /dev/null +++ b/classes/subscription_manager.php @@ -0,0 +1,187 @@ +. + +/** + * Provides the {@link subscription_manager} class. + * + * @package local_amos + * @copyright 2019 Tobias Reischmann + * @copyright 2019 Martin Gauk + * @copyright 2019 Jan Eberhardt + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace local_amos; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +require_once($CFG->dirroot.'/local/amos/mlanglib.php'); + +/** + * Manager class for accessing and updating the user's subscriptions. + * + * @copyright 2019 Tobias Reischmann + * @copyright 2019 Martin Gauk + * @copyright 2019 Jan Eberhardt + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class subscription_manager { + /** @var int user id */ + private $userid; + + /** @var array of array (component -> language) */ + private $subscriptions; + + /** @var array */ + private $addsubscriptions; + + /** @var array */ + private $remsubscriptions; + + /** + * Subscription manager constructor. + * + * @param int $userid user id + */ + public function __construct(int $userid) { + $this->userid = $userid; + $this->subscriptions = []; + $this->addsubscriptions = []; + $this->remsubscriptions = []; + } + + /** + * Fetch all subscriptions of the user. + * + * @return array + */ + public function fetch_subscriptions() { + global $DB; + + $this->subscriptions = []; + $rows = $DB->get_records('amos_subscription', ['userid' => $this->userid]); + foreach ($rows as $row) { + if (!isset($this->subscriptions[$row->component])) { + $this->subscriptions[$row->component] = [$row->lang]; + } else { + $this->subscriptions[$row->component][] = $row->lang; + } + } + + return $this->subscriptions; + } + + /** + * Add new subscription. + * + * @param string $component component name + * @param string $lang language code + */ + public function add_subscription(string $component, string $lang) { + $this->addsubscriptions[] = (object) ['component' => $component, 'lang' => $lang]; + } + + /** + * Remove one subscription. + * + * @param string $component component name + * @param string $lang language code + */ + public function remove_subscription(string $component, string $lang) { + $this->remsubscriptions[] = (object) ['component' => $component, 'lang' => $lang]; + } + + /** + * Remove all language subscriptions of one component. + * + * @param string $component component name + */ + public function remove_component_subscription(string $component) { + $this->remsubscriptions[] = (object) ['component' => $component, 'lang' => null]; + } + + /** + * Apply all changes to the database. + * + * All changes that you registered with add_subscription, remove_subscription and remove_component_subscription. + */ + public function apply_changes() { + global $DB; + + // Get available components and langs. + $components = \mlang_tools::list_components(); + $langs = \mlang_tools::list_languages(); + + $transaction = $DB->start_delegated_transaction(); + $this->fetch_subscriptions(); + + $inserts = []; + // Validate component names and language codes of subscriptions that should be added. + foreach ($this->addsubscriptions as $sub) { + if (!isset($components[$sub->component])) { + continue; + } + + if (!isset($langs[$sub->lang])) { + continue; + } + + // Check if not already subscribed. + if (isset($this->subscriptions[$sub->component]) && + in_array($sub->lang, $this->subscriptions[$sub->component])) { + continue; + } + + $sub->userid = $this->userid; + $inserts[] = $sub; + $this->subscriptions[$sub->component][] = $sub->lang; + } + $DB->insert_records('amos_subscription', $inserts); + + // Remove subscriptions. + foreach ($this->remsubscriptions as $sub) { + if ($sub->lang !== null) { + $DB->delete_records('amos_subscription', [ + 'userid' => $this->userid, + 'component' => $sub->component, + 'lang' => $sub->lang, + ]); + } else { + $DB->delete_records('amos_subscription', [ + 'userid' => $this->userid, + 'component' => $sub->component, + ]); + } + } + + $transaction->allow_commit(); + + // Reset list of changes. + $this->addsubscriptions = []; + $this->remsubscriptions = []; + } + + /** + * Remove all subscriptions of the user. + */ + public function remove_all_subscriptions() { + global $DB; + + $DB->delete_records('amos_subscription', ['userid' => $this->userid]); + } +} diff --git a/tests/subscription_manager_test.php b/tests/subscription_manager_test.php new file mode 100644 index 0000000..3a75938 --- /dev/null +++ b/tests/subscription_manager_test.php @@ -0,0 +1,120 @@ +. + +/** + * Provides the {@link local_amos_subscription_manager_test} class. + * + * @package local_amos + * @category test + * @copyright 2019 Tobias Reischmann + * @copyright 2019 Martin Gauk + * @copyright 2019 Jan Eberhardt + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +require_once($CFG->dirroot.'/local/amos/mlanglib.php'); + +/** + * Tests for the {@link local_amos\subscription_manager} class. + */ +class local_amos_subscription_manager_test extends advanced_testcase { + + /** + * Helper method to quickly register a language on the given branch(-es) + * + * @param string $langcode the code of the language, such as 'en' + * @param int|array $branchcodes the code of the branch or a list of them + */ + protected function register_language($langcode, $branchcodes) { + + if (!is_array($branchcodes)) { + $branchcodes = array($branchcodes); + } + + $stage = new mlang_stage(); + + foreach ($branchcodes as $branchcode) { + $component = new mlang_component('langconfig', $langcode, mlang_version::by_code($branchcode)); + $component->add_string(new mlang_string('thislanguage', $langcode)); + $component->add_string(new mlang_string('thislanguageint', $langcode)); + $stage->add($component); + $component->clear(); + } + + $stage->commit('Register language '.$langcode, array('source' => 'unittest')); + } + + /** + * Test the workflow of fetching, adding and removing subscriptions. + */ + public function test_subscriptions() { + $this->resetAfterTest(true); + + $this->register_language('en', mlang_version::MOODLE_36); + $this->register_language('de', mlang_version::MOODLE_36); + $this->register_language('fr', mlang_version::MOODLE_36); + $this->register_language('cz', mlang_version::MOODLE_36); + + // Test subscriptions of one user. + $manager = new \local_amos\subscription_manager(2); + $manager->add_subscription('langconfig', 'de'); + $manager->add_subscription('langconfig', 'fr'); + $manager->add_subscription('langconfig', 'cz'); + $manager->add_subscription('langconfig', 'aa'); // Should not be added as it is invalid. + $manager->apply_changes(); + + $subs = $manager->fetch_subscriptions(); + $this->assertArrayHasKey('langconfig', $subs); + $this->assertTrue(in_array('de', $subs['langconfig'])); + $this->assertTrue(in_array('fr', $subs['langconfig'])); + $this->assertTrue(in_array('cz', $subs['langconfig'])); + $this->assertFalse(in_array('aa', $subs['langconfig'])); + + // Remove one language. + $manager->remove_subscription('langconfig', 'de'); + $manager->apply_changes(); + + $subs = $manager->fetch_subscriptions(); + $this->assertArrayHasKey('langconfig', $subs); + $this->assertFalse(in_array('de', $subs['langconfig'])); + $this->assertTrue(in_array('fr', $subs['langconfig'])); + $this->assertTrue(in_array('cz', $subs['langconfig'])); + + // Remove all other languages for one component. + $manager->remove_component_subscription('langconfig'); + $manager->apply_changes(); + + $subs = $manager->fetch_subscriptions(); + $this->assertEmpty($subs); + + // Add languages again and remove all. + $manager->add_subscription('langconfig', 'de'); + $manager->add_subscription('langconfig', 'fr'); + $manager->add_subscription('langconfig', 'cz'); + $manager->apply_changes(); + + $subs = $manager->fetch_subscriptions(); + $this->assertEquals(3, count($subs['langconfig'])); + + $manager->remove_all_subscriptions(); + $subs = $manager->fetch_subscriptions(); + $this->assertEmpty($subs); + } +} From dcdecd14fc7913b3515df40c1fa26da919a43186 Mon Sep 17 00:00:00 2001 From: Jan Eberhardt Date: Wed, 11 Sep 2019 13:35:20 +0200 Subject: [PATCH 12/34] Basic UI - form actions missing Changes to be committed: new file: classes/local/subscription_table.php modified: lang/en/local_amos.php modified: renderer.php modified: subscription.php --- classes/local/subscription_table.php | 94 ++++++++++++++++++++++++++++ lang/en/local_amos.php | 2 + renderer.php | 2 +- subscription.php | 8 ++- 4 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 classes/local/subscription_table.php diff --git a/classes/local/subscription_table.php b/classes/local/subscription_table.php new file mode 100644 index 0000000..33c768d --- /dev/null +++ b/classes/local/subscription_table.php @@ -0,0 +1,94 @@ +. + +/** + * View your subscription and change settings + * + * @package local_amos + * @copyright 2019 Tobias Reischmann + * @copyright 2019 Martin Gauk + * @copyright 2019 Jan Eberhardt + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace local_amos\local; + + +use local_amos\subscription_manager; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +require_once $CFG->libdir . DIRECTORY_SEPARATOR . 'tablelib.php'; +require_once $CFG->dirroot . '/local/amos/classes/subscription_manager.php'; +require_once $CFG->dirroot . '/local/amos/mlanglib.php'; + +class subscription_table extends \table_sql { + + public function __construct($uniqueid) + { + global $USER; + + parent::__construct($uniqueid); + + $this->define_columns([ + 'component', + 'lang' + ]); + $this->define_headers([ + get_string('component', 'local_amos'), + get_string('languages', 'local_amos') + ]); + } + + public function query_db($pagesize, $useinitialsbar = true) + { + global $USER; + + $manager = new subscription_manager($USER->id); + $subs = $manager->fetch_subscriptions(); + + $this->pagesize($pagesize, count($subs)); + foreach ($subs as $component => $langarray) { + $this->rawdata[] = [ + 'component' => $component, + 'lang' => $langarray + ]; + } + } + + public function col_component($sub) { + return $sub->component; + } + + public function col_lang($sub) { + $icons = []; + foreach ($sub->lang as $lc) { + $query_string = sprintf('c=%s&l=%s&m=0', $sub->component, $lc); + $attributes = ['class' => 'langcode']; + $icons[] = \html_writer::link( + $this->baseurl . '?' . $query_string, + $lc . ' ×', + $attributes + ); + } + return join(' ', $icons); + } + +} \ No newline at end of file diff --git a/lang/en/local_amos.php b/lang/en/local_amos.php index 955bbee..25a6bc0 100644 --- a/lang/en/local_amos.php +++ b/lang/en/local_amos.php @@ -27,6 +27,7 @@ $string['about'] = '

AMOS is a central repository of Moodle strings and their history. It tracks the addition of English strings into Moodle code, gathers translations, handles common translation tasks and generates language packages to be deployed on Moodle servers.

See AMOS documentation for more information.

'; +$string['addsubscription'] = 'Add Subscription'; $string['amos'] = 'AMOS translation toolkit'; $string['amos:changecontriblang'] = 'Change language of contributed strings'; $string['amos:commit'] = 'Commit the staged strings into the main repository'; @@ -47,6 +48,7 @@ $string['commitstage_help'] = 'Permanently store all staged translations in AMOS repository. Stage is automatically pruned and rebased before it is committed. Only committable strings are stored. That means that only translations below highlighted in green will be stored. The stage is cleared after the commit.'; $string['committableall'] = 'all languages'; $string['committablenone'] = 'no languages allowed - please contact AMOS manager'; +$string['component'] = 'Component'; $string['componentsall'] = 'All'; $string['componentsapp'] = 'Moodle App'; $string['componentsenlarge'] = 'Enlarge'; diff --git a/renderer.php b/renderer.php index fbb784a..b42e58c 100644 --- a/renderer.php +++ b/renderer.php @@ -50,7 +50,7 @@ protected function render_local_amos_subscription_filter(local_amos_subscription // close form $output .= html_writer::end_tag('fieldset'); - $button = ''; + $button = ''; $output .= html_writer::div($button, 'form-actions'); $output .= html_writer::end_tag('form'); $output .= html_writer::end_div(); diff --git a/subscription.php b/subscription.php index 826ea6d..6bd541a 100644 --- a/subscription.php +++ b/subscription.php @@ -32,7 +32,9 @@ require_login(SITEID, false); #$name = optional_param('name', null, PARAM_RAW); // stash name -$mode = optional_param('m', null, PARAM_ALPHA); +$mode = optional_param('m', null, PARAM_INT); +$component = optional_param('c', null, PARAM_ALPHAEXT); +$lang = optional_param('l', null, PARAM_ALPHAEXT); $PAGE->set_pagelayout('standard'); $PAGE->set_url('/local/amos/subscription.php'); @@ -45,6 +47,10 @@ $filter = new local_amos_subscription_filter($PAGE->url); $output = $PAGE->get_renderer('local_amos'); +$table = new local_amos\local\subscription_table('subscription_table'); +$table->define_baseurl($PAGE->url); + echo $output->header(); echo $output->render($filter); +echo $table->out(40, true); echo $output->footer(); \ No newline at end of file From 38de627b12f5575fd1e02cc5ca40e20fb6fbb60c Mon Sep 17 00:00:00 2001 From: Kathrin Osswald Date: Wed, 11 Sep 2019 14:05:25 +0200 Subject: [PATCH 13/34] Added some styling to the mail template. --- templates/subscription_notification.mustache | 31 +++++++------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/templates/subscription_notification.mustache b/templates/subscription_notification.mustache index 2c06366..77356a3 100644 --- a/templates/subscription_notification.mustache +++ b/templates/subscription_notification.mustache @@ -42,46 +42,37 @@ } }} +

Dear {{user.firstname}} {{user.lastname}},
the following string changes were made in AMOS for your subscripted plugins:

{{#components}} -

{{{name}}} {{{lang}}}:

+

{{{name}}} ({{{lang}}}):

- - - - + + + + {{#changes}} - - - - - +
{{/changes}}
String identifierVersionOld valueNew valueString identifierVersionOld valueNew value
+ {{{ stringid }}} + {{{ branch }}} + {{{ oldtext }}} + {{{ newtext }}}
{{/components}}
-{{# components }} -For the component {{name}} -{{# langs }} -For the lang {{name}} -{{# changes }} -[{{branch}}/{{stringid}}]: {{oldtext}} => {{newtext}} -{{/ changes }} -{{/ langs}} -{{/ components }} - From 66c79d74dc641472359b4341a4e38679bbf12e2f Mon Sep 17 00:00:00 2001 From: Kathrin Osswald Date: Wed, 11 Sep 2019 14:07:09 +0200 Subject: [PATCH 14/34] Added HTML content to email_to_user --- classes/task/notify_subscribers.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/task/notify_subscribers.php b/classes/task/notify_subscribers.php index 71b873d..7295919 100644 --- a/classes/task/notify_subscribers.php +++ b/classes/task/notify_subscribers.php @@ -68,7 +68,7 @@ public function execute() { $notification = new \local_amos_sub_notification($user); $content = $output->render($notification); - email_to_user($user, \core_user::get_noreply_user(), $subject, $content); + email_to_user($user, \core_user::get_noreply_user(), $subject, $content, $content); } } set_config('timesubnotified', time(), 'local_amos'); From 6596c42caf36f48c839b9a3af6f34eba2a4842d0 Mon Sep 17 00:00:00 2001 From: Kathrin Osswald Date: Wed, 11 Sep 2019 14:32:17 +0200 Subject: [PATCH 15/34] Added plain text mail template. --- templates/subscription_notification.mustache | 45 ++++------- .../subscription_notification_html.mustache | 78 +++++++++++++++++++ 2 files changed, 92 insertions(+), 31 deletions(-) create mode 100644 templates/subscription_notification_html.mustache diff --git a/templates/subscription_notification.mustache b/templates/subscription_notification.mustache index 77356a3..fcf8ae7 100644 --- a/templates/subscription_notification.mustache +++ b/templates/subscription_notification.mustache @@ -43,36 +43,19 @@ }} -

Dear {{user.firstname}} {{user.lastname}},
- the following string changes were made in AMOS for your subscripted plugins:

+Dear {{user.firstname}} {{user.lastname}}, + +the following string changes were made in AMOS for your subscripted plugins: + +{{#components}} + {{{name}}} ({{{lang}}}): + + {{#changes}} + String identifier: {{{ stringid }}} + Version: {{{ branch }}} + Old string: {{{ oldtext }}} + New string: {{{ newtext }}} + {{/changes}} +{{/components}} -
- {{#components}} -

{{{name}}} ({{{lang}}}):

- - - - - - - - {{#changes}} - - - - - -
- {{/changes}} -
String identifierVersionOld valueNew value
- {{{ stringid }}} - - {{{ branch }}} - - {{{ oldtext }}} - - {{{ newtext }}} -
- {{/components}} -
diff --git a/templates/subscription_notification_html.mustache b/templates/subscription_notification_html.mustache new file mode 100644 index 0000000..77356a3 --- /dev/null +++ b/templates/subscription_notification_html.mustache @@ -0,0 +1,78 @@ +{{! + This file is part of Moodle - http://moodle.org/ + + Moodle is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Moodle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Moodle. If not, see . +}} +{{! + @template local_amos/subscription_notification + + Template for the notification of langpack changes to a subscriber. + + Context variables required for this template: + * user + * changes - list of changes + + Example context (json): + { "user": { + "firstname": "Mr", + "lastname": "T", + }, + "components": [ + { + "name": "admin", + "lang": "de", + changes: [ + branch: "3.7" + oldtext: "Admin" + newtext: "Admin Plugin" + ], + } + ] + } +}} + + +

Dear {{user.firstname}} {{user.lastname}},
+ the following string changes were made in AMOS for your subscripted plugins:

+ +
+ {{#components}} +

{{{name}}} ({{{lang}}}):

+ + + + + + + + {{#changes}} + + + + + +
+ {{/changes}} +
String identifierVersionOld valueNew value
+ {{{ stringid }}} + + {{{ branch }}} + + {{{ oldtext }}} + + {{{ newtext }}} +
+ {{/components}} +
+ From 4abd3dfadd1afd05158c2a49169fc3603b3e0b25 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 14:35:24 +0200 Subject: [PATCH 16/34] Account for html and plaintext mustache mail template --- classes/task/notify_subscribers.php | 4 +++- locallib.php | 8 +++++++- renderer.php | 8 +++++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/classes/task/notify_subscribers.php b/classes/task/notify_subscribers.php index 7295919..9595e78 100644 --- a/classes/task/notify_subscribers.php +++ b/classes/task/notify_subscribers.php @@ -65,10 +65,12 @@ public function execute() { foreach ($users as $user) { $user = \core_user::get_user($user->userid); if ($user) { + $notification_html = new \local_amos_sub_notification($user, true); $notification = new \local_amos_sub_notification($user); + $content_html = $output->render($notification_html); $content = $output->render($notification); - email_to_user($user, \core_user::get_noreply_user(), $subject, $content, $content); + email_to_user($user, \core_user::get_noreply_user(), $subject, $content, $content_html); } } set_config('timesubnotified', time(), 'local_amos'); diff --git a/locallib.php b/locallib.php index 3c3898f..35a3aec 100644 --- a/locallib.php +++ b/locallib.php @@ -1538,12 +1538,16 @@ class local_amos_sub_notification implements renderable, templatable { /** @var stdClass user */ public $user; + /** @var bool $html States if the notification should be */ + public $html; + /** * Fetches the required commits from the repository * * @param stdClass $user id of the subscriber + * @param bool $html States if the notification should be */ - public function __construct($user) { + public function __construct($user, $html = false) { global $DB; $time = time() - 86400; @@ -1624,6 +1628,8 @@ public function __construct($user) { $this->components = array_values($this->components); $this->user = $user; + + $this->html = $html; } /** diff --git a/renderer.php b/renderer.php index b42e58c..296e89c 100644 --- a/renderer.php +++ b/renderer.php @@ -1640,8 +1640,14 @@ protected function render_local_amos_contribution(local_amos_contribution $contr */ protected function render_local_amos_sub_notification(local_amos_sub_notification $notification) { global $OUTPUT; - return $OUTPUT->render_from_template('local_amos/subscription_notification', + if ($notification->html) { + $template = 'local_amos/subscription_notification+html'; + } else { + $template = 'local_amos/subscription_notification'; + } + return $OUTPUT->render_from_template($template, $notification->export_for_template($OUTPUT)); + } /** From 78349297886ef27d81aae0dca639b8a2031089b3 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 14:36:20 +0200 Subject: [PATCH 17/34] Alter subscription sql to not select the highest repo id but the newest --- locallib.php | 65 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/locallib.php b/locallib.php index 35a3aec..1676b86 100644 --- a/locallib.php +++ b/locallib.php @@ -1553,7 +1553,11 @@ public function __construct($user, $html = false) { $time = time() - 86400; // Get all unique combinations of stringid, component, lang and branch. - $getsqlrelevantstrings = "SELECT r.stringid, r.component, r.lang, r.branch + $getsqlrelevantstrings = "SELECT r.stringid, + r.component, + r.lang, + r.branch, + max(r.timemodified) timemodified FROM {amos_commits} co JOIN {amos_repository} r ON (co.id = r.commitid) JOIN {amos_texts} t ON (r.textid = t.id) @@ -1562,21 +1566,15 @@ public function __construct($user, $html = false) { GROUP BY r.stringid, r.component, r.lang, r.branch"; // Query the newest and oldest repository ids for the respective relevant string combinations. - $getsqlfortextids = "SELECT relevantstrings.stringid, + $getsqlforoldmodified = "SELECT relevantstrings.stringid, relevantstrings.component, relevantstrings.lang, relevantstrings.branch, - max(newtexts.id) newrid, - max(oldtexts.id) oldrid - FROM ($getsqlrelevantstrings) as relevantstrings JOIN - {amos_repository} newtexts ON - relevantstrings.stringid = newtexts.stringid AND - relevantstrings.component = newtexts.component AND - relevantstrings.lang = newtexts.lang AND - relevantstrings.branch = newtexts.branch AND - newtexts.timemodified > ? LEFT JOIN - {amos_repository} oldtexts ON - relevantstrings.stringid = oldtexts.stringid AND + relevantstrings.timemodified newmodified, + max(oldtexts.timemodified) oldmodified + FROM ($getsqlrelevantstrings) as relevantstrings LEFT JOIN + {amos_repository} oldtexts ON + relevantstrings.stringid = oldtexts.stringid AND relevantstrings.component = oldtexts.component AND relevantstrings.lang = oldtexts.lang AND relevantstrings.branch = oldtexts.branch AND @@ -1584,29 +1582,50 @@ public function __construct($user, $html = false) { GROUP BY relevantstrings.stringid, relevantstrings.component, relevantstrings.lang, + relevantstrings.branch, + relevantstrings.timemodified"; + + $getrepoids = "SELECT relevantstrings.stringid, + relevantstrings.component, + relevantstrings.lang, + relevantstrings.branch, + max(newrepo.textid) newtextid, + max(oldrepo.textid) oldtextid + FROM ($getsqlforoldmodified) as relevantstrings JOIN + {amos_repository} newrepo ON + relevantstrings.stringid = newrepo.stringid AND + relevantstrings.component = newrepo.component AND + relevantstrings.lang = newrepo.lang AND + relevantstrings.branch = newrepo.branch AND + relevantstrings.newmodified = newrepo.timemodified LEFT JOIN + {amos_repository} oldrepo ON + relevantstrings.stringid = oldrepo.stringid AND + relevantstrings.component = oldrepo.component AND + relevantstrings.lang = oldrepo.lang AND + relevantstrings.branch = oldrepo.branch AND + relevantstrings.oldmodified = oldrepo.timemodified + GROUP BY relevantstrings.stringid, + relevantstrings.component, + relevantstrings.lang, relevantstrings.branch"; // Actually query the result containing the identifiers of the unique combinaiton. // And also the old and the new texts. - $getsql = "SELECT newrepo.id, innersql.stringid, + $getsql = "SELECT innersql.stringid, innersql.component, innersql.lang, innersql.branch, newtext.text newtext, oldtext.text oldtext FROM - ($getsqlfortextids) innersql JOIN - {amos_repository} newrepo ON - innersql.newrid = newrepo.id JOIN + ($getrepoids) as innersql JOIN {amos_texts} newtext ON - newrepo.textid = newtext.id LEFT JOIN - {amos_repository} oldrepo ON - innersql.oldrid = oldrepo.id LEFT JOIN + innersql.newtextid = newtext.id LEFT JOIN {amos_texts} oldtext ON - oldrepo.textid = oldtext.id"; - $records = $DB->get_records_sql($getsql, array($time,$time,$time)); + innersql.oldtextid = oldtext.id"; + $recordset = $DB->get_recordset_sql($getsql, array($time, $time)); // Build the data format for the mustach template. - foreach ($records as $record) { + foreach ($recordset as $record) { $identifier = $record->component . '#' . $record->lang; if (!array_key_exists($identifier, $this->components)) { $component = new stdClass(); From 2b232ee709cbd0713ec7781a9ac07b475f4c900d Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 14:37:38 +0200 Subject: [PATCH 18/34] Fix wrong template name --- renderer.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/renderer.php b/renderer.php index 296e89c..c5a373e 100644 --- a/renderer.php +++ b/renderer.php @@ -1641,12 +1641,11 @@ protected function render_local_amos_contribution(local_amos_contribution $contr protected function render_local_amos_sub_notification(local_amos_sub_notification $notification) { global $OUTPUT; if ($notification->html) { - $template = 'local_amos/subscription_notification+html'; + $template = 'local_amos/subscription_notification_html'; } else { $template = 'local_amos/subscription_notification'; } - return $OUTPUT->render_from_template($template, - $notification->export_for_template($OUTPUT)); + return $OUTPUT->render_from_template($template, $notification->export_for_template($OUTPUT)); } From ff1a1f2a996969cf47956f74cf4e4068f4ed57f2 Mon Sep 17 00:00:00 2001 From: Kathrin Osswald Date: Wed, 11 Sep 2019 14:44:21 +0200 Subject: [PATCH 19/34] Removed hr tag. --- templates/subscription_notification_html.mustache | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/subscription_notification_html.mustache b/templates/subscription_notification_html.mustache index 77356a3..f26e725 100644 --- a/templates/subscription_notification_html.mustache +++ b/templates/subscription_notification_html.mustache @@ -70,7 +70,7 @@ {{{ newtext }}} -
+ {{/changes}} {{/components}} From 3881b60fe85cb4fef2267de74289eef50159a3a6 Mon Sep 17 00:00:00 2001 From: Martin Gauk Date: Wed, 11 Sep 2019 14:54:27 +0200 Subject: [PATCH 20/34] Inplace editable component languages --- classes/local/subscription_table.php | 32 +++++++++++++++++--------- classes/subscription_manager.php | 34 +++++++++++++++------------- lib.php | 25 ++++++++++++++++++++ 3 files changed, 64 insertions(+), 27 deletions(-) diff --git a/classes/local/subscription_table.php b/classes/local/subscription_table.php index 33c768d..861da70 100644 --- a/classes/local/subscription_table.php +++ b/classes/local/subscription_table.php @@ -55,6 +55,7 @@ public function __construct($uniqueid) get_string('component', 'local_amos'), get_string('languages', 'local_amos') ]); + } public function query_db($pagesize, $useinitialsbar = true) @@ -78,17 +79,26 @@ public function col_component($sub) { } public function col_lang($sub) { - $icons = []; - foreach ($sub->lang as $lc) { - $query_string = sprintf('c=%s&l=%s&m=0', $sub->component, $lc); - $attributes = ['class' => 'langcode']; - $icons[] = \html_writer::link( - $this->baseurl . '?' . $query_string, - $lc . ' ×', - $attributes - ); - } - return join(' ', $icons); + global $OUTPUT; + + $inplace = self::get_lang_inplace_editable($sub->component, $sub->lang); + return $OUTPUT->render_from_template('core/inplace_editable', $inplace->export_for_template($OUTPUT)); } + static public function get_lang_inplace_editable(string $component, array $langs) { + $options = \mlang_tools::list_languages(false); + + $values = json_encode($langs); + $displayvalues = implode(', ', array_map(function($lang) use ($options) { + return (isset($options[$lang])) ? $options[$lang] : ''; + }, $langs)); + + $inplace = new \core\output\inplace_editable('local_amos', 'subscription', $component, + true, $displayvalues, $values); + + $attributes = ['multiple' => true]; + $inplace->set_type_autocomplete($options, $attributes); + + return $inplace; + } } \ No newline at end of file diff --git a/classes/subscription_manager.php b/classes/subscription_manager.php index 75774e4..f11d13f 100644 --- a/classes/subscription_manager.php +++ b/classes/subscription_manager.php @@ -128,6 +128,24 @@ public function apply_changes() { $langs = \mlang_tools::list_languages(); $transaction = $DB->start_delegated_transaction(); + + // Remove subscriptions. + foreach ($this->remsubscriptions as $sub) { + if ($sub->lang !== null) { + $DB->delete_records('amos_subscription', [ + 'userid' => $this->userid, + 'component' => $sub->component, + 'lang' => $sub->lang, + ]); + } else { + $DB->delete_records('amos_subscription', [ + 'userid' => $this->userid, + 'component' => $sub->component, + ]); + } + } + + // Refresh subscriptions to check for duplicates. $this->fetch_subscriptions(); $inserts = []; @@ -153,22 +171,6 @@ public function apply_changes() { } $DB->insert_records('amos_subscription', $inserts); - // Remove subscriptions. - foreach ($this->remsubscriptions as $sub) { - if ($sub->lang !== null) { - $DB->delete_records('amos_subscription', [ - 'userid' => $this->userid, - 'component' => $sub->component, - 'lang' => $sub->lang, - ]); - } else { - $DB->delete_records('amos_subscription', [ - 'userid' => $this->userid, - 'component' => $sub->component, - ]); - } - } - $transaction->allow_commit(); // Reset list of changes. diff --git a/lib.php b/lib.php index 8669fb4..7e061f6 100644 --- a/lib.php +++ b/lib.php @@ -185,3 +185,28 @@ function local_amos_comment_template($params) { return $template; } + +function local_amos_inplace_editable($itemtype, $itemid, $newvalue) { + global $USER; + + \external_api::validate_context(context_system::instance()); + + if ($itemtype === 'subscription') { + $component = clean_param($itemid, PARAM_ALPHANUMEXT); + $langs = json_decode($newvalue); + + $manager = new \local_amos\subscription_manager($USER->id); + $manager->remove_component_subscription($component); + + foreach ($langs as $lang) { + $lang = clean_param($lang, PARAM_ALPHANUMEXT); + $manager->add_subscription($component, $lang); + } + + $manager->apply_changes(); + $subs = $manager->fetch_subscriptions(); + $newlangs = (isset($subs[$component])) ? $subs[$component] : []; + + return \local_amos\local\subscription_table::get_lang_inplace_editable($component, $newlangs); + } +} From 02ec08b56f4ccd17d38bfe667e2042c8071ad522 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 14:56:32 +0200 Subject: [PATCH 21/34] Initialize timesubnotified config --- classes/task/notify_subscribers.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/classes/task/notify_subscribers.php b/classes/task/notify_subscribers.php index 9595e78..b49340c 100644 --- a/classes/task/notify_subscribers.php +++ b/classes/task/notify_subscribers.php @@ -53,6 +53,12 @@ public function execute() { global $DB, $PAGE; $subject = get_string('subscription_mail_subject', 'local_amos'); $lasttimerun = get_config('local_amos', 'timesubnotified'); + // For the very first run, we do not want to send out everything that ever happend. + // So we initialize the config with the date from yesterday. + if (!$lasttimerun) { + $today = strtotime('12:00:00'); + $lasttimerun = strtotime('-1 day', $today); + } $getsql = "SELECT distinct s.userid FROM {amos_commits} c JOIN {amos_repository} r ON (c.id = r.commitid) From a0f5cb87d563d5e5e63baf7053be6ba0b69288ee Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 15:17:43 +0200 Subject: [PATCH 22/34] Add phpunit tests for notify subscribers --- tests/notify_subscribers_test.php | 212 ++++++++++++++++++++++++++---- 1 file changed, 187 insertions(+), 25 deletions(-) diff --git a/tests/notify_subscribers_test.php b/tests/notify_subscribers_test.php index 07a1a01..08c4c74 100644 --- a/tests/notify_subscribers_test.php +++ b/tests/notify_subscribers_test.php @@ -38,76 +38,238 @@ class local_amos_notify_subscribers_testcase extends advanced_testcase { /** - * Test the permission check for the update_strings_file external function. + * Test notifications only print most recent changes. */ - public function test_notifications() { + public function test_notifications_only_most_recent() { global $DB; $this->resetAfterTest(true); $generator = self::getDataGenerator(); - $user1 = $generator->create_user(); - $user2 = $generator->create_user(); - $user3 = $generator->create_user(); + $user1 = $generator->create_user(array('mailformat' => 2)); - $component = new mlang_component('langconfig', 'cs', mlang_version::by_branch('MOODLE_36_STABLE')); - $component->add_string(new mlang_string('thislanguage', 'Čeština')); - $component->add_string(new mlang_string('thislanguageint', 'Czech')); + $today = strtotime('12:00:00'); + $alsotoday = strtotime('11:00:00'); + $past = strtotime('-12 day', $today); + + $component = new mlang_component('admin', 'de', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('pluginname', 'OldString', $past)); $stage = new mlang_stage(); $stage->add($component); - $stage->commit('Registering the Czech language', array('source' => 'bot'), true); + $stage->commit('Add pluginname', array('source' => 'bot'), true); + $component->unlink_string('pluginname'); + $component->add_string(new mlang_string('pluginname', 'IntermediateString',$alsotoday)); + $stage->add($component); + $stage->commit('Alter pluginname', array('source' => 'amos')); + $component->unlink_string('pluginname'); + $component->add_string(new mlang_string('pluginname', 'NewString', $today)); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'amos')); $component->clear(); - $component = new mlang_component('langconfig', 'de', mlang_version::by_branch('MOODLE_36_STABLE')); - $component->add_string(new mlang_string('thislanguage', 'German')); - $component->add_string(new mlang_string('thislanguageint', 'DE')); + // Add a subscription. + $record = new stdClass(); + $record->userid = $user1->id; + $record->lang = 'de'; + $record->component = 'admin'; + $DB->insert_record('amos_subscription', $record); + + $sink = $this->redirectEmails(); + $task = new \local_amos\task\notify_subscribers(); + $task->execute(); + $this->assertCount(1, $sink->get_messages()); + $this->assertContains("OldString", $sink->get_messages()[0]->body); + $this->assertContains("NewString", $sink->get_messages()[0]->body); + $this->assertNotContains("IntermediateString", $sink->get_messages()[0]->body); + $sink->close(); + } + + /** + * Test that multiple changes can be displayed in the notification. + */ + public function test_multiple_subscriptions_one_user() { + global $DB; + $this->resetAfterTest(true); + + $generator = self::getDataGenerator(); + + // Print plain format, since htmlformat creates linebreaks within strings. + $user1 = $generator->create_user(array('mailformat' => 2)); + + $today = strtotime('12:00:00'); + $past = strtotime('-12 day', $today); + + $component = new mlang_component('admin', 'de', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('pluginname', 'OldString', $past)); $stage = new mlang_stage(); $stage->add($component); - $stage->commit('Registering the German language', array('source' => 'amos'), true); + $stage->commit('Add pluginname', array('source' => 'bot'), true); + $component->unlink_string('pluginname'); + $component->add_string(new mlang_string('pluginname', 'NewString', $today)); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'amos')); $component->clear(); - $component = new mlang_component('admin', 'de', mlang_version::by_branch('MOODLE_36_STABLE')); - $component->add_string(new mlang_string('accounts', 'Kontos')); + $component = new mlang_component('theme_boost', 'cs', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('pluginname', 'OldThemeString', $past)); $stage = new mlang_stage(); $stage->add($component); - $stage->commit('Registering the German string form accounts', array('source' => 'amos'), true); + $stage->commit('Add pluginname', array('source' => 'bot'), true); + $component->unlink_string('pluginname'); + $component->add_string(new mlang_string('pluginname', 'NewThemeString', $today)); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'amos')); $component->clear(); + // Add a subscription. + $record = new stdClass(); + $record->userid = $user1->id; + $record->lang = 'de'; + $record->component = 'admin'; + $DB->insert_record('amos_subscription', $record); + // Add a subscription. $record = new stdClass(); $record->userid = $user1->id; $record->lang = 'cs'; - $record->component = 'langconfig'; + $record->component = 'theme_boost'; $DB->insert_record('amos_subscription', $record); + $sink = $this->redirectEmails(); + $task = new \local_amos\task\notify_subscribers(); + $task->execute(); + $this->assertCount(1, $sink->get_messages()); + $this->assertContains("OldThemeString", $sink->get_messages()[0]->body); + $this->assertContains("NewThemeString", $sink->get_messages()[0]->body); + $sink->close(); + } + + /** + * Test that multiple subscriptions to multiple users work. + */ + public function test_multiple_subscriptions_multiple_users() { + global $DB; + $this->resetAfterTest(true); + + $generator = self::getDataGenerator(); + + // Print plain format, since htmlformat creates linebreaks within strings. + $user1 = $generator->create_user(array('mailformat' => 2)); + $user2 = $generator->create_user(array('mailformat' => 2)); + + $today = strtotime('12:00:00'); + $past = strtotime('-12 day', $today); + + $component = new mlang_component('admin', 'de', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('pluginname', 'OldString', $past)); + $stage = new mlang_stage(); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'bot'), true); + $component->unlink_string('pluginname'); + $component->add_string(new mlang_string('pluginname', 'NewString', $today)); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'amos')); + $component->clear(); + + $component = new mlang_component('theme_boost', 'cs', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('pluginname', 'OldThemeString', $past)); + $stage = new mlang_stage(); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'bot'), true); + $component->unlink_string('pluginname'); + $component->add_string(new mlang_string('pluginname', 'NewThemeString', $today)); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'amos')); + $component->clear(); + // Add a subscription. $record = new stdClass(); $record->userid = $user1->id; $record->lang = 'de'; - $record->component = 'langconfig'; + $record->component = 'admin'; $DB->insert_record('amos_subscription', $record); // Add a subscription. $record = new stdClass(); $record->userid = $user2->id; - $record->lang = 'de'; - $record->component = 'admin'; + $record->lang = 'cs'; + $record->component = 'theme_boost'; $DB->insert_record('amos_subscription', $record); - // Add a subscription, which has not changes. + $sink = $this->redirectEmails(); + $task = new \local_amos\task\notify_subscribers(); + $task->execute(); + $this->assertCount(2, $sink->get_messages()); + $sink->close(); + } + + /** + * Test that old and unsubscribed changes do not cause notification. + */ + public function test_no_matching_subscription() { + global $DB; + $this->resetAfterTest(true); + + $generator = self::getDataGenerator(); + + // Print plain format, since htmlformat creates linebreaks within strings. + $user1 = $generator->create_user(array('mailformat' => 2)); + $user2 = $generator->create_user(array('mailformat' => 2)); + + $today = strtotime('12:00:00'); + $past = strtotime('-12 day', $today); + $past2 = strtotime('-12 day', $today); + + // This change is old. + $component = new mlang_component('admin', 'de', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('pluginname', 'OldString', $past)); + $stage = new mlang_stage(); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'bot'), true); + $component->unlink_string('pluginname'); + $component->add_string(new mlang_string('pluginname', 'NewString', $past2)); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'amos')); + $component->clear(); + + // This change is new but not subscribed. + $component = new mlang_component('theme_boost', 'cs', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('pluginname', 'OldThemeString', $past)); + $stage = new mlang_stage(); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'bot'), true); + $component->unlink_string('pluginname'); + $component->add_string(new mlang_string('pluginname', 'NewThemeString', $today)); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'amos')); + $component->clear(); + + // Add a subscription. $record = new stdClass(); - $record->userid = $user3->id; + $record->userid = $user1->id; $record->lang = 'cs'; $record->component = 'admin'; $DB->insert_record('amos_subscription', $record); - set_config('timesubnotified', time() - 86400, 'local_amos'); + // Add a subscription. + $record = new stdClass(); + $record->userid = $user1->id; + $record->lang = 'de'; + $record->component = 'admin'; + $DB->insert_record('amos_subscription', $record); + + // Add a subscription. + $record = new stdClass(); + $record->userid = $user2->id; + $record->lang = 'cs'; + $record->component = 'theme_boost_campus'; + $DB->insert_record('amos_subscription', $record); $sink = $this->redirectEmails(); $task = new \local_amos\task\notify_subscribers(); $task->execute(); - $this->assertCount(2, $sink->get_messages()); - $this->assertContains("thislanguage", $sink->get_messages()[0]->body); + // There should be no + $this->assertCount(0, $sink->get_messages()); $sink->close(); } } From 79d3d55a2520ed09f96a3ec2ccfaf71da8ddb2c6 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 15:20:40 +0200 Subject: [PATCH 23/34] Fix old changes are send out to subscribers as well. --- classes/task/notify_subscribers.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/classes/task/notify_subscribers.php b/classes/task/notify_subscribers.php index b49340c..7d8af67 100644 --- a/classes/task/notify_subscribers.php +++ b/classes/task/notify_subscribers.php @@ -60,11 +60,10 @@ public function execute() { $lasttimerun = strtotime('-1 day', $today); } $getsql = "SELECT distinct s.userid - FROM {amos_commits} c - JOIN {amos_repository} r ON (c.id = r.commitid) + FROM {amos_repository} r JOIN {amos_texts} t ON (r.textid = t.id) JOIN {amos_subscription} s ON (s.lang = r.lang AND s.component = r.component) - WHERE timecommitted > $lasttimerun"; + WHERE r.timemodified > $lasttimerun"; $users = $DB->get_records_sql($getsql); $output = $PAGE->get_renderer('local_amos'); From b45810e06d43defa59d482d7629e5e6b03a53b6b Mon Sep 17 00:00:00 2001 From: Martin Gauk Date: Wed, 11 Sep 2019 15:24:11 +0200 Subject: [PATCH 24/34] Callback to remove user subscriptions when user will be deleted --- lib.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib.php b/lib.php index 7e061f6..72f9fee 100644 --- a/lib.php +++ b/lib.php @@ -210,3 +210,13 @@ function local_amos_inplace_editable($itemtype, $itemid, $newvalue) { return \local_amos\local\subscription_table::get_lang_inplace_editable($component, $newlangs); } } + +/** + * Callback to remove user subscriptions when user will be deleted. + * + * @param $user + */ +function local_amos_pre_user_delete($user) { + $manager = new \local_amos\subscription_manager($user->id); + $manager->remove_all_subscriptions(); +} From cbec74b23217dd26de89f41532c6c9f973389006 Mon Sep 17 00:00:00 2001 From: Kathrin Osswald Date: Wed, 11 Sep 2019 15:25:01 +0200 Subject: [PATCH 25/34] Changed hardcoded strings to lang strings. --- lang/en/local_amos.php | 5 +++++ templates/subscription_notification_html.mustache | 12 +++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lang/en/local_amos.php b/lang/en/local_amos.php index 25a6bc0..47df770 100644 --- a/lang/en/local_amos.php +++ b/lang/en/local_amos.php @@ -199,6 +199,8 @@ $string['diffstrings'] = 'Compare strings at two branches'; $string['diffstrings_help'] = 'This will compare all strings at the two selected branches. If there is difference in strings on the branches, both versions are staged. You can then use "Edit staged strings" feature to review and fix the changes as needed.'; $string['diffversions'] = 'Versions'; +$string['emailintroduction'] = 'Dear {$a->firstname} {$a->lastname},
+ the following string changes were made in AMOS for your subscripted plugins:'; $string['err_exception'] = 'Error: {$a}'; $string['err_invalidlangcode'] = 'Invalid language code'; $string['err_parser'] = 'Parsing error: {$a}'; @@ -291,6 +293,7 @@ $string['messageprovider:contribution'] = 'Contributed translations'; $string['morefilteringoptions'] = 'More options'; $string['newlanguage'] = 'New language'; +$string['newstring'] = 'New string'; $string['nodiffs'] = 'No differences found'; $string['nofiletoimport'] = 'Please provide a file to import from.'; $string['nologsfound'] = 'No strings found, please modify filters'; @@ -303,6 +306,7 @@ $string['numofcommitsabovelimit'] = 'Found {$a->found} commits matching the commit filter, using {$a->limit} most recent'; $string['numofcommitsunderlimit'] = 'Found {$a->found} commits matching the commit filter'; $string['numofmatchingstrings'] = 'Within that, {$a->strings} modifications in {$a->commits} commits match the string filter'; +$string['oldstring'] = 'Old string'; $string['outdatednotcommitted'] = 'Outdated string'; $string['outdatednotcommitted_help'] = 'AMOS detected that the string may be outdated as the English version was modified after it had been translated. Please review the translation.'; $string['outdatednotcommittedwarning'] = 'Outdated'; @@ -450,6 +454,7 @@ $string['stashtitle'] = 'Stash title'; $string['stashtitledefault'] = 'Work in progress - {$a->time}'; $string['stringhistory'] = 'History'; +$string['stringidentifier'] = 'String identifier'; $string['strings'] = 'Strings'; $string['submitting'] = 'Submitting a contribution'; $string['submitting_help'] = 'This will send translated strings to official language maintainers. They will be able to apply your work into their stage, review it and eventually commit. Please provide a message for them describing your work and why you would like to see your contribution included.'; diff --git a/templates/subscription_notification_html.mustache b/templates/subscription_notification_html.mustache index f26e725..8436455 100644 --- a/templates/subscription_notification_html.mustache +++ b/templates/subscription_notification_html.mustache @@ -43,18 +43,17 @@ }} -

Dear {{user.firstname}} {{user.lastname}},
- the following string changes were made in AMOS for your subscripted plugins:

+

{{# str }} stringidentifier, local_amos, { "firstname": {{user.firstname}}, "lastname": {{user.lastname}} }{{/ str }}

{{#components}}

{{{name}}} ({{{lang}}}):

- - - - + + + + {{#changes}} @@ -75,4 +74,3 @@
String identifierVersionOld valueNew value{{# str }} stringidentifier, local_amos {{/ str }}{{# str }} version, local_amos {{/ str }}{{# str }} oldstring, local_amos {{/ str }}{{# str }} newstring, local_amos {{/ str }}
{{/components}}
- From 914453295446c08b7d140d85b54b1cf9910a6c84 Mon Sep 17 00:00:00 2001 From: Kathrin Osswald Date: Wed, 11 Sep 2019 15:31:30 +0200 Subject: [PATCH 26/34] Fixed wrong string reference. --- templates/subscription_notification_html.mustache | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/subscription_notification_html.mustache b/templates/subscription_notification_html.mustache index 8436455..60908dc 100644 --- a/templates/subscription_notification_html.mustache +++ b/templates/subscription_notification_html.mustache @@ -43,7 +43,7 @@ }} -

{{# str }} stringidentifier, local_amos, { "firstname": {{user.firstname}}, "lastname": {{user.lastname}} }{{/ str }}

+

{{# str }} emailintroduction, local_amos, { "firstname": {{user.firstname}}, "lastname": {{user.lastname}} }{{/ str }}

{{#components}} From 052b40239eb689fad3204fb790730f3cd994d1bb Mon Sep 17 00:00:00 2001 From: Kathrin Osswald Date: Wed, 11 Sep 2019 15:37:18 +0200 Subject: [PATCH 27/34] Hopefully fixed missing variable replacement. --- templates/subscription_notification_html.mustache | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/subscription_notification_html.mustache b/templates/subscription_notification_html.mustache index 60908dc..5b5b396 100644 --- a/templates/subscription_notification_html.mustache +++ b/templates/subscription_notification_html.mustache @@ -43,7 +43,7 @@ }} -

{{# str }} emailintroduction, local_amos, { "firstname": {{user.firstname}}, "lastname": {{user.lastname}} }{{/ str }}

+

{{# str }} emailintroduction, local_amos, { "firstname": "{{user.firstname}}", "lastname": "{{user.lastname}}" }{{/ str }}

{{#components}} From 702bef27ccfb64a9de3d2afa7da00cf0cc4e0f7b Mon Sep 17 00:00:00 2001 From: Jan Eberhardt Date: Wed, 11 Sep 2019 15:54:41 +0200 Subject: [PATCH 28/34] Table UI ready (still needs some improvements) Changes to be committed: modified: classes/local/subscription_table.php modified: lang/en/local_amos.php modified: renderer.php modified: subscription.php --- classes/local/subscription_table.php | 61 +++++++++++++++------ lang/en/local_amos.php | 4 ++ renderer.php | 1 + subscription.php | 81 ++++++++++++++++++++++++---- 4 files changed, 121 insertions(+), 26 deletions(-) diff --git a/classes/local/subscription_table.php b/classes/local/subscription_table.php index 861da70..75deb68 100644 --- a/classes/local/subscription_table.php +++ b/classes/local/subscription_table.php @@ -20,6 +20,7 @@ * View your subscription and change settings * * @package local_amos + * @tags MoodleMootDACH2019 * @copyright 2019 Tobias Reischmann * @copyright 2019 Martin Gauk * @copyright 2019 Jan Eberhardt @@ -39,49 +40,73 @@ require_once $CFG->dirroot . '/local/amos/classes/subscription_manager.php'; require_once $CFG->dirroot . '/local/amos/mlanglib.php'; -class subscription_table extends \table_sql { +class subscription_table extends \flexible_table { - public function __construct($uniqueid) + public function init($baseurl, $pagesize = -1) { global $USER; - parent::__construct($uniqueid); + $manager = new subscription_manager($USER->id); + $subs = $manager->fetch_subscriptions(); + $this->define_baseurl($baseurl); $this->define_columns([ 'component', - 'lang' + 'language' ]); $this->define_headers([ get_string('component', 'local_amos'), get_string('languages', 'local_amos') ]); - + $this->pagesize($pagesize > 0 ? $pagesize : $this->pagesize, count($subs)); + $this->sortable(true, 'component'); + $this->no_sorting('language'); + $this->setup(); } - public function query_db($pagesize, $useinitialsbar = true) - { + public function out() { global $USER; - $manager = new subscription_manager($USER->id); $subs = $manager->fetch_subscriptions(); - - $this->pagesize($pagesize, count($subs)); + $sort = $this->get_sort_columns()['component']; + if ($sort === SORT_ASC) { + ksort($subs); + } else { + krsort($subs); + } + $this->start_output(); foreach ($subs as $component => $langarray) { - $this->rawdata[] = [ + $row = $this->format_row([ 'component' => $component, - 'lang' => $langarray - ]; + 'language' => $langarray + ]); + $this->add_data_keyed($row); } + $this->finish_output(); + } + + public function __construct($uniqueid) + { + parent::__construct($uniqueid); + + } public function col_component($sub) { - return $sub->component; + global $PAGE; + $icon = $PAGE->get_renderer('local_amos')->pix_icon( + 't/delete', + get_string('unsubscribe', 'local_amos') + ); + $query_string = sprintf('?c=%s&m=unsubscribe', $sub->component); + $link = \html_writer::link($this->baseurl . $query_string, $icon, ['class' => 'unsubscribe']); + return sprintf('%s %s', $sub->component, $link); } - public function col_lang($sub) { + public function col_language($sub) { global $OUTPUT; - $inplace = self::get_lang_inplace_editable($sub->component, $sub->lang); + $inplace = self::get_lang_inplace_editable($sub->component, $sub->language); return $OUTPUT->render_from_template('core/inplace_editable', $inplace->export_for_template($OUTPUT)); } @@ -90,7 +115,9 @@ static public function get_lang_inplace_editable(string $component, array $langs $values = json_encode($langs); $displayvalues = implode(', ', array_map(function($lang) use ($options) { - return (isset($options[$lang])) ? $options[$lang] : ''; + return (isset($options[$lang])) + ? $options[$lang] + : get_string('unknown_language', 'local_amos', $lang); }, $langs)); $inplace = new \core\output\inplace_editable('local_amos', 'subscription', $component, diff --git a/lang/en/local_amos.php b/lang/en/local_amos.php index 47df770..0ee1be7 100644 --- a/lang/en/local_amos.php +++ b/lang/en/local_amos.php @@ -458,6 +458,7 @@ $string['strings'] = 'Strings'; $string['submitting'] = 'Submitting a contribution'; $string['submitting_help'] = 'This will send translated strings to official language maintainers. They will be able to apply your work into their stage, review it and eventually commit. Please provide a message for them describing your work and why you would like to see your contribution included.'; +$string['subscribe_info'] = 'You have been subscribed successfully'; $string['subscription'] = 'Subscription'; $string['subscription_mail_subject'] = 'AMOS Notification - New language pack changes'; $string['targetversion'] = 'Target version'; @@ -481,9 +482,12 @@ $string['unableenfixaddon'] = 'English fixes allowed for standard plugins only'; $string['unableenfixcountries'] = 'Country names are copied from ISO 3166-1'; $string['unableunmaintained'] = 'The language pack \'{$a}\' has no maintainer at the moment, so translation contributions cannot be accepted. Please consider volunteering to become \'{$a}\' language pack maintainer.'; +$string['unknown_language'] = 'Unknown language ({$a})'; $string['unstage'] = 'Unstage'; $string['unstageconfirm'] = 'Really?'; $string['unstaging'] = 'Unstaging'; +$string['unsubscribe'] = 'Unsubscribe'; +$string['unsubscribe_info'] = 'You have been unsubscribed successfully'; $string['untranslate'] = 'untranslate'; $string['untranslateconfirm'] = '

You are going to remove existing translation of the string {$a->stringid}, component {$a->component}, from all the versions of the language pack {$a->language}.

Are you sure?

'; $string['untranslatetitle'] = 'Removing translation from the language pack'; diff --git a/renderer.php b/renderer.php index c5a373e..190b762 100644 --- a/renderer.php +++ b/renderer.php @@ -47,6 +47,7 @@ protected function render_local_amos_subscription_filter(local_amos_subscription $alerts = []; $defaults = (object) ['language' => [], 'component' => [], 'last' => true]; $output .= $this->render_local_amos_filter_basic($defaults, $alerts, false, false); + $output .= html_writer::empty_tag('input', ['type' => 'hidden', 'value' => 'subscribe', 'name' => 'm']); // close form $output .= html_writer::end_tag('fieldset'); diff --git a/subscription.php b/subscription.php index 6bd541a..d14dd42 100644 --- a/subscription.php +++ b/subscription.php @@ -25,6 +25,8 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +use local_amos\subscription_manager; + require_once(dirname(dirname(dirname(__FILE__))).'/config.php'); require_once(dirname(__FILE__).'/locallib.php'); require_once(dirname(__FILE__).'/mlanglib.php'); @@ -32,25 +34,86 @@ require_login(SITEID, false); #$name = optional_param('name', null, PARAM_RAW); // stash name -$mode = optional_param('m', null, PARAM_INT); -$component = optional_param('c', null, PARAM_ALPHAEXT); -$lang = optional_param('l', null, PARAM_ALPHAEXT); - +$mode = optional_param('m', null, PARAM_ALPHA); $PAGE->set_pagelayout('standard'); $PAGE->set_url('/local/amos/subscription.php'); $PAGE->set_title('AMOS ' . get_string('subscription', 'local_amos')); $PAGE->set_heading('AMOS ' . get_string('subscription', 'local_amos')); $PAGE->requires->strings_for_js(array('processing', 'googletranslate'), 'local_amos'); $PAGE->requires->yui_module('moodle-local_amos-filter', 'M.local_amos.init_filter', null, null, true); -#$PAGE->requires->yui_module('moodle-local_amos-translator', 'M.local_amos.init_translator', null, null, true); $PAGE->requires->yui_module('moodle-local_amos-timeline', 'M.local_amos.init_timeline', null, null, true); +$output = $PAGE->get_renderer('local_amos'); +echo $output->header(); + +# add subscription +if ($mode === 'subscribe') { + $languages = optional_param_array('flng', null, PARAM_ALPHA); + $components = optional_param_array('fcmp', null, PARAM_ALPHAEXT); + $error = false; + if (empty($components)) { + echo html_writer::div( + get_string('filtercmpnothingselected', 'local_amos'), + 'alert alert-danger', + ['role' => 'alert'] + ); + $error = true; + } + if (empty($languages)) { + echo html_writer::div( + get_string('filterlngnothingselected', 'local_amos'), + 'alert alert-danger', + ['role' => 'alert'] + ); + $error = true; + } + if (!$error) { + $subscribed = false; + $manager = new subscription_manager($USER->id); + $clist = array_keys(mlang_tools::list_components()); + $llist = array_keys(mlang_tools::list_languages()); + foreach ($components as $cmp) { + if (in_array($cmp, $clist)) { + foreach ($languages as $lang) { + if (in_array($lang, $llist)) { + $manager->add_subscription($cmp, $lang); + $subscribed = true; + } + } + } + } + $manager->apply_changes(); + if ($subscribed) { + echo html_writer::div( + get_string('subscribed_info', 'amos_local'), 'alert alert-success', ['role' => 'alert']); + } + } +} + +if ($mode === 'unsubscribe') { + $language = optional_param('l', null, PARAM_ALPHAEXT); + $component = optional_param('c', null, PARAM_ALPHAEXT); + if (!empty($component)) { + $manager = new subscription_manager($USER->id); + if (empty($language)) { + $manager->remove_component_subscription($component); + + } else { + $manager->remove_subscription($component, $language); + } + $manager->apply_changes(); + echo html_writer::div( + get_string('unsubscribe_info', 'local_amos'), + 'alert alert-success', + ['role' => 'alert'] + ); + } +} $filter = new local_amos_subscription_filter($PAGE->url); -$output = $PAGE->get_renderer('local_amos'); $table = new local_amos\local\subscription_table('subscription_table'); -$table->define_baseurl($PAGE->url); -echo $output->header(); +$table->init($PAGE->url, 40); + echo $output->render($filter); -echo $table->out(40, true); +$table->out(); echo $output->footer(); \ No newline at end of file From 9fbe9f1cc2b61c7187eaf6d6cab785caea975e68 Mon Sep 17 00:00:00 2001 From: Jan Eberhardt Date: Wed, 11 Sep 2019 16:02:11 +0200 Subject: [PATCH 29/34] Bugfix langstring Changes to be committed: modified: classes/local/subscription_table.php modified: subscription.php --- classes/local/subscription_table.php | 7 ------- subscription.php | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/classes/local/subscription_table.php b/classes/local/subscription_table.php index 75deb68..a80dbfb 100644 --- a/classes/local/subscription_table.php +++ b/classes/local/subscription_table.php @@ -85,13 +85,6 @@ public function out() { $this->finish_output(); } - public function __construct($uniqueid) - { - parent::__construct($uniqueid); - - - } - public function col_component($sub) { global $PAGE; $icon = $PAGE->get_renderer('local_amos')->pix_icon( diff --git a/subscription.php b/subscription.php index d14dd42..a859e23 100644 --- a/subscription.php +++ b/subscription.php @@ -84,7 +84,7 @@ $manager->apply_changes(); if ($subscribed) { echo html_writer::div( - get_string('subscribed_info', 'amos_local'), 'alert alert-success', ['role' => 'alert']); + get_string('subscribe_info', 'amos_local'), 'alert alert-success', ['role' => 'alert']); } } } From 7a81276a007dc095736aa2d7517746ffefc55d8e Mon Sep 17 00:00:00 2001 From: Martin Gauk Date: Wed, 11 Sep 2019 16:07:38 +0200 Subject: [PATCH 30/34] Check component with isset --- subscription.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/subscription.php b/subscription.php index a859e23..39385f9 100644 --- a/subscription.php +++ b/subscription.php @@ -69,12 +69,12 @@ if (!$error) { $subscribed = false; $manager = new subscription_manager($USER->id); - $clist = array_keys(mlang_tools::list_components()); - $llist = array_keys(mlang_tools::list_languages()); + $clist = mlang_tools::list_components(); + $llist = mlang_tools::list_languages(); foreach ($components as $cmp) { - if (in_array($cmp, $clist)) { + if (isset($clist[$cmp])) { foreach ($languages as $lang) { - if (in_array($lang, $llist)) { + if (isset($llist[$lang])) { $manager->add_subscription($cmp, $lang); $subscribed = true; } @@ -84,7 +84,7 @@ $manager->apply_changes(); if ($subscribed) { echo html_writer::div( - get_string('subscribe_info', 'amos_local'), 'alert alert-success', ['role' => 'alert']); + get_string('subscribe_info', 'local_amos'), 'alert alert-success', ['role' => 'alert']); } } } From f5ca19812d3fb5f390c2fe4756c12027c447ade6 Mon Sep 17 00:00:00 2001 From: Martin Gauk Date: Wed, 11 Sep 2019 16:07:57 +0200 Subject: [PATCH 31/34] Documentation --- lib.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib.php b/lib.php index 72f9fee..a533ac8 100644 --- a/lib.php +++ b/lib.php @@ -186,6 +186,13 @@ function local_amos_comment_template($params) { return $template; } +/** + * Callback to update a value in an inplace_editable. + * + * @param $itemtype + * @param $itemid + * @param $newvalue + */ function local_amos_inplace_editable($itemtype, $itemid, $newvalue) { global $USER; From 296c7614b06cf21d58f6316902bbe51f985b1033 Mon Sep 17 00:00:00 2001 From: Martin Gauk Date: Wed, 11 Sep 2019 16:08:25 +0200 Subject: [PATCH 32/34] Add more asserts to subscription manager test --- tests/subscription_manager_test.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/subscription_manager_test.php b/tests/subscription_manager_test.php index 3a75938..8def37e 100644 --- a/tests/subscription_manager_test.php +++ b/tests/subscription_manager_test.php @@ -86,6 +86,13 @@ public function test_subscriptions() { $this->assertTrue(in_array('fr', $subs['langconfig'])); $this->assertTrue(in_array('cz', $subs['langconfig'])); $this->assertFalse(in_array('aa', $subs['langconfig'])); + $this->assertEquals(3, count($subs['langconfig'])); + + // Try to add one language again. + $manager->add_subscription('langconfig', 'de'); + $manager->apply_changes(); + $subs = $manager->fetch_subscriptions(); + $this->assertEquals(3, count($subs['langconfig'])); // Remove one language. $manager->remove_subscription('langconfig', 'de'); From 9aa6bed60eb83eaa43f97e996754a3e58071b323 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 16:26:10 +0200 Subject: [PATCH 33/34] Use timelastrun as a parameter for sql query and also for innersql --- classes/task/notify_subscribers.php | 13 ++++++------- locallib.php | 7 +++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/classes/task/notify_subscribers.php b/classes/task/notify_subscribers.php index 7d8af67..c0ddd00 100644 --- a/classes/task/notify_subscribers.php +++ b/classes/task/notify_subscribers.php @@ -56,22 +56,21 @@ public function execute() { // For the very first run, we do not want to send out everything that ever happend. // So we initialize the config with the date from yesterday. if (!$lasttimerun) { - $today = strtotime('12:00:00'); - $lasttimerun = strtotime('-1 day', $today); + $today = strtotime('12:00:00'); + $lasttimerun = strtotime('-1 day', $today); } $getsql = "SELECT distinct s.userid FROM {amos_repository} r - JOIN {amos_texts} t ON (r.textid = t.id) JOIN {amos_subscription} s ON (s.lang = r.lang AND s.component = r.component) - WHERE r.timemodified > $lasttimerun"; + WHERE r.timemodified > ?"; - $users = $DB->get_records_sql($getsql); + $users = $DB->get_records_sql($getsql, array($lasttimerun)); $output = $PAGE->get_renderer('local_amos'); foreach ($users as $user) { $user = \core_user::get_user($user->userid); if ($user) { - $notification_html = new \local_amos_sub_notification($user, true); - $notification = new \local_amos_sub_notification($user); + $notification_html = new \local_amos_sub_notification($user, $lasttimerun, true); + $notification = new \local_amos_sub_notification($user, $lasttimerun); $content_html = $output->render($notification_html); $content = $output->render($notification); diff --git a/locallib.php b/locallib.php index 1676b86..e328ba0 100644 --- a/locallib.php +++ b/locallib.php @@ -1545,13 +1545,12 @@ class local_amos_sub_notification implements renderable, templatable { * Fetches the required commits from the repository * * @param stdClass $user id of the subscriber + * @param int $since timestamp since when changes should be returned. * @param bool $html States if the notification should be */ - public function __construct($user, $html = false) { + public function __construct($user, $since, $html = false) { global $DB; - $time = time() - 86400; - // Get all unique combinations of stringid, component, lang and branch. $getsqlrelevantstrings = "SELECT r.stringid, r.component, @@ -1622,7 +1621,7 @@ public function __construct($user, $html = false) { innersql.newtextid = newtext.id LEFT JOIN {amos_texts} oldtext ON innersql.oldtextid = oldtext.id"; - $recordset = $DB->get_recordset_sql($getsql, array($time, $time)); + $recordset = $DB->get_recordset_sql($getsql, array($since, $since)); // Build the data format for the mustach template. foreach ($recordset as $record) { From b26d515c884e2864a847657b0e9142d8bdc5e1c4 Mon Sep 17 00:00:00 2001 From: Tobias Reischmann Date: Wed, 11 Sep 2019 16:26:49 +0200 Subject: [PATCH 34/34] Added tests for notify_subscribers task --- tests/notify_subscribers_test.php | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/notify_subscribers_test.php b/tests/notify_subscribers_test.php index 08c4c74..58ac0b7 100644 --- a/tests/notify_subscribers_test.php +++ b/tests/notify_subscribers_test.php @@ -272,4 +272,62 @@ public function test_no_matching_subscription() { $this->assertCount(0, $sink->get_messages()); $sink->close(); } + + /** + * Test that old and unsubscribed changes do not cause notification. + */ + public function test_only_subscriptions_since_lastrun() { + global $DB; + $this->resetAfterTest(true); + + $generator = self::getDataGenerator(); + + // Print plain format, since htmlformat creates linebreaks within strings. + $user1 = $generator->create_user(array('mailformat' => 2)); + + $today = strtotime('12:00:00'); + $yesterday = strtotime('-1 day', $today); + $past = strtotime('-12 day', $today); + + // This change is old. + $component = new mlang_component('admin', 'de', mlang_version::by_branch('MOODLE_36_STABLE')); + $component->add_string(new mlang_string('pluginname', 'OldPluginString', $past)); + $stage = new mlang_stage(); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'bot'), true); + $component->unlink_string('pluginname'); + $component->add_string(new mlang_string('pluginname', 'NewPluginString', $yesterday)); + $component->add_string(new mlang_string('mystring', 'NewString', $today)); + $stage->add($component); + $stage->commit('Add pluginname', array('source' => 'amos')); + $component->clear(); + + // Add a subscription. + $record = new stdClass(); + $record->userid = $user1->id; + $record->lang = 'de'; + $record->component = 'admin'; + $DB->insert_record('amos_subscription', $record); + + $sink = $this->redirectEmails(); + $task = new \local_amos\task\notify_subscribers(); + $task->execute(); + // There should be no + $this->assertCount(1, $sink->get_messages()); + $this->assertContains("NewString", $sink->get_messages()[0]->body); + $this->assertNotContains("NewPluginString", $sink->get_messages()[0]->body); + $sink->close(); + + $daybeforeyesterday = strtotime('-2 day', $today); + set_config('timesubnotified', $daybeforeyesterday, 'local_amos'); + + $sink = $this->redirectEmails(); + $task = new \local_amos\task\notify_subscribers(); + $task->execute(); + // There should be no + $this->assertCount(1, $sink->get_messages()); + $this->assertContains("NewString", $sink->get_messages()[0]->body); + $this->assertContains("NewPluginString", $sink->get_messages()[0]->body); + $sink->close(); + } }