Moodle
  1. Moodle
  2. MDL-25552

2.0 Lesson module :: cannot remove a "mediafile"

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0, 2.3.3
    • Fix Version/s: 2.3.4
    • Component/s: Lesson
    • Labels:
    • Testing Instructions:
      Hide
      1. Create a few lesson activities in a course with different file types BEFORE applying this patch.
      2. Apply this patch and check that those lesson activities still show the file you uploaded earlier when editing them.
      3. Create new lesson activities, some with no file, others with a file. Choose multiple file types (pdf, txt, jpg etc). Try and upload more than one file per lesson (should only ever been one file allowed at one time).
      4. Save and check that the files are still there.
      5. Look at the database table 'lesson' and ensure the mediafile column is being generated correctly (should be a '/' with the filename afterwards or blank for no file)
      6. In the lesson settings delete the files for those activities that contain files and save.
      7. Look at the database table 'lesson' and ensure the mediafile column is an empty string.
      8. Check the files have been deleted from the 'files' table as well.
      9. Click back on the settings and ensure the file is not there.
      10. Make sure there are no error messages during these steps.
      Show
      Create a few lesson activities in a course with different file types BEFORE applying this patch. Apply this patch and check that those lesson activities still show the file you uploaded earlier when editing them. Create new lesson activities, some with no file, others with a file. Choose multiple file types (pdf, txt, jpg etc). Try and upload more than one file per lesson (should only ever been one file allowed at one time). Save and check that the files are still there. Look at the database table 'lesson' and ensure the mediafile column is being generated correctly (should be a '/' with the filename afterwards or blank for no file) In the lesson settings delete the files for those activities that contain files and save. Look at the database table 'lesson' and ensure the mediafile column is an empty string. Check the files have been deleted from the 'files' table as well. Click back on the settings and ensure the file is not there. Make sure there are no error messages during these steps.
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-25552_master

      Description

      1. In the Lesson Pop-up to file or web page section of the settings, once you have selected a file, you can no longer remove it on subsequent edits of that Lesson settings.

      2.- The attached patch (lesson_mediafile20.txt) provides a solution by making a Delete button available.
      Please note that if you want to replace an existing file with another you first have to tick the Delete box, save the settings and re-edit the settings and load the new file.

      3.- Please note
      a. that it should really be called Pop-up to a media file since it can not link to a Web page at all.
      b. the only type of media file it actually accepts is an image (no texts, HTML files, the audio player does not work, etc.
      c. In the longer term, that Pop-up to file or web page feature should probably be removed, as it can be replaced by using an HTML block.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Eloy Lafuente (stronk7) added a comment -

            Moving to STABLE backlog. Thanks for the report!

            Show
            Eloy Lafuente (stronk7) added a comment - Moving to STABLE backlog. Thanks for the report!
            Hide
            Joseph Rézeau added a comment -

            BUMP ! bug reported (with fix provided) almost a year ago and still not fixed...

            Show
            Joseph Rézeau added a comment - BUMP ! bug reported (with fix provided) almost a year ago and still not fixed...
            Hide
            Jai Gupta added a comment -

            Based on existing patch, contains patched files for Moodle v 2.2.3.

            Show
            Jai Gupta added a comment - Based on existing patch, contains patched files for Moodle v 2.2.3.
            Hide
            Nicolas Martignoni added a comment -

            Still present. Any progress here?

            Show
            Nicolas Martignoni added a comment - Still present. Any progress here?
            Hide
            Rossiani Wijaya added a comment -

            Added for next Sprint.

            Show
            Rossiani Wijaya added a comment - Added for next Sprint.
            Hide
            Mark Nelson added a comment -

            Thanks for the patch guys, however I am simply going to change the lesson from using a filepicker to a filemanager which allows this functionality.

            Show
            Mark Nelson added a comment - Thanks for the patch guys, however I am simply going to change the lesson from using a filepicker to a filemanager which allows this functionality.
            Hide
            Rajesh Taneja added a comment -

            Thanks Mark,

            Patch looks good, but not sure if this can go back on 23. Leaving it for integrators.
            [y] Syntax
            [-] Output
            [y] Whitespace
            [-] Language
            [-] Databases
            [y] Testing
            [-] Security
            [-] Documentation
            [y] Git
            [y] Sanity check

            Show
            Rajesh Taneja added a comment - Thanks Mark, Patch looks good, but not sure if this can go back on 23. Leaving it for integrators. [y] Syntax [-] Output [y] Whitespace [-] Language [-] Databases [y] Testing [-] Security [-] Documentation [y] Git [y] Sanity check
            Hide
            Sam Hemelryk added a comment -

            Sending this back Mark.

            I've tried (with the patch applied) creating a new lesson with an image as a mediafile. I found nothing was saved to the mediafile field so something must be going wrong in there presently.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Sending this back Mark. I've tried (with the patch applied) creating a new lesson with an image as a mediafile. I found nothing was saved to the mediafile field so something must be going wrong in there presently. Cheers Sam
            Hide
            CiBoT added a comment -

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

            Show
            CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            Mark Nelson added a comment -

            Doh, I used 'mod_resource' instead of 'mod_lesson'.

            Show
            Mark Nelson added a comment - Doh, I used 'mod_resource' instead of 'mod_lesson'.
            Hide
            Mark Nelson added a comment -

            Sam, made some changes to the code and tested to ensure the mediafile column was getting generated correctly, which it appears to be. Before I was testing that it worked and forgot to look that this column was being populated correctly.

            Show
            Mark Nelson added a comment - Sam, made some changes to the code and tested to ensure the mediafile column was getting generated correctly, which it appears to be. Before I was testing that it worked and forgot to look that this column was being populated correctly.
            Hide
            Joseph Rézeau added a comment - - edited

            @Mark,
            Just tested your patch. The change from using a filepicker to a filemanager does work. Good!
            Unfortunately you must have broken something else, because now the Linked media/Click here to view link is not displayed to the student when taking the lesson.

            EDIT.- Actually, the mediafile reference is NOT saved to the lesson table in the moodle database.

            Show
            Joseph Rézeau added a comment - - edited @Mark, Just tested your patch. The change from using a filepicker to a filemanager does work. Good! Unfortunately you must have broken something else, because now the Linked media/Click here to view link is not displayed to the student when taking the lesson. EDIT.- Actually, the mediafile reference is NOT saved to the lesson table in the moodle database.
            Hide
            Mark Nelson added a comment -

            Hi Joseph, I think you may still be testing on the old commit without the changes as I have tested on both 2.3 and master and the mediafile column is being populated correctly. Can you make sure you have the latest changes? Also, this patch should not have any affect on whether the mediafile link shows or not - strange this is suddenly happening. I tested on my machine and the student was able to see the link. If you are sure you are using the latest commit and are still experiencing issues can you please attach the file you are testing with? That would be great. Thanks for your help.

            Show
            Mark Nelson added a comment - Hi Joseph, I think you may still be testing on the old commit without the changes as I have tested on both 2.3 and master and the mediafile column is being populated correctly. Can you make sure you have the latest changes? Also, this patch should not have any affect on whether the mediafile link shows or not - strange this is suddenly happening. I tested on my machine and the student was able to see the link. If you are sure you are using the latest commit and are still experiencing issues can you please attach the file you are testing with? That would be great. Thanks for your help.
            Hide
            Mark Nelson added a comment -

            Also, if you are using a lesson module you created when using the old patch, please delete it and try with a new lesson activity. There was an issue with the old patch allowing 2 files to be uploaded which will fail the check "count($files) == 1)" and never populate the mediafile column.

            Show
            Mark Nelson added a comment - Also, if you are using a lesson module you created when using the old patch, please delete it and try with a new lesson activity. There was an issue with the old patch allowing 2 files to be uploaded which will fail the check "count($files) == 1)" and never populate the mediafile column.
            Hide
            Mark Nelson added a comment -

            Sam, just a note $fs->get_area_files has a different default value in 2.3 than 2.4 for the 5th parameter hence the difference.

            Show
            Mark Nelson added a comment - Sam, just a note $fs->get_area_files has a different default value in 2.3 than 2.4 for the 5th parameter hence the difference.
            Hide
            Joseph Rézeau added a comment -

            Hi Mark,

            You're right, I had not updated to your latest version. Just re-tested your patch now (on moodle 2.4 latest) and everything works fine. Many thanks for working on this (and not taking into account my pessimistic view of the matter where I went as fas as suggesting that that popup feature should be removed altogether from Lesson)...

            Just a small point. Once a file has been attached, if teacher wants to replace this with another one, by drag-and-drop or using the file picker, they get the standard message: $string['maxfilesreached'] = 'You are allowed to attach a maximum of {$a} file(s) to this item';
            For non-savvy users, this standard message does not tell them how they can replace the uploaded file with a new one, i.e. "Click on file icon/name" which will trigger a window where they can Delete, before uploading a new one.
            Of course this is not an issue related to the issue at hand (Lesson) and maybe a new issue should be raised?

            Thanks again,
            Joseph

            Show
            Joseph Rézeau added a comment - Hi Mark, You're right, I had not updated to your latest version. Just re-tested your patch now (on moodle 2.4 latest) and everything works fine. Many thanks for working on this (and not taking into account my pessimistic view of the matter where I went as fas as suggesting that that popup feature should be removed altogether from Lesson)... Just a small point. Once a file has been attached, if teacher wants to replace this with another one, by drag-and-drop or using the file picker, they get the standard message: $string ['maxfilesreached'] = 'You are allowed to attach a maximum of {$a} file(s) to this item'; For non-savvy users, this standard message does not tell them how they can replace the uploaded file with a new one, i.e. "Click on file icon/name" which will trigger a window where they can Delete, before uploading a new one. Of course this is not an issue related to the issue at hand (Lesson) and maybe a new issue should be raised? Thanks again, Joseph
            Hide
            Sam Hemelryk added a comment -

            Thanks for fixing that up Mark, this has been integrated now

            Show
            Sam Hemelryk added a comment - Thanks for fixing that up Mark, this has been integrated now
            Hide
            Sam Hemelryk added a comment -

            Thanks Joseph for testing.

            I've marked this tested now as you're already tested it, and MDLQA-5407 will see it tested again.

            Many thanks
            Sam

            Show
            Sam Hemelryk added a comment - Thanks Joseph for testing. I've marked this tested now as you're already tested it, and MDLQA-5407 will see it tested again. Many thanks Sam
            Hide
            Mark Nelson added a comment -

            Hi Joseph, the change in the language string to be more descriptive is indeed another tracker issue, feel free to create it. Thanks!

            Show
            Mark Nelson added a comment - Hi Joseph, the change in the language string to be more descriptive is indeed another tracker issue, feel free to create it. Thanks!
            Hide
            Dan Poltawski added a comment -

            Congratulations! Another bug solved.. only another 7330 to go, thanks for contributing to contributing to 0.8% of all bugs being fixed this week!

            ciao
            Dan

            Show
            Dan Poltawski added a comment - Congratulations! Another bug solved.. only another 7330 to go, thanks for contributing to contributing to 0.8% of all bugs being fixed this week! ciao Dan

              People

              • Votes:
                18 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: