[MDL-42508] Create module generators for scorm, imscp and folder - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 2.6
Fix Version/s: 2.6.1
Component/s: Automated functional tests (behat), IMS-CP resource type, SCORM, Unit tests
Labels:
- triaged

Testing Instructions:
Hide

Some changes had to be made to the form data processing in modules scorm and imscp. No changes to modules for folder so only automated tests are necessary.

Regression test:

Add SCORM activity specifying a zip file

View it and make sure it is ok

Edit SCORM activity, do not change zip file

View it and make sure it is ok

Edit SCORM activity, change zip file

View it and make sure it is ok

Edit SCORM activity, pick a file by reference (it can be a zip file from anywhere or imsmanifest.xml from filesystem repository)

View it and make sure it is ok

Try creating and editing scorm activity specifying url or file. (it looks from the code that file will never be deleted after the type is changed to url, this is not related to this issue).

Try creating scorm activity by drag&drop of the zip file on the course page (in editing mode), make sure it is created ok

Add IMSCP activity

View it and make sure it is ok

Edit IMSCP activity, do not upload file

View it and make sure it is ok

Edit IMSCP activity, upload a new file

View it and make sure it is ok
Show
Some changes had to be made to the form data processing in modules scorm and imscp. No changes to modules for folder so only automated tests are necessary. Regression test: Add SCORM activity specifying a zip file View it and make sure it is ok Edit SCORM activity, do not change zip file View it and make sure it is ok Edit SCORM activity, change zip file View it and make sure it is ok Edit SCORM activity, pick a file by reference (it can be a zip file from anywhere or imsmanifest.xml from filesystem repository) View it and make sure it is ok Try creating and editing scorm activity specifying url or file. (it looks from the code that file will never be deleted after the type is changed to url, this is not related to this issue). Try creating scorm activity by drag&drop of the zip file on the course page (in editing mode), make sure it is created ok Add IMSCP activity View it and make sure it is ok Edit IMSCP activity, do not upload file View it and make sure it is ok Edit IMSCP activity, upload a new file View it and make sure it is ok
Affected Branches:
MOODLE_26_STABLE
Fixed Branches:
MOODLE_26_STABLE
Pull from Repository:
git://github.com/marinaglancy/moodle.git
Pull Master Branch:
wip-~~MDL-42508~~-master
Pull Master Diff URL:
https://github.com/marinaglancy/moodle/compare/moodle:master...wip-MDL-42508-master
Sprint:
BACKEND Sprint 7

Sprint:
BACKEND Sprint 7

Description

Following ~~MDL-42400~~. Scorm and imscp generators are delayed because they need changes in the modules as well so they don't need instance of moodleform.
Also for some reason I forgot about generator for folder.

Gliffy Diagrams

Attachments

Issue Links

blocks

MDL-43026 Add tests for all existing module generators in behat

Closed

Discovered while testing

MDL-43078 Scorm package: when using AICC Direct URL on a private site, no clear error message is displayed, just a PHP notice.

Open

has been marked as being related by

MDL-40056 Replace add_to_log with an event trigger - mod_scorm

Closed

is blocked by

MDL-42400 Make module generator use add_moduleinfo

Closed

MDL-42677 Scorm package in tests/packages produces warnings

Closed

MDL-42287 Automate MDLQA-82 - The SCORM module enables teachers to add SCORM packages to a course

Closed

Show 1 more links (1 is blocked by)

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Marina Glancy added a comment - 19/Nov/13 4:31 AM

Ready for peer review

Show

Marina Glancy added a comment - 19/Nov/13 4:31 AM Ready for peer review

Hide

Permalink

Dan Marsden added a comment - 22/Nov/13 7:10 AM

Will leave for Ankit to do proper peer review but quick look at SCORM changes look good to me - would be good to add Drag/drop onto course homepage and "add an activity" for SCORM to the testing instructions though. Thanks Marina!

Show

Dan Marsden added a comment - 22/Nov/13 7:10 AM Will leave for Ankit to do proper peer review but quick look at SCORM changes look good to me - would be good to add Drag/drop onto course homepage and "add an activity" for SCORM to the testing instructions though. Thanks Marina!

Hide

Permalink

Ankit Agarwal added a comment - 22/Nov/13 7:37 AM

Hi Marina,

Changes look perfect. Can you please add defaults for 'showexpanded' and 'display' fields in folder generator. Rest looks good.

cheers

Show

Ankit Agarwal added a comment - 22/Nov/13 7:37 AM Hi Marina, Changes look perfect. Can you please add defaults for 'showexpanded' and 'display' fields in folder generator. Rest looks good. cheers

Hide

Permalink

Marina Glancy added a comment - 22/Nov/13 7:58 AM

Thanks Ankit, I added default values for folder.

Thanks Dan, I added testing instruction

Submitting for integration

Show

Marina Glancy added a comment - 22/Nov/13 7:58 AM Thanks Ankit, I added default values for folder. Thanks Dan, I added testing instruction Submitting for integration

Hide

Permalink

Sam Hemelryk added a comment - 25/Nov/13 7:29 PM

My feeling on this is that it is an improvement not a bug, and master only.
If everyone is happy with that I will mark this integration_held and it can wait until after the on-sync period ends.
Sound OK to everyone?

Show

Sam Hemelryk added a comment - 25/Nov/13 7:29 PM My feeling on this is that it is an improvement not a bug, and master only. If everyone is happy with that I will mark this integration_held and it can wait until after the on-sync period ends. Sound OK to everyone?

Hide

Permalink

Dan Marsden added a comment - 25/Nov/13 10:21 PM

yeah - my feeling was that this should be master only too - thanks Sam.

Show

Dan Marsden added a comment - 25/Nov/13 10:21 PM yeah - my feeling was that this should be master only too - thanks Sam.

Hide

Permalink

Marina Glancy added a comment - 26/Nov/13 1:38 AM

I thought we had a policy to backport generators and behat tests where possible. But if you say master only - ok

Show

Marina Glancy added a comment - 26/Nov/13 1:38 AM I thought we had a policy to backport generators and behat tests where possible. But if you say master only - ok

Hide

Permalink

David Monllaó added a comment - 26/Nov/13 2:42 AM

Hi, just adding the link, http://docs.moodle.org/dev/Integration_Review#Integration_Principles #6, but it is as long as it is not breaking #1 and #2

Show

David Monllaó added a comment - 26/Nov/13 2:42 AM Hi, just adding the link, http://docs.moodle.org/dev/Integration_Review#Integration_Principles #6, but it is as long as it is not breaking #1 and #2

Hide

Permalink

Sam Hemelryk added a comment - 26/Nov/13 7:08 PM

Aha - well backport it is then.

Marina could you please create branches for me, it cherry-picks cleanly to 26 but not to 25.

Cheers
Sam

Show

Sam Hemelryk added a comment - 26/Nov/13 7:08 PM Aha - well backport it is then. Marina could you please create branches for me, it cherry-picks cleanly to 26 but not to 25. Cheers Sam

Hide

Permalink

Marina Glancy added a comment - 26/Nov/13 11:08 PM

Sam, it can not be backported to 2.5 because of new module generator introduced in 2.6

Show

Marina Glancy added a comment - 26/Nov/13 11:08 PM Sam, it can not be backported to 2.5 because of new module generator introduced in 2.6

Hide

Permalink

Sam Hemelryk added a comment - 26/Nov/13 11:12 PM

lol lovin' it

Show

Sam Hemelryk added a comment - 26/Nov/13 11:12 PM lol lovin' it

Hide

Permalink

Sam Hemelryk added a comment - 26/Nov/13 11:39 PM

Thanks Marina - this has been integrated now.

Show

Sam Hemelryk added a comment - 26/Nov/13 11:39 PM Thanks Marina - this has been integrated now.

Hide

Permalink

Jérôme Mouneyrac added a comment - 27/Nov/13 4:35 AM

Except 9., everything else work. Marina tested on Master, the PHP notice I can see in 9. seems to be on Master too so I'll create an issue for 9.
Passing, thanks.

Show

Jérôme Mouneyrac added a comment - 27/Nov/13 4:35 AM Except 9., everything else work. Marina tested on Master, the PHP notice I can see in 9. seems to be on Master too so I'll create an issue for 9. Passing, thanks.

Hide

Permalink

Jérôme Mouneyrac added a comment - 27/Nov/13 4:40 AM

I created MDL-43078

Show

Jérôme Mouneyrac added a comment - 27/Nov/13 4:40 AM I created MDL-43078

Hide

Permalink

Dan Poltawski added a comment - 29/Nov/13 6:34 AM

Congratulations, this change has now made its way upstream. Thanks for your contribution!

“ Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. ” - Rick Osborne

Show

Dan Poltawski added a comment - 29/Nov/13 6:34 AM Congratulations, this change has now made its way upstream. Thanks for your contribution! “ Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. ” - Rick Osborne

People

Assignee:

Marina Glancy

Reporter:

Marina Glancy

Peer reviewer:

Ankit Agarwal

Integrator:

Sam Hemelryk

Tester:

Jérôme Mouneyrac

Participants:

Ankit Agarwal, Dan Marsden, Dan Poltawski, David Monllaó, Jérôme Mouneyrac, Marina Glancy, Sam Hemelryk

Votes:

0 Vote for this issue

Watchers:

7 Start watching this issue

Dates

Created:

24/Oct/13 2:01 AM

Updated:

29/Nov/13 6:34 AM

Resolved:

29/Nov/13 6:34 AM

Fix Release Date:

13/Jan/14