Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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:

      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

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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 Glancy added a comment -

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

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

          Hi Marina, not in the near future.

          Show
          Charles Fulton added a comment - Hi Marina, not in the near future.
          Hide
          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 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 Glancy added a comment -

          Requesting peer review

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

          Results for MDL-33486

          • Remote repository: git://github.com/marinaglancy/moodle.git
          Show
          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
          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
          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 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 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 added a comment -

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

          Show
          CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
          Hide
          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
          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
          Rajesh Taneja added a comment -

          Thanks for fixing this Marina,

          Works well for me.

          Passing...

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

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

          Show
          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: