MDL-77067 reportbuilder: indicate when audiences are used in schedules.

Indicate in each audience card whether it's used as part of a report
schedule, and inform user again upon deletion of said audience.
This commit is contained in:
Paul Holden 2023-01-30 19:57:10 +00:00
parent cccc00954d
commit b9a5a3626e
No known key found for this signature in database
GPG Key ID: A81A96D6045F6164
12 changed files with 142 additions and 42 deletions

View File

@ -51,6 +51,7 @@ $string['audiencemultiselectpostfix'] = '{$a->elements} plus {$a->morecount} mor
$string['audiencenotsaved'] = 'Audience not saved';
$string['audiencesaved'] = 'Audience saved';
$string['audienceupdated'] = 'Audience updated';
$string['audienceusedbyschedule'] = 'This audience is used in a schedule for this report';
$string['cardview'] = 'Card view';
$string['cardview_help'] = 'Card view allows you to define the layout of your report when viewed on narrow devices. Columns will collapse beyond the limit set here, with a toggle to expand the card to view all report data.';
$string['cardviewfirstcolumntitle'] = 'First column title';

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View File

@ -86,7 +86,7 @@ const editAudienceCard = audienceCard => {
// Load audience form with data for editing, then toggle visible controls in the card.
const audienceForm = initAudienceCardForm(audienceCard);
audienceForm.load({id: audienceCard.dataset.instanceid})
audienceForm.load({id: audienceCard.dataset.audienceId})
.then(() => {
const audienceFormContainer = audienceCard.querySelector(reportSelectors.regions.audienceFormContainer);
const audienceDescription = audienceCard.querySelector(reportSelectors.regions.audienceDescription);
@ -116,7 +116,7 @@ const initAudienceCardForm = audienceCard => {
const audienceHeading = audienceCard.querySelector(reportSelectors.regions.audienceHeading);
const audienceDescription = audienceCard.querySelector(reportSelectors.regions.audienceDescription);
audienceCard.dataset.instanceid = data.detail.instanceid;
audienceCard.dataset.audienceId = data.detail.instanceid;
audienceHeading.innerHTML = data.detail.heading;
audienceDescription.innerHTML = data.detail.description;
@ -129,7 +129,7 @@ const initAudienceCardForm = audienceCard => {
// If cancelling the form, close the card or remove it if it was never created.
audienceForm.addEventListener(audienceForm.events.FORM_CANCELLED, () => {
if (audienceCard.dataset.instanceid > 0) {
if (audienceCard.dataset.audienceId > 0) {
closeAudienceCardForm(audienceCard);
} else {
removeAudienceCard(audienceCard);
@ -146,17 +146,20 @@ const initAudienceCardForm = audienceCard => {
*/
const deleteAudienceCard = audienceDelete => {
const audienceCard = audienceDelete.closest(reportSelectors.regions.audienceCard);
const audienceTitle = audienceCard.dataset.title;
const {audienceId, audienceTitle, audienceEditWarning = false} = audienceCard.dataset;
// The edit warning indicates the audience is in use in a report schedule.
const audienceDeleteConfirmation = audienceEditWarning ? 'audienceusedbyschedule' : 'deleteaudienceconfirm';
Notification.saveCancelPromise(
getString('deleteaudience', 'core_reportbuilder', audienceTitle),
getString('deleteaudienceconfirm', 'core_reportbuilder', audienceTitle),
getString(audienceDeleteConfirmation, 'core_reportbuilder', audienceTitle),
getString('delete', 'core'),
{triggerElement: audienceDelete}
).then(() => {
const pendingPromise = new Pending('core_reportbuilder/audience:delete');
return deleteAudience(reportId, audienceCard.dataset.instanceid)
return deleteAudience(reportId, audienceId)
.then(() => addToast(getString('audiencedeleted', 'core_reportbuilder', audienceTitle)))
.then(() => {
removeAudienceCard(audienceCard);
@ -218,6 +221,7 @@ export const init = (id, contextid) => {
'audienceadded',
'audiencedeleted',
'audiencesaved',
'audienceusedbyschedule',
'deleteaudience',
'deleteaudienceconfirm',
]);

View File

@ -24,7 +24,7 @@ use context_system;
use core_collator;
use core_component;
use core_reportbuilder\local\audiences\base;
use core_reportbuilder\local\models\audience as audience_model;
use core_reportbuilder\local\models\{audience as audience_model, schedule};
/**
* Class containing report audience helper methods
@ -229,6 +229,25 @@ class audience {
return [$wheres, $params];
}
/**
* Return a list of audiences that are used by any schedule of the given report
*
* @param int $reportid
* @return int[] Array of audience IDs
*/
public static function get_audiences_for_report_schedules(int $reportid): array {
global $DB;
$audiences = $DB->get_fieldset_select(schedule::TABLE, 'audiences', 'reportid = ?', [$reportid]);
// Reduce JSON encoded audience data of each schedule to an array of audience IDs.
$audienceids = array_reduce($audiences, static function(array $carry, string $audience): array {
return array_merge($carry, (array) json_decode($audience));
}, []);
return array_unique($audienceids, SORT_NUMERIC);
}
/**
* Returns the list of audiences types in the system.
*

View File

@ -42,23 +42,23 @@ class audience extends base {
* @return array
*/
public function export_for_template(renderer_base $output): array {
$reportid = (int) $this->data['reportid'];
// Get all the audiences types to populate the left menu.
$menucardsexporter = new custom_report_audience_cards_exporter(null);
$menucards = (array) $menucardsexporter->export($output);
// Get all current audiences instances for this report.
$audienceinstances = $this->get_all_report_audiences();
$audienceinstances = $this->get_all_report_audiences($reportid);
$data = [
return [
'tabheading' => get_string('audience', 'core_reportbuilder'),
'reportid' => $this->data['reportid'],
'contextid' => (new report((int)$this->data['reportid']))->get('contextid'),
'reportid' => $reportid,
'contextid' => (new report($reportid))->get('contextid'),
'sidebarmenucards' => $menucards,
'instances' => $audienceinstances,
'hasinstances' => !empty($audienceinstances),
];
return $data;
}
/**
@ -92,32 +92,39 @@ class audience extends base {
/**
* Get all current audiences instances for this report.
*
* @param int $reportid
* @return array
*/
private function get_all_report_audiences(): array {
private function get_all_report_audiences(int $reportid): array {
global $PAGE;
$renderer = $PAGE->get_renderer('core');
$audienceinstances = [];
$reportaudiences = audience_helper::get_base_records((int)$this->data['reportid']);
$showormessage = false;
// Retrieve list of audiences that are used in report schedules, to warn user when editing.
$scheduleaudiences = audience_helper::get_audiences_for_report_schedules($reportid);
$reportaudiences = audience_helper::get_base_records($reportid);
foreach ($reportaudiences as $reportaudience) {
$persistent = $reportaudience->get_persistent();
$canedit = $reportaudience->user_can_edit();
$editable = new audience_heading_editable(0, $persistent);
$params = [
$audienceinstances[] = [
'instanceid' => $persistent->get('id'),
'description' => $reportaudience->get_description(),
'heading' => $reportaudience->get_name(),
'headingeditable' => $editable->render($renderer),
'editwarning' => in_array($persistent->get('id'), $scheduleaudiences) ?
get_string('audienceusedbyschedule', 'core_reportbuilder') : '',
'canedit' => $canedit,
'candelete' => $canedit,
'showormessage' => $showormessage,
];
$audienceinstances[] = $params;
$showormessage = true;
}

View File

@ -26,13 +26,18 @@
"heading": "Title",
"headingeditable": "Title (edit me)",
"showormessage": 1,
"editwarning": "Oh dear",
"canedit": 1,
"candelete": 1,
"description": "description",
"form": "form"
}
}}
<div class="instance-card" data-instanceid="{{instanceid}}" data-title="{{heading}}" data-region="audience-card">
<div data-region="audience-card" class="instance-card" data-audience-id="{{instanceid}}" data-audience-title="{{heading}}"
{{#editwarning}}
data-audience-edit-warning="1"
{{/editwarning}}
>
{{#showormessage}}
<span class="audience-separator d-flex mb-3 justify-content-center align-items-center small text-muted font-weight-bold">
{{#str}} or, core_reportbuilder {{/str}}
@ -67,6 +72,7 @@
</div>
</div>
<div class="card-body p-3" data-region="audience-form">
{{#editwarning}}<div class="alert alert-warning p-2">{{.}}</div>{{/editwarning}}
<div data-region="audience-description" {{^description}}class="hidden"{{/description}}>
{{{description}}}
</div>

View File

@ -28,12 +28,13 @@ Feature: Configure access to reports based on intended audience
And I should see "There are no audiences for this report"
And I click on "Add audience 'Manually added users'" "link"
And I should see "Added audience 'Manually added users'"
And I set the field "Add users manually" to "User 1,User 3"
And I should see "Audience not saved" in the "Manually added users" "core_reportbuilder > Audience"
And I set the following fields in the "Manually added users" "core_reportbuilder > Audience" to these values:
| Add users manually | User 1,User 3 |
And I press "Save changes"
And I should see "Audience saved"
And I should see "User 1"
And I should not see "User 2"
And I should see "User 3"
And I should not see "Audience not saved" in the "Manually added users" "core_reportbuilder > Audience"
And I should see "User 1, User 3" in the "Manually added users" "core_reportbuilder > Audience"
And I should not see "There are no audiences for this report"
And I click on the "Access" dynamic tab
And I should see "User 1" in the "reportbuilder-table" "table"
@ -59,21 +60,18 @@ Feature: Configure access to reports based on intended audience
And I should not see "User 3" in the "reportbuilder-table" "table"
Scenario: Configure report audience with has system role audience type
Given the following "roles" exist:
| shortname | name | archetype |
| testrole | Test role | |
And the following "role assigns" exist:
Given the following "role assigns" exist:
| user | role | contextlevel | reference |
| user2 | testrole | System | |
| user2 | manager | System | |
And I am on the "My report" "reportbuilder > Editor" page logged in as "admin"
And I click on the "Audience" dynamic tab
When I click on "Add audience 'Assigned system role'" "link"
And I should see "Added audience 'Assigned system role'"
And I set the field "Select a role" to "Test role"
And I set the following fields in the "Assigned system role" "core_reportbuilder > Audience" to these values:
| Select a role | Manager |
And I press "Save changes"
Then I should see "Audience saved"
And I should see "Test role"
And I should not see "There are no audiences for this report"
And I should see "Manager" in the "Assigned system role" "core_reportbuilder > Audience"
And I click on the "Access" dynamic tab
And I should not see "User 1" in the "reportbuilder-table" "table"
And I should see "User 2" in the "reportbuilder-table" "table"
@ -93,11 +91,11 @@ Feature: Configure access to reports based on intended audience
And I click on the "Audience" dynamic tab
When I click on "Add audience 'Member of cohort'" "link"
And I should see "Added audience 'Member of cohort'"
And I set the field "Select members from cohort" to "Cohort1"
And I set the following fields in the "Member of cohort" "core_reportbuilder > Audience" to these values:
| Select members from cohort | Cohort1 |
And I press "Save changes"
Then I should see "Audience saved"
And I should see "Cohort1"
And I should not see "There are no audiences for this report"
And I should see "Cohort1" in the "Member of cohort" "core_reportbuilder > Audience"
And I click on the "Access" dynamic tab
And I should not see "User 1" in the "reportbuilder-table" "table"
And I should not see "User 2" in the "reportbuilder-table" "table"
@ -167,8 +165,33 @@ Feature: Configure access to reports based on intended audience
And I click on "Add audience 'All users'" "link"
And I press "Save changes"
When I click on "Delete audience 'All users'" "button"
And I should see "Are you sure you want to delete the audience 'All users'?" in the "Delete audience 'All users'" "dialogue"
And I click on "Delete" "button" in the "Delete audience 'All users'" "dialogue"
Then I should see "Deleted audience 'All users'"
And "All users" "core_reportbuilder > Audience" should not exist
And I should see "There are no audiences for this report"
Scenario: Delete report audience used in schedule
Given the following "core_reportbuilder > Audiences" exist:
| report | configdata |
| My report | |
And I am on the "My report" "reportbuilder > Editor" page logged in as "admin"
And I click on the "Schedules" dynamic tab
And I press "New schedule"
And I set the following fields in the "New schedule" "dialogue" to these values:
| Name | My schedule |
| Starting from | ##tomorrow 11:00## |
| All users | 1 |
| Subject | Cause you know just what to say |
| Body | And you know just what to do |
And I click on "Save" "button" in the "New schedule" "dialogue"
When I click on the "Audience" dynamic tab
Then I should see "This audience is used in a schedule for this report" in the "All users" "core_reportbuilder > Audience"
And I click on "Delete audience 'All users'" "button"
And I should see "This audience is used in a schedule for this report" in the "Delete audience 'All users'" "dialogue"
And I click on "Delete" "button" in the "Delete audience 'All users'" "dialogue"
And I should see "Deleted audience 'All users'"
And "All users" "core_reportbuilder > Audience" should not exist
And I should see "There are no audiences for this report"
Scenario: Edit report audience with manually added users audience type
@ -176,17 +199,18 @@ Feature: Configure access to reports based on intended audience
And I click on the "Access" dynamic tab
And I should see "Nothing to display"
And I click on the "Audience" dynamic tab
And I should see "There are no audiences for this report"
And I click on "Add audience 'Manually added users'" "link"
And I set the field "Add users manually" to "User 1,User 3"
And I set the following fields in the "Manually added users" "core_reportbuilder > Audience" to these values:
| Add users manually | User 1,User 3 |
And I press "Save changes"
When I press "Edit audience 'Manually added users'"
And I set the field "Add users manually" to "User 2"
And "User 1" "autocomplete_selection" in the "Manually added users" "core_reportbuilder > Audience" should be visible
And "User 3" "autocomplete_selection" in the "Manually added users" "core_reportbuilder > Audience" should be visible
And I set the following fields in the "Manually added users" "core_reportbuilder > Audience" to these values:
| Add users manually | User 2 |
And I press "Save changes"
Then I should see "Audience saved"
And I should not see "User 1"
And I should see "User 2"
And I should not see "User 3"
And I should see "User 2" in the "Manually added users" "core_reportbuilder > Audience"
And I click on the "Access" dynamic tab
And I should not see "User 1" in the "reportbuilder-table" "table"
And I should see "User 2" in the "reportbuilder-table" "table"

View File

@ -75,6 +75,9 @@ class behat_reportbuilder extends behat_base {
new behat_component_named_selector('Condition', [
".//*[@data-region='conditions-form']//*[@data-condition-name=%locator%]",
]),
new behat_component_named_selector('Audience', [
".//*[@data-region='audiences']//*[@data-audience-title=%locator%]",
]),
];
}

View File

@ -34,7 +34,7 @@ Feature: Manage custom report schedules
| Name | My schedule |
| Starting from | ##tomorrow 11:00## |
| Subject | You're all I've ever wanted |
And I set the field "Body" to "And my arms are open wide"
| Body | And my arms are open wide |
# Confirm each audience is present in the form, select only the manually added users.
And I should see "All my lovely users" in the "New schedule" "dialogue"
And I set the field "Manually added users: User One, User Two" to "1"

View File

@ -288,4 +288,36 @@ class audience_test extends advanced_testcase {
$reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params);
$this->assertEmpty($reports);
}
/**
* Test getting list of audiences in use within schedules for a report
*/
public function test_get_audiences_for_report_schedules(): void {
$this->resetAfterTest();
/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
$audienceone = $generator->create_audience(['reportid' => $report->get('id'), 'configdata' => []]);
$audiencetwo = $generator->create_audience(['reportid' => $report->get('id'), 'configdata' => []]);
$audiencethree = $generator->create_audience(['reportid' => $report->get('id'), 'configdata' => []]);
// The first schedule contains audience one and two.
$generator->create_schedule(['reportid' => $report->get('id'), 'name' => 'Schedule one', 'audiences' =>
json_encode([$audienceone->get_persistent()->get('id'), $audiencetwo->get_persistent()->get('id')])
]);
// Second schedule contains only audience one.
$generator->create_schedule(['reportid' => $report->get('id'), 'name' => 'Schedule two', 'audiences' =>
json_encode([$audienceone->get_persistent()->get('id')])
]);
// The first two audiences should be returned, the third omitted.
$audiences = audience::get_audiences_for_report_schedules($report->get('id'));
$this->assertEqualsCanonicalizing([
$audienceone->get_persistent()->get('id'),
$audiencetwo->get_persistent()->get('id'),
], $audiences);
}
}

View File

@ -19,6 +19,10 @@ Information provided here is intended especially for developers.
- `instance:contexturl` (tag) => `context:link`
* The following report entity filters/conditions have been deprecated, with replacements as follows:
- `enrolment:method` => `enrol:plugin`
* The `local/audience/form` template outer region has renamed some of it's `data-` attributes (relevant for any themes that
have overridden this template):
- `data-instanceid` => `data-audience-id`
- `data-title` => `data-audience-title`
* The `add_base_condition_sql` method of the base report class will now ignore empty where clauses
* If a non-default column is specified in a datasource `get_default_column_sorting` method, a coding exception will be thrown
* Trying to add/annotate duplicate entity names to a report will now throw a coding exception