Moodle
  1. Moodle
  2. MDL-42508

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

              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
              Discovered while testing

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

              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

              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

                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

                  • Votes:
                    0 Vote for this issue
                    Watchers:
                    7 Start watching this issue

                    Dates

                    • Created:
                      Updated:
                      Resolved:

                      Agile