MDL-62554 tool_dataprivacy: Integration review fixes
authorJun Pataleta <jun@moodle.com>
Tue, 11 Sep 2018 08:57:40 +0000 (16:57 +0800)
committerJun Pataleta <jun@moodle.com>
Thu, 13 Sep 2018 06:07:25 +0000 (14:07 +0800)
admin/tool/dataprivacy/classes/api.php
admin/tool/dataprivacy/classes/data_registry.php
admin/tool/dataprivacy/classes/external.php
admin/tool/dataprivacy/classes/external/category_exporter.php
admin/tool/dataprivacy/classes/external/purpose_exporter.php
admin/tool/dataprivacy/classes/local/helper.php
admin/tool/dataprivacy/classes/output/data_registry_page.php
admin/tool/dataprivacy/classes/output/defaults_page.php
admin/tool/dataprivacy/defaults.php
admin/tool/dataprivacy/styles.css
admin/tool/dataprivacy/templates/defaults_page.mustache

index 8e89c23..7428152 100644 (file)
@@ -1216,11 +1216,13 @@ class api {
             $sql = implode("\n", $statements);
 
             // Get the context records matching our query.
-            $contextinstances = $DB->get_records_sql($sql, $params);
+            $contextids = $DB->get_fieldset_sql($sql, $params);
 
-            // Delete the matching context instances by passing an instance record without the purpose and instance.
-            foreach ($contextinstances as $instance) {
-                self::set_context_instance($instance);
+            // Delete the matching context instances.
+            foreach ($contextids as $contextid) {
+                if ($instance = context_instance::get_record_by_contextid($contextid, false)) {
+                    self::unset_context_instance($instance);
+                }
             }
         }
 
index 5f1fc4e..7b46b4d 100644 (file)
@@ -309,8 +309,8 @@ class data_registry {
      * Returns the effective default purpose and category for a context level.
      *
      * @param int $contextlevel
-     * @param bool $forcedpurposevalue Use this value as if this was this context level purpose.
-     * @param bool $forcedcategoryvalue Use this value as if this was this context level category.
+     * @param int|bool $forcedpurposevalue Use this value as if this was this context level purpose.
+     * @param int|bool $forcedcategoryvalue Use this value as if this was this context level category.
      * @param string $activity The plugin name of the activity.
      * @return int[]
      */
index ff8ce80..692e072 100644 (file)
@@ -1117,7 +1117,7 @@ class external extends external_api {
      */
     public static function set_context_defaults_parameters() {
         return new external_function_parameters([
-            'contextlevel' => new external_value(PARAM_INT, 'Expired context record ID', VALUE_REQUIRED),
+            'contextlevel' => new external_value(PARAM_INT, 'The context level', VALUE_REQUIRED),
             'category' => new external_value(PARAM_INT, 'The default category for the given context level', VALUE_REQUIRED),
             'purpose' => new external_value(PARAM_INT, 'The default purpose for the given context level', VALUE_REQUIRED),
             'activity' => new external_value(PARAM_PLUGIN, 'The plugin name of the activity', VALUE_DEFAULT, null),
@@ -1210,23 +1210,12 @@ class external extends external_api {
         self::validate_context($context);
 
         $categories = api::get_categories();
+        $options = data_registry_page::category_options($categories, $includenotset, $includeinherit);
         $categoryoptions = [];
-        if ($includeinherit) {
+        foreach ($options as $id => $name) {
             $categoryoptions[] = [
-                'id' => context_instance::INHERIT,
-                'name' => get_string('inherit', 'tool_dataprivacy'),
-            ];
-        }
-        if ($includenotset) {
-            $categoryoptions[] = [
-                'id' => context_instance::NOTSET,
-                'name' => get_string('notset', 'tool_dataprivacy'),
-            ];
-        }
-        foreach ($categories as $category) {
-            $categoryoptions[] = [
-                'id' => $category->get('id'),
-                'name' => $category->get('name'),
+                'id' => $id,
+                'name' => $name,
             ];
         }
 
@@ -1288,23 +1277,12 @@ class external extends external_api {
         self::validate_context($context);
 
         $purposes = api::get_purposes();
+        $options = data_registry_page::purpose_options($purposes, $includenotset, $includeinherit);
         $purposeoptions = [];
-        if ($includeinherit) {
-            $purposeoptions[] = [
-                'id' => context_instance::INHERIT,
-                'name' => get_string('inherit', 'tool_dataprivacy'),
-            ];
-        }
-        if ($includenotset) {
-            $purposeoptions[] = [
-                'id' => context_instance::NOTSET,
-                'name' => get_string('notset', 'tool_dataprivacy'),
-            ];
-        }
-        foreach ($purposes as $purpose) {
+        foreach ($options as $id => $name) {
             $purposeoptions[] = [
-                'id' => $purpose->get('id'),
-                'name' => $purpose->get('name'),
+                'id' => $id,
+                'name' => $name,
             ];
         }
 
@@ -1365,15 +1343,15 @@ class external extends external_api {
 
         // Get activity module plugin info.
         $pluginmanager = \core_plugin_manager::instance();
-        $modplugins = $pluginmanager->get_plugins_of_type('mod');
+        $modplugins = $pluginmanager->get_enabled_plugins('mod');
         $modoptions = [];
 
         // Get the module-level defaults. data_registry::get_defaults falls back to this when there are no activity defaults.
         list($levelpurpose, $levelcategory) = data_registry::get_defaults(CONTEXT_MODULE);
-        foreach ($modplugins as $plugin) {
+        foreach ($modplugins as $name) {
             // Check if we have default purpose and category for this module if we want don't want to fetch everything.
             if ($nodefaults) {
-                list($purpose, $category) = data_registry::get_defaults(CONTEXT_MODULE, $plugin->name);
+                list($purpose, $category) = data_registry::get_defaults(CONTEXT_MODULE, $name);
                 // Compare this with the module-level defaults.
                 if ($purpose !== $levelpurpose || $category !== $levelcategory) {
                     // If the defaults for this activity has been already set, there's no need to add this in the list of options.
@@ -1381,9 +1359,10 @@ class external extends external_api {
                 }
             }
 
+            $displayname = $pluginmanager->plugin_name('mod_' . $name);
             $modoptions[] = (object)[
-                'name' => $plugin->name,
-                'displayname' => $plugin->displayname
+                'name' => $name,
+                'displayname' => $displayname
             ];
         }
 
index c5c350c..06d2469 100644 (file)
@@ -25,6 +25,8 @@ namespace tool_dataprivacy\external;
 defined('MOODLE_INTERNAL') || die();
 
 use core\external\persistent_exporter;
+use tool_dataprivacy\category;
+use tool_dataprivacy\context_instance;
 
 /**
  * Class for exporting field data.
@@ -53,4 +55,25 @@ class category_exporter extends persistent_exporter {
             'context' => 'context',
         );
     }
+
+    /**
+     * Utility function that fetches a category name from the given ID.
+     *
+     * @param int $categoryid The category ID. Could be INHERIT (false, -1), NOT_SET (0), or the actual ID.
+     * @return string The purpose name.
+     */
+    public static function get_name($categoryid) {
+        global $PAGE;
+        if ($categoryid === false || $categoryid == context_instance::INHERIT) {
+            return get_string('inherit', 'tool_dataprivacy');
+        } else if ($categoryid == context_instance::NOTSET) {
+            return get_string('notset', 'tool_dataprivacy');
+        } else {
+            $purpose = new category($categoryid);
+            $output = $PAGE->get_renderer('tool_dataprivacy');
+            $exporter = new self($purpose, ['context' => \context_system::instance()]);
+            $data = $exporter->export($output);
+            return $data->name;
+        }
+    }
 }
index f91e2f2..a6519c6 100644 (file)
@@ -29,6 +29,7 @@ use core\external\persistent_exporter;
 use DateInterval;
 use Exception;
 use renderer_base;
+use tool_dataprivacy\context_instance;
 use tool_dataprivacy\purpose;
 
 /**
@@ -143,4 +144,25 @@ class purpose_exporter extends persistent_exporter {
 
         return $values;
     }
+
+    /**
+     * Utility function that fetches a purpose name from the given ID.
+     *
+     * @param int $purposeid The purpose ID. Could be INHERIT (false, -1), NOT_SET (0), or the actual ID.
+     * @return string The purpose name.
+     */
+    public static function get_name($purposeid) {
+        global $PAGE;
+        if ($purposeid === false || $purposeid == context_instance::INHERIT) {
+            return get_string('inherit', 'tool_dataprivacy');
+        } else if ($purposeid == context_instance::NOTSET) {
+            return get_string('notset', 'tool_dataprivacy');
+        } else {
+            $purpose = new purpose($purposeid);
+            $output = $PAGE->get_renderer('tool_dataprivacy');
+            $exporter = new self($purpose, ['context' => \context_system::instance()]);
+            $data = $exporter->export($output);
+            return $data->name;
+        }
+    }
 }
index d196008..36dd93a 100644 (file)
@@ -27,9 +27,6 @@ defined('MOODLE_INTERNAL') || die();
 use coding_exception;
 use moodle_exception;
 use tool_dataprivacy\api;
-use tool_dataprivacy\category;
-use tool_dataprivacy\context_instance;
-use tool_dataprivacy\purpose;
 
 /**
  * Class containing helper functions for the data privacy tool.
@@ -212,38 +209,4 @@ class helper {
         }
         return $options;
     }
-
-    /**
-     * Fetches a category name from the given ID.
-     *
-     * @param int $category The category ID. Could be INHERIT (false, -1), NOT_SET (0), or the actual ID.
-     * @return string The category name.
-     */
-    public static function get_category_name($category) {
-        if ($category === false || $category == context_instance::INHERIT) {
-            return get_string('inherit', 'tool_dataprivacy');
-        } else if ($category == context_instance::NOTSET) {
-            return get_string('notset', 'tool_dataprivacy');
-        } else {
-            $category = new category($category);
-            return format_string($category->get('name'));
-        }
-    }
-
-    /**
-     * Fetches a purpose name from the given ID.
-     *
-     * @param int $purpose The purpose ID. Could be INHERIT (false, -1), NOT_SET (0), or the actual ID.
-     * @return string The purpose name.
-     */
-    public static function get_purpose_name($purpose) {
-        if ($purpose === false || $purpose == context_instance::INHERIT) {
-            return get_string('inherit', 'tool_dataprivacy');
-        } else if ($purpose == context_instance::NOTSET) {
-            return get_string('notset', 'tool_dataprivacy');
-        } else {
-            $purpose = new purpose($purpose);
-            return format_string($purpose->get('name'));
-        }
-    }
 }
index 8d5fd35..be0e996 100644 (file)
@@ -425,7 +425,7 @@ class data_registry_page implements renderable, templatable {
     /**
      * From a list of purpose persistents to a list of id => name purposes.
      *
-     * @param \tool_dataprivacy\purpose $purposes
+     * @param \tool_dataprivacy\purpose[] $purposes
      * @param bool $includenotset
      * @param bool $includeinherit
      * @return string[]
@@ -442,7 +442,7 @@ class data_registry_page implements renderable, templatable {
     /**
      * From a list of category persistents to a list of id => name categories.
      *
-     * @param \tool_dataprivacy\category $categories
+     * @param \tool_dataprivacy\category[] $categories
      * @param bool $includenotset
      * @param bool $includeinherit
      * @return string[]
index 16907dd..166228f 100644 (file)
@@ -24,6 +24,7 @@
 namespace tool_dataprivacy\output;
 defined('MOODLE_INTERNAL') || die();
 
+use action_menu_link_primary;
 use coding_exception;
 use moodle_exception;
 use moodle_url;
@@ -32,7 +33,8 @@ use renderer_base;
 use stdClass;
 use templatable;
 use tool_dataprivacy\data_registry;
-use tool_dataprivacy\local\helper;
+use tool_dataprivacy\external\category_exporter;
+use tool_dataprivacy\external\purpose_exporter;
 
 /**
  * Class containing data for the data registry defaults.
@@ -122,23 +124,48 @@ class defaults_page implements renderable, templatable {
 
         // Set default category.
         $data->categoryid = $this->category;
-        $data->category = helper::get_category_name($this->category);
+        $data->category = category_exporter::get_name($this->category);
 
         // Set default purpose.
         $data->purposeid = $this->purpose;
-        $data->purpose = helper::get_purpose_name($this->purpose);
+        $data->purpose = purpose_exporter::get_name($this->purpose);
 
         // Set other defaults.
         $otherdefaults = [];
+        $url = new moodle_url('#');
         foreach ($this->otherdefaults as $pluginname => $values) {
             $defaults = [
-                'activityname' => $pluginname,
                 'name' => $values->name,
-                'categoryid' => $values->category,
-                'category' => helper::get_category_name($values->category),
-                'purposeid' => $values->purpose,
-                'purpose' => helper::get_purpose_name($values->purpose),
+                'category' => category_exporter::get_name($values->category),
+                'purpose' => purpose_exporter::get_name($values->purpose),
             ];
+            if ($this->canedit) {
+                $actions = [];
+                // Edit link.
+                $editattrs = [
+                    'data-action' => 'edit-activity-defaults',
+                    'data-contextlevel' => $this->mode,
+                    'data-activityname' => $pluginname,
+                    'data-category' => $values->category,
+                    'data-purpose' => $values->purpose,
+                ];
+                $editlink = new action_menu_link_primary($url, new \pix_icon('t/edit', get_string('edit')),
+                    get_string('edit'), $editattrs);
+                $actions[] = $editlink->export_for_template($output);
+
+                // Delete link.
+                $deleteattrs = [
+                    'data-action' => 'delete-activity-defaults',
+                    'data-contextlevel' => $this->mode,
+                    'data-activityname' => $pluginname,
+                    'data-activitydisplayname' => $values->name,
+                ];
+                $deletelink = new action_menu_link_primary($url, new \pix_icon('t/delete', get_string('delete')),
+                    get_string('delete'), $deleteattrs);
+                $actions[] = $deletelink->export_for_template($output);
+
+                $defaults['actions'] = $actions;
+            }
             $otherdefaults[] = (object)$defaults;
         }
         $data->otherdefaults = $otherdefaults;
index 24b2862..9faad87 100644 (file)
@@ -42,18 +42,20 @@ $otherdefaults = [];
 if ($mode == CONTEXT_MODULE) {
     // Get activity module plugin info.
     $pluginmanager = core_plugin_manager::instance();
-    $modplugins = $pluginmanager->get_plugins_of_type('mod');
+    $modplugins = $pluginmanager->get_enabled_plugins('mod');
 
-    foreach ($modplugins as $plugin) {
-        list($purposevar, $categoryvar) = \tool_dataprivacy\data_registry::var_names_from_context($classname, $plugin->name);
+    foreach ($modplugins as $name) {
+        list($purposevar, $categoryvar) = \tool_dataprivacy\data_registry::var_names_from_context($classname, $name);
         $plugincategory = get_config('tool_dataprivacy', $categoryvar);
         $pluginpurpose = get_config('tool_dataprivacy', $purposevar);
         if ($plugincategory === false && $pluginpurpose === false) {
             // If no purpose and category has been set for this plugin, then there's no need to show this on the list.
             continue;
         }
-        $otherdefaults[$plugin->name] = (object)[
-            'name' => $plugin->displayname,
+
+        $displayname = $pluginmanager->plugin_name('mod_' . $name);
+        $otherdefaults[$name] = (object)[
+            'name' => $displayname,
             'category' => $plugincategory,
             'purpose' => $pluginpurpose,
         ];
index e6ddf93..7a7015a 100644 (file)
@@ -28,3 +28,7 @@ dd a.contactdpo {
 [data-region="data-requests-table"] .moodle-actionmenu {
     min-width: 150px;
 }
+
+.context-level-view {
+    margin: 1em;
+}
\ No newline at end of file
index c7777c1..bb531da 100644 (file)
@@ -69,8 +69,6 @@
         ]
     }
 }}
-
-
 <div class="card">
     <div class="card-header">
         <ul class="nav nav-tabs card-header-tabs">
@@ -88,7 +86,7 @@
             </li>
         </ul>
     </div>
-    <div class="card-body">
+    <div class="card-body context-level-view">
         <div class="alert alert-primary" role="alert">
             {{#str}}defaultsinfo, tool_dataprivacy{{/str}}
         </div>
                         <td>{{purpose}}</td>
                         {{#canedit}}
                             <td>
-                                <a href="#" role="button" data-action="edit-activity-defaults" data-contextlevel="{{contextlevel}}" data-activityname="{{activityname}}" data-category="{{categoryid}}" data-purpose="{{purposeid}}">
-                                    {{#pix}}t/edit, core, {{#str}}edit{{/str}}{{/pix}}
-                                </a>
-                                <a href="#" role="button" data-action="delete-activity-defaults" data-contextlevel="{{contextlevel}}" data-activityname="{{activityname}}" data-activitydisplayname="{{name}}">
-                                    {{#pix}}t/delete, core, {{#str}}delete{{/str}}{{/pix}}
-                                </a>
+                                {{#actions}}
+                                    {{> core/action_menu_link}}
+                                {{/actions}}
                             </td>
                         {{/canedit}}
                     </tr>