search

brendanheywood / moodle-local_cleanurls

Lets drag Moodle's url structure into this century...
37 stars 24 forks source link

Development Branch: Loading Unclean URL #128

Open nyanginator opened 6 years ago

nyanginator commented 6 years ago

Hi, I wasn't sure where to post to reference the Development Branch, so sorry if I am posting in the wrong place.

EDIT: Added additional test case of /moodle/course/index.php

In the Master Branch, I could go to an unclean URL and have them resolve as follows:

However, in the Development Branch, I get:

Actually, I didn't even know about the last 2 test cases until I looked at the cleaner code. Where does this link (to display all mods of a certain course) usually appear?

I know this is an experimental side project, so any help or insight is very much appreciated.

Thanks again!

nyanginator commented 6 years ago

Narrowed it down to classes/url_rewriter.php. In the function html_head_setup_info(), when a new moodle_url is created with $ME, the Moodle directory (in my case, "'/moodle") is prepended to the path automatically. Doing a check for the Moodle directory at the start of the string, and removing it if present, seems to fix the issues I mentioned above.

private static function html_head_setup_info() {
    global $CFG, $ME, $PAGE;

    // Remove Moodle directory at beginning of URL, if present
    $wwwroot = new moodle_url($CFG->wwwroot);
    $wwwpath = $wwwroot->get_path();
    if (substr($ME, 0, strlen($wwwpath)) === $wwwpath) {
        $ME = substr($ME, strlen($wwwpath));
    }

    // Continue with the rest of the code as usual ...

    $me = new moodle_url($ME);
     ...
}
brendanheywood commented 6 years ago

thanks @nyanginator

I'm on long term leave from Catalyst so can't really spend much time on this. If you can put together a pull request including unit tests for this then someone will probably merge it in for you.

nyanginator commented 6 years ago

Thanks for the reply. I haven't done unit tests before, so it may take me awhile to read up on it and set everything up. From my initial tries, it looks like the Moodle setup for PHPUnit doesn't play nice with XAMPP, or at least how XAMPP is on my system. I'll be working on reinstalling my LAMP stack using tasksel (Ubuntu-based distro).

Do you or anybody else happen to have a link handy to a previous pull request with accompanying unit tests that I can use as a reference?

brendanheywood commented 6 years ago

In theory this should be everything you need:

https://docs.moodle.org/dev/PHPUnit

nyanginator commented 6 years ago

Thanks, I think I got it. Pull request https://github.com/brendanheywood/moodle-local_cleanurls/pull/129. Included additional test cases:

  1. Course view URL
    (e.g. http://www.example.com/moodle/course/view.php?id=5)
  2. Course users URL
    (e.g. http://www.example.com/moodle/user/?id=5)
  3. Course users URL with "index.php"
    (e.g. http://www.example.com/moodle/user/index.php?id=5)
  4. Course index URL
    (e.g. http://www.example.com/moodle/course/?categoryid=2)
  5. Course index URL with "index.php"
    (e.g. http://www.example.com/moodle/course/index.php?categoryid=2)
  6. Module URL
    (e.g. http://www.example.com/moodle/mod/page/?id=5)
  7. Module URL with "index.php"
    (e.g. http://www.example.com/moodle/mod/page/index.php?id=5)
  8. Module view URL
    (e.g. http://www.example.com/moodle/mod/page/view.php?id=9)
  9. User profile URL
    (e.g. http://www.example.com/moodle/user/profile.php?id=11)