Moodle
  1. Moodle
  2. MDL-43109

When editing a wiki page the default value for the tags is not being populated correctly

        Details

          Description

          When editing a wiki page the default value for the tags input box is empty even if there are tags. Saving the page will cause the tags to be deleted.

          Replication steps:

          1. Log in as teacher/admin.
          2. Create a Wiki activity.
          3. Add an initial page including multiple "Other" tags, separated by commas.
          4. Save the page.
          5. Click "Edit" in the tabs at the top.

          Expected result: The previously added tags should appear in the editing form.

          Actual result: No tags appear and the following error appears for each missing tag...

          Warning: Invalid argument supplied for foreach() in D:\xampp\htdocs\master_integration\lib\form\tags.php on line 216

            Gliffy Diagrams

              Issue Links

              Discovered while testing

              Task - A task that needs to be completed. Tasks usually refer to work that must be done outside the product. MDL-43673 Cover wiki page tags edition with acceptance test

              • Minor - Minor loss of function where workaround is possible
              • Closed

                Activity

                Ascending order - Click to sort in descending order
                Hide
                Permalink
                Michael Wylde added a comment - - edited

                Please find a link to my commit which fixes this issue:
                https://github.com/mdikih/moodle/commit/99c9638c483234621ed3704f75598f7d88c47e44

                Any feedback is appreciated.

                Thanks,
                Michael

                Show
                Michael Wylde added a comment - - edited Please find a link to my commit which fixes this issue: https://github.com/mdikih/moodle/commit/99c9638c483234621ed3704f75598f7d88c47e44 Any feedback is appreciated. Thanks, Michael
                Hide
                Permalink
                Michael de Raadt added a comment -

                Thanks for reporting that and providing a fix, Michael and Adam.

                This should be backported to earlier supported branches.

                Show
                Michael de Raadt added a comment - Thanks for reporting that and providing a fix, Michael and Adam. This should be backported to earlier supported branches.
                Hide
                Permalink
                Ankit Agarwal added a comment -

                Hi Guys,
                Thanks for working on this issue and providing a patch. The patch looks good for the most part. Just wondering if we need these two lines any more

                        if (!empty($CFG->usetags)) {
                		 
                            $params['tags'] = tag_get_tags_csv('wiki_pages', $this->page->id, TAG_RETURN_TEXT);
                		 
                        }

                Also consider back-porting this issue to stables.
                I would be watching this issue and will push for integration, once these concerns are addressed.
                Thanks

                Show
                Ankit Agarwal added a comment - Hi Guys, Thanks for working on this issue and providing a patch. The patch looks good for the most part. Just wondering if we need these two lines any more if (!empty($CFG->usetags)) { $params['tags'] = tag_get_tags_csv('wiki_pages', $this->page->id, TAG_RETURN_TEXT); } Also consider back-porting this issue to stables. I would be watching this issue and will push for integration, once these concerns are addressed. Thanks
                Hide
                Permalink
                Michael Wylde added a comment -

                Hi Ankit,

                I have removed the code like you suggested. I have applied this to master and back-ported to MOODLE_26_STABLE and MOODLE_25_STABLE. The commits are:
                25 - https://github.com/mdikih/moodle/commit/23353087ca50a9fb890201e17c05c83cebd7fc04
                26 - https://github.com/mdikih/moodle/commit/79f0a612c4d9db3c01d3ceec1d74d8e39d0b9275
                master - https://github.com/mdikih/moodle/commit/f2a4eac05aace2700cfc7198983a2b3273b6fe5a

                Thanks,
                Michael

                Show
                Michael Wylde added a comment - Hi Ankit, I have removed the code like you suggested. I have applied this to master and back-ported to MOODLE_26_STABLE and MOODLE_25_STABLE. The commits are: 25 - https://github.com/mdikih/moodle/commit/23353087ca50a9fb890201e17c05c83cebd7fc04 26 - https://github.com/mdikih/moodle/commit/79f0a612c4d9db3c01d3ceec1d74d8e39d0b9275 master - https://github.com/mdikih/moodle/commit/f2a4eac05aace2700cfc7198983a2b3273b6fe5a Thanks, Michael
                Hide
                Permalink
                Ankit Agarwal added a comment -

                Thanks for the changes,
                Pushing forward.

                Show
                Ankit Agarwal added a comment - Thanks for the changes, Pushing forward.
                Hide
                Permalink
                Eloy Lafuente (stronk7) added a comment -

                Integrated (25, 26 & master), thanks!

                Side note, I've created MDL-43673 to cover this with acceptance tests (note that we should try to always provide them).

                Ciao

                Show
                Eloy Lafuente (stronk7) added a comment - Integrated (25, 26 & master), thanks! Side note, I've created MDL-43673 to cover this with acceptance tests (note that we should try to always provide them). Ciao
                Hide
                Permalink
                Rossiani Wijaya added a comment -

                This is working as expected.

                Tested for 2.5, 2.6 and master

                Test passed.

                Show
                Rossiani Wijaya added a comment - This is working as expected. Tested for 2.5, 2.6 and master Test passed.
                Hide
                Permalink
                Eloy Lafuente (stronk7) added a comment -

                Well done is better than well said.

                Closing, big thanks!

                Show
                Eloy Lafuente (stronk7) added a comment - Well done is better than well said. Closing, big thanks!

                  People

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

                    Dates

                    • Created:
                      Updated:
                      Resolved: