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

require_login simplified and improved

XMLWordPrintable

    • MOODLE_26_STABLE
    • wip-MDL-44615-master
    • Hide

      To check for regressions we actually need to go through all affected pages. Luckily most of them are covered by behat tests.

      Create IMSCP and LTI modules and navigate through different pages.

      Errors check:

      1. Create several course categories, make some hidden
      2. Create several coruses, make some hidden
      3. In a visible course create several modules, make some hidden or otherwise inaccessible
      4. Login in one browser as admin (for copying the links)
      5. Login in another browser as regular user
      6. Try requesting pages course/index.php, course/info.php, course/view.php with different id's (copy-paste from admin session)
      7. Make sure that for the user there is no difference in error messages when course is missing or hidden. Same with category
      8. As a user try to access module view page, make sure there is no difference in error messages between: course module does not exist, course module is not available (but user is enrolled in the course), course module is in the course that user can't access.
      9. Log out the user
      10. As a non-logged in user try to type URLs for non-existing or hidden course / category / module, make sure you are redirected to login page and not given any error at all
      Show
      To check for regressions we actually need to go through all affected pages. Luckily most of them are covered by behat tests. Create IMSCP and LTI modules and navigate through different pages. Errors check: Create several course categories, make some hidden Create several coruses, make some hidden In a visible course create several modules, make some hidden or otherwise inaccessible Login in one browser as admin (for copying the links) Login in another browser as regular user Try requesting pages course/index.php, course/info.php, course/view.php with different id's (copy-paste from admin session) Make sure that for the user there is no difference in error messages when course is missing or hidden. Same with category As a user try to access module view page, make sure there is no difference in error messages between: course module does not exist, course module is not available (but user is enrolled in the course), course module is in the course that user can't access. Log out the user As a non-logged in user try to type URLs for non-existing or hidden course / category / module, make sure you are redirected to login page and not given any error at all

      This issue has three main goals

      Goal 1: Performance for course modules.

      At some point function require_login() started calling get_fast_modinfo() and taking the course module from there ignoring the comprehensive $cm object passed as argument. Therefore we call very expensive function get_coursemodule_from_id() on EACH module page for nothing. Removing the call to this function gives us 2-3 queries less per page.

      Goal 2: Code readability.

      Before calling require_login() we usually retrieve lots of records from DB and repeat very similar code on each page.

      old code:

      $cmrecord = get_coursemodule_from_id('forum', $cmid, 0, true, MUST_EXIST);
      		 
      $course = $DB->get_record('course', array('id' => $cm->course), '*', MUST_EXIST);
      		 
      $forum = $DB->get_record("forum", array("id" => $cm->instance), '*', MUST_EXIST);
      		 
      require_course_login($course, true, $cmrecord);
      		 
      $context = context_module::instance($cmrecord->id);
      		 
      $cminfo = $PAGE->cm; // or: get_fast_modinfo($course)->get_cm($id);

      new code:

      list($context, $course, $cminfo) = $PAGE->login_to_cm('forum', $cmid);
      		 
      $forum = $PAGE->activityrecord;
      		 
      $cmrecord = $cminfo->get_course_module_record(true);

      another useful function:

      list($context, $course, $cminfo) = $PAGE->login_to_activity('forum', $forumid);

      $cminfo (cm_info) can almost always be used instead of $cmrecord (stdClass). It has more fields, saves additional queries in group functions. But cm_info properties are read-only.

      Another example of better readability is substituting arguments with named constants:
      OLD:

      require_login($course, false);

      NEW:

      $PAGE->login($course, PAGELOGIN_NO_AUTOLOGIN);

      OLD:

      require_course_login($course, true, $cm);

      NEW:

      $PAGE->login_to_cm(null, $cm, $course, PAGELOGIN_ALLOW_FRONTPAGE_GUEST);

      Goal 3. Privacy.

      All exceptions raised during login will be masked with require_login_exception. Therefore modules will not disclose information about particular error (missing course module, missing course, etc.). If page needs to perform additional queries before calling login it can use function login_expected():

      $PAGE->login_expected(PAGELOGIN_NO_AUTOLOGIN);
      		 
      $entry = $DB->get_record('glossary_entries', array('id'=> $eid), '*', MUST_EXIST);
      		 
      list($context, $course, $cm) = $PAGE->login_to_activity('glossary', $entry->glossaryid);

            marina Marina Glancy
            marina Marina Glancy
            Votes:
            4 Vote for this issue
            Watchers:
            12 Start watching this issue

              Created:
              Updated:
              Resolved:

                Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.