Moodle

Create module generators for scorm, imscp and folder

Details

  • 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:

    1. Add SCORM activity specifying a zip file
    2. View it and make sure it is ok
    3. Edit SCORM activity, do not change zip file
    4. View it and make sure it is ok
    5. Edit SCORM activity, change zip file
    6. View it and make sure it is ok
    7. Edit SCORM activity, pick a file by reference (it can be a zip file from anywhere or imsmanifest.xml from filesystem repository)
    8. View it and make sure it is ok
    9. 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).
    10. Try creating scorm activity by drag&drop of the zip file on the course page (in editing mode), make sure it is created ok
    1. Add IMSCP activity
    2. View it and make sure it is ok
    3. Edit IMSCP activity, do not upload file
    4. View it and make sure it is ok
    5. Edit IMSCP activity, upload a new file
    6. 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:
  • Pull Master Branch:
    wip-MDL-42508-master
  • Pull Master Diff URL:

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.

Issue Links

blocks

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-43026 Add tests for all existing module generators in behat

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
has been marked as being related by

Task - A task that needs to be completed. Tasks usually refer to work that must be done outside the product. MDL-40056 Replace add_to_log with an event trigger - mod_scorm

  • Major - Major loss of function, incorrect output
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
is blocked by

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-42400 Make module generator use add_moduleinfo

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-42677 Scorm package in tests/packages produces warnings

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Task - A task that needs to be completed. Tasks usually refer to work that must be done outside the product. MDL-42287 Automate MDLQA-82 - The SCORM module enables teachers to add SCORM packages to a course

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
testing discovered

Improvement - An enhancement to an existing Moodle feature. MDL-43078 Scorm package: when using AICC Direct URL on a private site, no clear error message is displayed, just a PHP notice.

  • Trivial - Cosmetic problem like misspelt words or misaligned text
  • Open - The issue is open and ready for the assignee to start work on it.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Marina Glancy added a comment -

Ready for peer review

Show
Marina Glancy added a comment - Ready for peer review
Hide
Permalink
Dan Marsden added a comment -

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 - 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 -

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 - 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 -

Thanks Ankit, I added default values for folder.

Thanks Dan, I added testing instruction

Submitting for integration

Show
Marina Glancy added a comment - Thanks Ankit, I added default values for folder. Thanks Dan, I added testing instruction Submitting for integration
Hide
Permalink
Sam Hemelryk added a comment -

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 - 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 -

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

Show
Dan Marsden added a comment - yeah - my feeling was that this should be master only too - thanks Sam.
Hide
Permalink
Marina Glancy added a comment -

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 - 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 -

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 - 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 -

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 - 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 -

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

Show
Marina Glancy added a comment - 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 -

lol lovin' it

Show
Sam Hemelryk added a comment - lol lovin' it
Hide
Permalink
Sam Hemelryk added a comment -

Thanks Marina - this has been integrated now.

Show
Sam Hemelryk added a comment - Thanks Marina - this has been integrated now.
Hide
Permalink
Jérôme Mouneyrac added a comment -

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 - 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 -

I created MDL-43078

Show
Jérôme Mouneyrac added a comment - I created MDL-43078
Hide
Permalink
Dan Poltawski added a comment -

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 - 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

Vote (0)
Watch (7)

Dates

  • Created:
    Updated:
    Resolved:

Agile