Skip to content

Commit

Permalink
MDL-83562 core_courseformat: deprecate movehere actions
Browse files Browse the repository at this point in the history
  • Loading branch information
ferranrecio committed Nov 26, 2024
1 parent 15fc365 commit 9da7ae2
Show file tree
Hide file tree
Showing 13 changed files with 186 additions and 168 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ protected function get_cm_move_item(): ?link {
return null;
}

$url = new url($this->basemodurl, ['copy' => $this->mod->id]);
$url = new url($this->basemodurl);

return new link_secondary(
url: $url,
Expand Down
4 changes: 0 additions & 4 deletions course/format/classes/output/local/content/section.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ class section implements named_templatable, renderable {
/** @var section availability output class */
protected $availabilityclass;

/** @var optional move here output class */
protected $movehereclass;

/** @var optional visibility output class */
protected $visibilityclass;

Expand Down Expand Up @@ -104,7 +101,6 @@ public function __construct(course_format $format, section_info $section) {
$this->cmsummaryclass = $format->get_output_classname('content\\section\\cmsummary');
$this->controlmenuclass = $format->get_output_classname('content\\section\\controlmenu');
$this->availabilityclass = $format->get_output_classname('content\\section\\availability');
$this->movehereclass = $format->get_output_classname('content\\section\\movehere');
$this->visibilityclass = $format->get_output_classname('content\\section\\visibility');
}

Expand Down
12 changes: 7 additions & 5 deletions course/format/classes/output/local/content/section/cmlist.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class cmlist implements named_templatable, renderable {
/** @var string the item output class name */
protected $itemclass;

// TODO remove movehereclass as part of MDL-83530.
/** @var optional move here output class */
protected $movehereclass;

Expand Down Expand Up @@ -92,12 +93,12 @@ public function export_for_template(\renderer_base $output): stdClass {
$data = new stdClass();
$data->cms = [];

// By default, non-ajax controls are disabled but in some places like the frontpage
// it is necessary to display them. This is a temporal solution while JS is still
// optional for course editing.
// TODO remove showmovehere and the if clause as part of MDL-83530.
$showmovehere = ismoving($course->id);

if ($showmovehere) {
// By default, non-ajax controls are disabled but in some places like the frontpage
// it is necessary to display them. This is a temporal solution while JS is still
// optional for course editing.
$data->hascms = true;
$data->showmovehere = true;
$data->strmovefull = strip_tags(get_string("movefull", "", "'$user->activitycopyname'"));
Expand All @@ -112,8 +113,9 @@ public function export_for_template(\renderer_base $output): stdClass {

foreach ($modinfo->sections[$section->section] as $modnumber) {
$mod = $modinfo->cms[$modnumber];
// If the old non-ajax move is necessary, we do not print the selected cm.
// TODO remove this if as part of MDL-83530.
if ($showmovehere && $USER->activitycopy == $mod->id) {
// If the old non-ajax move is necessary, we do not print the selected cm.
continue;
}
if ($mod->is_visible_on_course_page()) {
Expand Down
24 changes: 21 additions & 3 deletions course/format/classes/output/local/content/section/controlmenu.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@ public function section_control_items() {
$controls['duplicate'] = $this->get_section_duplicate_item();
$controls['visibility'] = $this->get_section_visibility_item();
$controls['movesection'] = $this->get_section_movesection_item();
$controls['moveup'] = $this->get_section_moveup_item();
$controls['movedown'] = $this->get_section_movedown_item();
$controls['permalink'] = $this->get_section_permalink_item();
}

Expand Down Expand Up @@ -285,11 +283,22 @@ protected function get_section_movesection_item(): ?link {
* or when javascript is not available.
*
* Note: this action will be removed, do not depend on it for your
* custom formats. For more information, see MDL-83562.
* custom formats. For more information, see MDL-83562. Use this method
* only if your format is not compatible with the move section modal
* and you are still migrating to components.
*
* @deprecated since Moodle 5.0
* @todo Remove this method in Moodle 6.0 (MDL-83530).
* @return link|null The menu item if applicable, otherwise null.
*/
#[\core\attribute\deprecated(
replacement: 'core_courseformat\output\local\content\section::get_section_movesection_item',
since: '5.0',
reason: 'Non-ajax move is section deprecated.',
mdl: 'MDL-83562',
)]
protected function get_section_moveup_item(): ?link {
\core\deprecation::emit_deprecation_if_present([self::class, __FUNCTION__]);
if (
$this->section->sectionnum <= 1
|| $this->format->get_sectionid()
Expand Down Expand Up @@ -327,9 +336,18 @@ protected function get_section_moveup_item(): ?link {
* Note: this action will be removed, do not depend on it for your
* custom formats. For more information, see MDL-83562.
*
* @deprecated since Moodle 5.0
* @todo Remove this method in Moodle 6.0 (MDL-83530).
* @return link|null The menu item if applicable, otherwise null.
*/
#[\core\attribute\deprecated(
replacement: 'core_courseformat\output\local\content\section::get_section_movesection_item',
since: '5.0',
reason: 'Non-ajax section move is deprecated.',
mdl: 'MDL-83562',
)]
protected function get_section_movedown_item(): ?link {
\core\deprecation::emit_deprecation_if_present([self::class, __FUNCTION__]);
$numsections = $this->format->get_last_section_number();

if (
Expand Down
3 changes: 3 additions & 0 deletions course/format/templates/local/content/section/cmlist.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
Displays the course module list inside a course section.
TODO remove showmovehere, strmovefull, movingstr, cancelcopyurl, movetosectionurl,
strmovefull and all associated HTML as part of MDL-83530.
Example context (json):
{
"cms": [
Expand Down
60 changes: 60 additions & 0 deletions course/format/tests/behat/activity_move.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
@core @core_course @core_courseformat
Feature: Activities can be moved between sections
In order to rearrange my course contents
As a teacher
I need to move activities between sections

Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
And the following "course" exists:
| fullname | Course 1 |
| shortname | C1 |
| format | topics |
| coursedisplay | 0 |
| numsections | 5 |
| initsections | 1 |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
And the following "activities" exist:
| activity | name | course | idnumber | section |
| forum | Test forum name | C1 | 00001 | 1 |
| forum | Second forum name | C1 | 00002 | 1 |
| forum | Third forum name | C1 | 00002 | 3 |
| forum | Fourth forum name | C1 | 00002 | 3 |
And I log in as "teacher1"
And I am on "Course 1" course homepage with editing mode on

@javascript
Scenario: Move activity step test
When I move "Test forum name" activity to section "3"
Then I should see "Test forum name" in the "Section 3" "section"
And "Test forum name" "activity" should appear before "Third forum name" "activity"

@javascript
Scenario: The teacher can move an activity to another section using the activity action menu
When I open "Test forum name" actions menu
And I click on "Move" "link" in the "Test forum name" activity
And I click on "Section 3" "link" in the "Move activity" "dialogue"
Then I should see "Test forum name" in the "Section 3" "section"

@javascript
Scenario: The teacher can reorder activities in the same section using the activity action menu
Given "Test forum name" "activity" should appear before "Second forum name" "activity"
When I open "Test forum name" actions menu
And I click on "Move" "link" in the "Test forum name" activity
And I click on "Second forum name" "link" in the "Move activity" "dialogue"
Then I should see "Test forum name" in the "Section 1" "section"
And "Second forum name" "activity" should appear before "Test forum name" "activity"

@javascript
Scenario: The teacher can move an in the middle of a section using the activity action menu
When I open "Test forum name" actions menu
And I click on "Move" "link" in the "Test forum name" activity
And I click on "Expand" "link" in the "movemodalsection3" "region"
And I click on "Third forum name" "link" in the "Move activity" "dialogue"
Then I should see "Test forum name" in the "Section 3" "section"
And "Third forum name" "activity" should appear before "Test forum name" "activity"
And "Test forum name" "activity" should appear before "Fourth forum name" "activity"
47 changes: 47 additions & 0 deletions course/format/tests/behat/section_move.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
@core @core_course
Feature: Sections can be moved
In order to rearrange my course contents
As a teacher
I need to move sections up and down

Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
And the following "course" exists:
| fullname | Course 1 |
| shortname | C1 |
| format | topics |
| coursedisplay | 0 |
| numsections | 5 |
|initsections | 1 |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
And the following "activities" exist:
| activity | name | course | idnumber | section |
| forum | Test forum name | C1 | forum1 | 1 |
| forum | Second forum name | C1 | forum1 | 3 |

@javascript
Scenario: Teahcer can move a section to another location
Given I log in as "teacher1"
And I am on "Course 1" course homepage with editing mode on
When I open section "1" edit menu
And I click on "Move" "link" in the "Section 1" "section"
And I click on "Section 3" "link" in the "Move section" "dialogue"
Then "Section 1" "section" should appear after "Section 3" "section"
And I should see "Test forum name" in the "Section 1" "section"
And I should see "Second forum name" in the "Section 3" "section"

@javascript
Scenario: Teahcer can move a section under the general section
Given I log in as "teacher1"
And I am on "Course 1" course homepage with editing mode on
When I open section "3" edit menu
And I click on "Move" "link" in the "Section 3" "section"
And I click on "General" "link" in the "Move section" "dialogue"
Then "General" "section" should appear before "Section 3" "section"
Then "Section 3" "section" should appear before "Section 1" "section"
And I should see "Test forum name" in the "Section 1" "section"
And I should see "Second forum name" in the "Section 3" "section"
24 changes: 19 additions & 5 deletions course/mod.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@
$hide = optional_param('hide', 0, PARAM_INT); // TODO remove this param as part of MDL-83530.
$stealth = optional_param('stealth', 0, PARAM_INT); // TODO remove this param as part of MDL-83530.
$show = optional_param('show', 0, PARAM_INT); // TODO remove this param as part of MDL-83530.
$copy = optional_param('copy', 0, PARAM_INT);
$moveto = optional_param('moveto', 0, PARAM_INT);
$movetosection = optional_param('movetosection', 0, PARAM_INT);
$copy = optional_param('copy', 0, PARAM_INT); // TODO remove this param as part of MDL-83530.
$moveto = optional_param('moveto', 0, PARAM_INT); // TODO remove this param as part of MDL-83530.
$movetosection = optional_param('movetosection', 0, PARAM_INT); // TODO remove this param as part of MDL-83530.
$delete = optional_param('delete', 0, PARAM_INT); // TODO remove this param as part of MDL-83530.
$course = optional_param('course', 0, PARAM_INT);
$groupmode = optional_param('groupmode', -1, PARAM_INT); // TODO remove this param as part of MDL-83530.
$cancelcopy = optional_param('cancelcopy', 0, PARAM_BOOL);
$cancelcopy = optional_param('cancelcopy', 0, PARAM_BOOL); // TODO remove this param as part of MDL-83530.
$confirm = optional_param('confirm', 0, PARAM_BOOL); // TODO remove this param as part of MDL-83530.

// This page should always redirect
Expand Down Expand Up @@ -188,6 +188,11 @@


if ((!empty($movetosection) or !empty($moveto)) and confirm_sesskey()) {
// TODO remove this if as part of MDL-83530.
debugging(
'The moveto param is deprecated. Please use the standard move modal instead.',
DEBUG_DEVELOPER
);
$cm = get_coursemodule_from_id('', $USER->activitycopy, 0, true, MUST_EXIST);
$course = $DB->get_record('course', array('id' => $cm->course), '*', MUST_EXIST);

Expand Down Expand Up @@ -326,6 +331,11 @@
redirect(course_get_url($course, $cm->sectionnum, $urloptions));

} else if (!empty($copy) and confirm_sesskey()) { // value = course module
// TODO remove this else if as part of MDL-83530.
debugging(
'The copy param is deprecated. Please use the standard move modal instead.',
DEBUG_DEVELOPER
);
$cm = get_coursemodule_from_id('', $copy, 0, true, MUST_EXIST);
$course = $DB->get_record('course', array('id' => $cm->course), '*', MUST_EXIST);

Expand All @@ -344,7 +354,11 @@
redirect(course_get_url($course, $section->section, $urloptions));

} else if (!empty($cancelcopy) and confirm_sesskey()) { // value = course module

// TODO remove this else if as part of MDL-83530.
debugging(
'The copy param is deprecated. Please use the standard move modal instead.',
DEBUG_DEVELOPER
);
$courseid = $USER->activitycopycourse;
$course = $DB->get_record('course', array('id' => $courseid), '*', MUST_EXIST);

Expand Down
38 changes: 15 additions & 23 deletions course/tests/behat/behat_course.php
Original file line number Diff line number Diff line change
Expand Up @@ -851,9 +851,12 @@ public function label_should_be_hidden($activityname) {
* @param int $sectionnumber The number of section
*/
public function i_move_activity_to_section($activityname, $sectionnumber): void {
$this->require_javascript('Moving activities requires javascript.');

// Ensure the destination is valid.
$sectionxpath = $this->section_exists($sectionnumber);

// TODO: remove this if clause as part of MDL-83627 when YUI is removed from course.
// Not all formats are compatible with the move tool.
$activitynode = $this->get_activity_node($activityname);
if (!$activitynode->find('css', "[data-action='moveCm']", false, false, 0)) {
Expand All @@ -862,36 +865,25 @@ public function i_move_activity_to_section($activityname, $sectionnumber): void
return;
}

// JS enabled.
if ($this->running_javascript()) {
$this->i_open_actions_menu($activityname);
$this->execute(
'behat_course::i_click_on_in_the_activity',
[get_string('move'), "link", $this->escape($activityname)]
);
$this->execute("behat_general::i_click_on_in_the", [
"[data-for='section'][data-number='$sectionnumber']",
'css_element',
"[data-region='modal-container']",
'css_element'
]);
} else {
$this->execute(
'behat_course::i_click_on_in_the_activity',
[get_string('move'), "link", $this->escape($activityname)]
);
$this->execute(
'behat_general::i_click_on_in_the',
["li.movehere a", "css_element", $this->escape($sectionxpath), "xpath_element"]
);
}
$this->i_open_actions_menu($activityname);
$this->execute(
'behat_course::i_click_on_in_the_activity',
[get_string('move'), "link", $this->escape($activityname)]
);
$this->execute("behat_general::i_click_on_in_the", [
"[data-for='section'][data-number='$sectionnumber']",
'css_element',
"[data-region='modal-container']",
'css_element'
]);
}

/**
* Moves the specified activity to the first slot of a section using the YUI course format.
*
* This step is experimental when using it in Javascript tests. Editing mode should be on.
*
* @todo remove this module as part of MDL-83627.
* @param string $activityname The activity name
* @param int $sectionnumber The number of section
*/
Expand Down
2 changes: 2 additions & 0 deletions course/tests/behat/general_section.feature
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ Feature: General section does not show in navigation when empty
| unaddableblocks | | theme_boost|
And I add the "Navigation" block if not present

@javascript
Scenario: General section is visible in navigation when it is not empty
When I move "Test forum name" activity to section "0"
And I am on "Course 1" course homepage
Then I should see "General" in the "Navigation" "block"

@javascript
Scenario: General section is not visible in navigation when it is empty
When I move "Test forum name" activity to section "3"
And I am on "Course 1" course homepage
Expand Down
Loading

0 comments on commit 9da7ae2

Please sign in to comment.