Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.5.5, 2.6.2
    • Fix Version/s: 2.5.6, 2.6.3
    • Component/s: Wiki (2.x)
    • Labels:
    • Testing Instructions:
      Hide

      1. Create an individual wiki.
      2. Edit and save a page with content.
      3. Search. Verify that you get correct results.
      4. Add additional individual subwikis to that search.
      5. Switch to one. Search. Verify that you get correct results.
      6. Create a collaborative wiki.
      7. Add pages. Search. Verify that you get correct results.

      Show
      1. Create an individual wiki. 2. Edit and save a page with content. 3. Search. Verify that you get correct results. 4. Add additional individual subwikis to that search. 5. Switch to one. Search. Verify that you get correct results. 6. Create a collaborative wiki. 7. Add pages. Search. Verify that you get correct results.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      wip-MDL-33486-master

      Description

      When using individual wikis, trying to search (/mod/wiki/search.php) returns a completely blank page - no header, footer or debug information.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            tsala Helen Foster added a comment -

            James, thanks for reporting this issue. Just adding a link to a forum discussion about it: http://moodle.org/mod/forum/discuss.php?d=203934

            Show
            tsala Helen Foster added a comment - James, thanks for reporting this issue. Just adding a link to a forum discussion about it: http://moodle.org/mod/forum/discuss.php?d=203934
            Hide
            cfulton Charles Fulton added a comment -

            I'm not sure if I'm taking the right approach but here's a patch. The problem is that the existing search code doesn't pass a uid parameter to wiki_get_subwiki_by_group() so that call always fails with individual wikis. For some reason it's set to return if that fails, thus the blank screen (odd).

            My approach adds a subwiki parameter to the search form, since we already know what subwiki we're on when searching and as far as I can tell search only searches within a subwiki anyway. I can't see a reason not to do it this way but I may have missed something.

            Show
            cfulton Charles Fulton added a comment - I'm not sure if I'm taking the right approach but here's a patch. The problem is that the existing search code doesn't pass a uid parameter to wiki_get_subwiki_by_group() so that call always fails with individual wikis. For some reason it's set to return if that fails, thus the blank screen (odd). My approach adds a subwiki parameter to the search form, since we already know what subwiki we're on when searching and as far as I can tell search only searches within a subwiki anyway. I can't see a reason not to do it this way but I may have missed something.
            Hide
            abgreeve Adrian Greeve added a comment -

            Hi Charles,

            Thanks for the fix. The code looks good. The only problem that I saw was in the extreme case where the user does a search on the wiki when there are no entries in the wiki. Obviously a subwiki ID hasn't been generated and an error is displayed. This may be more likely to happen when there are groups enabled, but as you pointed out, you require a subwiki ID to do a search.

            Ideally I think it would be nice to to see the search page say that it found no results rather than an error in that situation.

            Show
            abgreeve Adrian Greeve added a comment - Hi Charles, Thanks for the fix. The code looks good. The only problem that I saw was in the extreme case where the user does a search on the wiki when there are no entries in the wiki. Obviously a subwiki ID hasn't been generated and an error is displayed. This may be more likely to happen when there are groups enabled, but as you pointed out, you require a subwiki ID to do a search. Ideally I think it would be nice to to see the search page say that it found no results rather than an error in that situation.
            Hide
            cfulton Charles Fulton added a comment -

            Hi Adrian,

            Thanks for taking a look at this. I've dealt with those use cases, and added a new check to the search page which verifies that the user has the permission to view the subwiki they're searching.

            Show
            cfulton Charles Fulton added a comment - Hi Adrian, Thanks for taking a look at this. I've dealt with those use cases, and added a new check to the search page which verifies that the user has the permission to view the subwiki they're searching.
            Hide
            abgreeve Adrian Greeve added a comment -

            Hi Charles,

            Sorry about the wait for this peer review.

            After looking at this patch I have a couple of points.

            • mod/wiki/lib.php line 480: There is an extra " mark at the end which produces a quote mark between the search box and search button.
            • mod/wiki/pagelib.php line 108: When creating a page for the first you get the following notice:

            Notice: Trying to get property of non-object in /var/www/repositories/master/moodle/mod/wiki/pagelib.php on line 108

            $this->subwiki->id is blank at this stage and that is the reason for this notice.

            If you try to search the wiki from this page then you get a new notice:

            This page did not call $PAGE->set_url(...).

            One possible solution could be to do a check in pagelib.php, possibly not showing the search box at this stage. This would remove the need for the checks that you are doing in search.php.

            Show
            abgreeve Adrian Greeve added a comment - Hi Charles, Sorry about the wait for this peer review. After looking at this patch I have a couple of points. mod/wiki/lib.php line 480: There is an extra " mark at the end which produces a quote mark between the search box and search button. mod/wiki/pagelib.php line 108: When creating a page for the first you get the following notice: Notice: Trying to get property of non-object in /var/www/repositories/master/moodle/mod/wiki/pagelib.php on line 108 $this->subwiki->id is blank at this stage and that is the reason for this notice. If you try to search the wiki from this page then you get a new notice: This page did not call $PAGE->set_url(...). One possible solution could be to do a check in pagelib.php, possibly not showing the search box at this stage. This would remove the need for the checks that you are doing in search.php.
            Hide
            marina Marina Glancy added a comment -

            Hi Charles, do you plan to continue working on this issue? Thanks

            Show
            marina Marina Glancy added a comment - Hi Charles, do you plan to continue working on this issue? Thanks
            Hide
            cfulton Charles Fulton added a comment -

            Hi Marina, not in the near future.

            Show
            cfulton Charles Fulton added a comment - Hi Marina, not in the near future.
            Hide
            marina Marina Glancy added a comment -

            Thanks Charles, I'm posting here link to your branch for reference: https://github.com/mackensen/moodle/compare/master...MDL-33486-master

            Show
            marina Marina Glancy added a comment - Thanks Charles, I'm posting here link to your branch for reference: https://github.com/mackensen/moodle/compare/master...MDL-33486-master
            Hide
            marina Marina Glancy added a comment -

            Requesting peer review

            Show
            marina Marina Glancy added a comment - Requesting peer review
            Hide
            cibot CiBoT added a comment -

            Results for MDL-33486

            • Remote repository: git://github.com/marinaglancy/moodle.git
            Show
            cibot CiBoT added a comment - Results for MDL-33486 Remote repository: git://github.com/marinaglancy/moodle.git Remote branch wip- MDL-33486 -m25 to be integrated into upstream MOODLE_25_STABLE Executed job http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2497 Details: http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2497/artifact/work/smurf.html Remote branch wip- MDL-33486 -m26 to be integrated into upstream MOODLE_26_STABLE Executed job http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2498 Details: http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2498/artifact/work/smurf.html Remote branch wip- MDL-33486 -master to be integrated into upstream master Executed job http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2499 Details: http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2499/artifact/work/smurf.html
            Hide
            abgreeve Adrian Greeve added a comment -

            Thanks for picking this up Marina,

            Besides some simple stuff listed in the smurf file, everything else looks good. I particularly like the behat tests.

            Thanks again.

            Show
            abgreeve Adrian Greeve added a comment - Thanks for picking this up Marina, Besides some simple stuff listed in the smurf file, everything else looks good. I particularly like the behat tests. Thanks again.
            Hide
            marina Marina Glancy added a comment -

            Thanks Adrian for reviewing. It looks like codechecker does not know that lines are indented 4 spaces additionally after "case: " and moodlecheck does not know that phpdocs can be skipped if method overrides parent.
            The comments fullstop is unfortunately forgotten because I copied this code from view.php ...

            Show
            marina Marina Glancy added a comment - Thanks Adrian for reviewing. It looks like codechecker does not know that lines are indented 4 spaces additionally after "case: " and moodlecheck does not know that phpdocs can be skipped if method overrides parent. The comments fullstop is unfortunately forgotten because I copied this code from view.php ...
            Hide
            cibot CiBoT added a comment -

            Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

            Show
            cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated to master, 25 and 26 - thanks Marina!

            (I was getting weird issues with chrome behat on @mod_wiki, but they seemed unrelated to this issue, looked good on firefox).

            Show
            poltawski Dan Poltawski added a comment - Integrated to master, 25 and 26 - thanks Marina! (I was getting weird issues with chrome behat on @mod_wiki, but they seemed unrelated to this issue, looked good on firefox).
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for fixing this Marina,

            Works well for me.

            Passing...

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for fixing this Marina, Works well for me. Passing...
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks for your efforts, this change is now part of Moodle!

            Show
            poltawski Dan Poltawski added a comment - Thanks for your efforts, this change is now part of Moodle!

              People

              • Votes:
                14 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/May/14