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

      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.

        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: