[MDL-33774] Convert hardcode tests for label to module_supports() - Moodle Tracker

Details

Type: Task
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 2.2.3, 2.5
Fix Version/s: None
Component/s: Libraries
Labels:
- api_change
- triaged

Testing Instructions:
Hide

Hopefully this issue will be integrated together with ~~MDL-37455~~ and they can be tested together.

Test 1. Upgrade

BEFORE integrating this issue

Create course with labels in it (or several courses)

Apply commits from this issue

Run upgrade

Make sure that you can view the courses and see all labels

Test 2.

Create a course in weeks/topics format with at least one label, one folder that is displayed inside the course (see ~~MDL-37455~~) and one normal module

Add recent activity block

Turn editing on and try different actions on modules - inline edit of the name, hide/show, duplicate, delete. Make sure all works

Make sure recent activity block shows actions on non-inline module only (exception: the delete action on inline folder will be shown)

Login as student, view the first course, do some activities on the course (view, submit assignment, etc.)

Login as admin/teacher, view all available reports on the course, make sure that only activities on non-inline module(s) are shown

Enable filter "activity names"

Create some text in the course where you use all names of existing activities

Make sure that filter auto-link only activities that are not displayed inline

Create a course in social format, add similar 3 activities in the block, turn editing on, make sure editing buttons work
Show
Hopefully this issue will be integrated together with MDL-37455 and they can be tested together. Test 1. Upgrade BEFORE integrating this issue Create course with labels in it (or several courses) Apply commits from this issue Run upgrade Make sure that you can view the courses and see all labels Test 2. Create a course in weeks/topics format with at least one label, one folder that is displayed inside the course (see MDL-37455 ) and one normal module Add recent activity block Turn editing on and try different actions on modules - inline edit of the name, hide/show, duplicate, delete. Make sure all works Make sure recent activity block shows actions on non-inline module only (exception: the delete action on inline folder will be shown) Login as student, view the first course, do some activities on the course (view, submit assignment, etc.) Login as admin/teacher, view all available reports on the course, make sure that only activities on non-inline module(s) are shown Enable filter "activity names" Create some text in the course where you use all names of existing activities Make sure that filter auto-link only activities that are not displayed inline Create a course in social format, add similar 3 activities in the block, turn editing on, make sure editing buttons work
Affected Branches:
MOODLE_22_STABLE, MOODLE_25_STABLE
Pull from Repository:
git://github.com/marinaglancy/moodle.git
Pull Master Branch:
wip-~~MDL-33774~~-master
Pull Master Diff URL:
https://github.com/marinaglancy/moodle/compare/master...wip-MDL-33774-master

Rank:
41818

Description

As discovered in ~~MDL-33726~~, we need to convert hardcoded use of labels:

grep -r "== [\"']label[\"']" *

course/lib.php:            if ($info[0] == 'label') {     // Labels are ignored in recent activity
course/lib.php:    if ($mod->modname !== 'label' && $hasmanageactivities && course_ajax_enabled($COURSE)) {
lib/modinfolib.php:        if ($this->modname === 'label' && $this->content === '') {
report/log/locallib.php:            if ($mod->mod == "label") {
report/log/locallib.php:            if ($mod->mod == "label") {

Issue Links

blocks

MDL-31885 The activity name auto-linking filter should not create links to webpages it was created on.

Closed

MDL-37455 Allow mod_folder to display the content inline on course page instead of on separate page

Closed

has a non-specific relationship to

MDL-33726 In single section mode summary do not list label items

Closed

is duplicated by

MDL-37454 Add core support for activities other than labels to display contents inline

Closed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Petr Škoda added a comment - 15/Jun/12 3:53 PM

I am going to drop everything simpletest related in 2.4dev asap.

Show

Petr Škoda added a comment - 15/Jun/12 3:53 PM I am going to drop everything simpletest related in 2.4dev asap.

Hide

Permalink

Dan Poltawski added a comment - 15/Jun/12 3:54 PM

oops, just removed the simpletest things here as not relevant (not regarding mod label)

Show

Dan Poltawski added a comment - 15/Jun/12 3:54 PM oops, just removed the simpletest things here as not relevant (not regarding mod label)

Hide

Permalink

Marina Glancy added a comment - 23/Jan/13 4:50 AM

oh no, there are even more places where label is hardcoded. Reopening

Show

Marina Glancy added a comment - 23/Jan/13 4:50 AM oh no, there are even more places where label is hardcoded. Reopening

Hide

Permalink

Marina Glancy added a comment - 23/Jan/13 5:58 AM

Ok, I corrected almost all hardcoded 'label's that I've found.

I have not used module_supports() rather cm_info::has_view()

We should have better solution for reports but since the logging/reporting API will be redesigned I thought it is unreasonable to implement new callback for reporting purposes now.

There is one know place where I did not remove hardcoded label:
https://github.com/moodle/moodle/blob/master/course/format/renderer.php#L356
because here no other module should be hidden

Show

Marina Glancy added a comment - 23/Jan/13 5:58 AM Ok, I corrected almost all hardcoded 'label's that I've found. I have not used module_supports() rather cm_info::has_view() We should have better solution for reports but since the logging/reporting API will be redesigned I thought it is unreasonable to implement new callback for reporting purposes now. There is one know place where I did not remove hardcoded label: https://github.com/moodle/moodle/blob/master/course/format/renderer.php#L356 because here no other module should be hidden

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 25/Jan/13 11:35 AM

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Show

Eloy Lafuente (stronk7) added a comment - 25/Jan/13 11:35 AM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao

Hide

Permalink

Sam Hemelryk added a comment - 29/Jan/13 10:36 PM

Hi Marina,

This is presently causing a couple of unit tests to fail.
Somethings not quite right yet sorry.

Many thanks
Sam

Show

Sam Hemelryk added a comment - 29/Jan/13 10:36 PM Hi Marina, This is presently causing a couple of unit tests to fail. Somethings not quite right yet sorry. Many thanks Sam

Hide

Permalink

CiBoT added a comment - 30/Jan/13 12:01 AM

Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

Show

CiBoT added a comment - 30/Jan/13 12:01 AM Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

Hide

Permalink

CiBoT added a comment - 30/Jan/13 12:01 AM

Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

Show

CiBoT added a comment - 30/Jan/13 12:01 AM Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

Hide

Permalink

Marina Glancy added a comment - 14/Feb/13 6:38 AM

YES! After spending several days trying to find what broke the phpunit tests I found it! The filter_manager instance was cached. I have no idea why nobody else experienced those conflicts before and I was so lucky. Anyway, added a commit clearing the cached filter_manager instance

Show

Marina Glancy added a comment - 14/Feb/13 6:38 AM YES! After spending several days trying to find what broke the phpunit tests I found it! The filter_manager instance was cached. I have no idea why nobody else experienced those conflicts before and I was so lucky. Anyway, added a commit clearing the cached filter_manager instance

Hide

Permalink

Damyon Wiese added a comment - 14/Feb/13 7:49 AM

Thanks!

Show

Damyon Wiese added a comment - 14/Feb/13 7:49 AM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Thanks!

Hide

Permalink

Damyon Wiese added a comment - 19/Feb/13 4:38 AM

Thanks Marina - I like removing special cases from the code!

I didn't spot any issues and the unit tests are passing.

I have integrated this to master only (improvement).

Show

Damyon Wiese added a comment - 19/Feb/13 4:38 AM Thanks Marina - I like removing special cases from the code! I didn't spot any issues and the unit tests are passing. I have integrated this to master only (improvement).

Hide

Permalink

David Monllaó added a comment - 20/Feb/13 6:30 AM

It passes, tested in master

Show

David Monllaó added a comment - 20/Feb/13 6:30 AM It passes, tested in master

Hide

Permalink

Damyon Wiese added a comment - 22/Feb/13 3:59 AM

Thanks for your hard work - this issue has made it! Moodle is now a little bit better.

Regards, Damyon

Show

Damyon Wiese added a comment - 22/Feb/13 3:59 AM Thanks for your hard work - this issue has made it! Moodle is now a little bit better. Regards, Damyon

People

Assignee:

Marina Glancy

Reporter:

Dan Poltawski

Integrator:

Damyon Wiese

Tester:

David Monllaó

Participants:

CiBoT, Damyon Wiese, Dan Poltawski, David Monllaó, Eloy Lafuente (stronk7), Marina Glancy, Petr Škoda, Sam Hemelryk

Votes:

1 Vote for this issue

Watchers:

7 Start watching this issue

Dates

Created:

15/Jun/12 3:46 PM

Updated:

30/Sep/13 1:30 PM

Resolved:

22/Feb/13 3:59 AM

Agile

View on Board