Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29086

deleting database activity preset is not possible

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide

      1) Login as admin
      2) Create a database activity
      3) Add one Database Field
      4) Create one template (just click save template as soon as you enter the form)
      5) Click the Presets tab
      6) Go to the 'Save as preset' section and click continue
      [TEST] After clicking continue and returning to the Presets tab check to make sure that your preset is now present in the 'Use a preset' section and that there is an X located next to it to delete it.

      [Please note that if you have any presets that you created before this fix they will not have an X next to them]

      7) Delete the preset.
      [TEST] make sure that the preset has been deleted.

      Show
      1) Login as admin 2) Create a database activity 3) Add one Database Field 4) Create one template (just click save template as soon as you enter the form) 5) Click the Presets tab 6) Go to the 'Save as preset' section and click continue [TEST] After clicking continue and returning to the Presets tab check to make sure that your preset is now present in the 'Use a preset' section and that there is an X located next to it to delete it. [Please note that if you have any presets that you created before this fix they will not have an X next to them] 7) Delete the preset. [TEST] make sure that the preset has been deleted.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-29086-master

      Description

      If you use "Save as preset" there is no way to delete this preset. Tested as Admin and Manager. It seems that mod/data:manageuserpresets didn't work.

      Replication steps:

      1. Login as Admin / Manager
      2. Create Database activity
      3. Add one Database Field
      4. Create one template (like Add template)
      5. Save this template
      6. goto Presets
      7. Save as preset

      Now you can see and chose the preset, but there is no way to deleting it.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for reporting this.

              I've put that on the backlog.

              In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.

              Show
              salvetore Michael de Raadt added a comment - Thanks for reporting this. I've put that on the backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.
              Hide
              salvetore Michael de Raadt added a comment -

              A comment from the duplicate bug:

              "As far as I can tell from a quick investigation, sharing a preset by 'Save as preset' should but doesn't register the user id of the saver. The display of the delete button is conditioned by user id but since the user id is 0, the button is not displayed."

              Show
              salvetore Michael de Raadt added a comment - A comment from the duplicate bug: "As far as I can tell from a quick investigation, sharing a preset by 'Save as preset' should but doesn't register the user id of the saver. The display of the delete button is conditioned by user id but since the user id is 0, the button is not displayed."
              Hide
              atze Matthias Hunstock added a comment -

              I can confirm the previous comment:
              if the column "userid" in table "mdl_files" is updated to the ID of the user who created the preset then the delete button is shown

              BUT

              the button doesn't work correctly. When pressing it, there is the confirmation page, on confirming the deletion it prints "deleted successfully", but the preset ist still there.

              HTH

              Show
              atze Matthias Hunstock added a comment - I can confirm the previous comment: if the column "userid" in table "mdl_files" is updated to the ID of the user who created the preset then the delete button is shown BUT the button doesn't work correctly. When pressing it, there is the confirmation page, on confirming the deletion it prints "deleted successfully", but the preset ist still there. HTH
              Hide
              atze Matthias Hunstock added a comment -

              In line 187 of mod/data/preset.php there is a call to "fulldelete" (in /lib/filelib.php) which tries to delete the preset from /var/moodledata/data/preset/<id>/<name> but in a fresh install I do not even have /var/moodledata/data. The presets do instead reside in /var/moodledata/filedir. Seems like there was an incomplete API change or something.

              Show
              atze Matthias Hunstock added a comment - In line 187 of mod/data/preset.php there is a call to "fulldelete" (in /lib/filelib.php) which tries to delete the preset from /var/moodledata/data/preset/<id>/<name> but in a fresh install I do not even have /var/moodledata/data. The presets do instead reside in /var/moodledata/filedir. Seems like there was an incomplete API change or something.
              Hide
              atze Matthias Hunstock added a comment -

              Patches the missing userid in the table mdl_files when creating a preset with mod/data.

              Show
              atze Matthias Hunstock added a comment - Patches the missing userid in the table mdl_files when creating a preset with mod/data.
              Hide
              atze Matthias Hunstock added a comment -

              patches a missing function call to delete database presets

              Show
              atze Matthias Hunstock added a comment - patches a missing function call to delete database presets
              Hide
              abgreeve Adrian Greeve added a comment -

              Thanks to Matthias for putting in the hard work for this. I noticed that data_delete_site_preset() was actually just looking for a preset name and not a full file path. I've fixed that. Now heading to peer review.

              Show
              abgreeve Adrian Greeve added a comment - Thanks to Matthias for putting in the hard work for this. I noticed that data_delete_site_preset() was actually just looking for a preset name and not a full file path. I've fixed that. Now heading to peer review.
              Hide
              poltawski Dan Poltawski added a comment -

              Makes sense and looks good please submit for integration!

              As we just discussed the test instructions need a tweak to point out that if the preset existed before the change then they won't have the delete option. Creating another issue about the fact you can overwrite any preset..

              Show
              poltawski Dan Poltawski added a comment - Makes sense and looks good please submit for integration! As we just discussed the test instructions need a tweak to point out that if the preset existed before the change then they won't have the delete option. Creating another issue about the fact you can overwrite any preset..
              Hide
              nebgor Aparup Banerjee added a comment - - edited

              Adrian, the author should be you here since you created this git commmit (even if verbatim). Credits can be mentioned in commit messages.

              Show
              nebgor Aparup Banerjee added a comment - - edited Adrian, the author should be you here since you created this git commmit (even if verbatim). Credits can be mentioned in commit messages.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited

              So, just for clarification, this only enables deletion of new presets which have userid defined, correct?

              And the linked MDL-31763 will solve BOTH the (potential) overwrite problem AND the deletion of presets without userid assigned.

              Plz, confirm. I'll be happy to integrate this once the Q above is clarified.

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited So, just for clarification, this only enables deletion of new presets which have userid defined, correct? And the linked MDL-31763 will solve BOTH the (potential) overwrite problem AND the deletion of presets without userid assigned. Plz, confirm. I'll be happy to integrate this once the Q above is clarified. Ciao
              Hide
              abgreeve Adrian Greeve added a comment -

              Yes, that is correct. Only if the userid has been defined will it allow for the preset to be deleted. MDL-31763 was created to fix presets being overwritten by anyone. I think that it would be a good idea to have the ability to delete any old presets that exist as well. I'll leave a comment says as much on that issue.

              Thanks,

              Show
              abgreeve Adrian Greeve added a comment - Yes, that is correct. Only if the userid has been defined will it allow for the preset to be deleted. MDL-31763 was created to fix presets being overwritten by anyone. I think that it would be a good idea to have the ability to delete any old presets that exist as well. I'll leave a comment says as much on that issue. Thanks,
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Thanks, about to integrate this now...

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Thanks, about to integrate this now...
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Integrated, thanks! (21, 22 & master)

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (21, 22 & master)
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Hi Eloy,
              I guess that you forgot to integrate it to 21? it works fine in master and 22. I cannot find the commit in the 21 logs. Please have a look.
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - Hi Eloy, I guess that you forgot to integrate it to 21? it works fine in master and 22. I cannot find the commit in the 21 logs. Please have a look. Thanks
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Thanks Eloy for sending in the commit.
              All good now.
              Passing
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - Thanks Eloy for sending in the commit. All good now. Passing Thanks
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!).

              icao_reverse('arreis olik rebemevon afla letoh ognat');

              Closing, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!). icao_reverse('arreis olik rebemevon afla letoh ognat'); Closing, ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    12/Mar/12