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

Add get_title() to auth base class

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.9, 2.0
    • Component/s: Authentication
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE

      Description

      I propose adding the following method to the auth base class:
      /**

      • Return the properly translated human-friendly title of this auth plugin
        */
        function get_title()

      This places responsibility for working out the auth plugin's title within the auth plugin object, rather than having the code to do so in other places. This could also reduce code reuse. There would be little need for sub-classes to overload the method.

      The attached patch adds the function, and replaces the code in admin/auth.php with a call to the new function.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            phildriscoll Phil Driscoll added a comment -

            There is also some descriptive text for the auth plugin, so I would suggest the addition of
            function get_description()

            Show
            phildriscoll Phil Driscoll added a comment - There is also some descriptive text for the auth plugin, so I would suggest the addition of function get_description()
            Hide
            lukehudson Luke Hudson added a comment -

            In that case, let's make two methods. Some code only needs the translated title, others would need the description..

            $title = $auth->get_title()
            $desc = $auth->get_description()

            ^^ This looks nice and clear to me...

            Show
            lukehudson Luke Hudson added a comment - In that case, let's make two methods. Some code only needs the translated title, others would need the description.. $title = $auth->get_title() $desc = $auth->get_description() ^^ This looks nice and clear to me...
            Hide
            mjollnir Penny Leach added a comment -

            Luke - this patch seems to just add get_title - did you make another one that adds get_description too ?

            Show
            mjollnir Penny Leach added a comment - Luke - this patch seems to just add get_title - did you make another one that adds get_description too ?
            Hide
            lukehudson Luke Hudson added a comment -

            get_title() and get_description() added to auth base class.

            Show
            lukehudson Luke Hudson added a comment - get_title() and get_description() added to auth base class.
            Hide
            dougiamas Martin Dougiamas added a comment -

            Yes please!

            Show
            dougiamas Martin Dougiamas added a comment - Yes please!
            Hide
            mjollnir Penny Leach added a comment -

            Luke - I just went to merge this and I don't think your patch covers all the cases. Here's where the title needs to be replaced for example (I haven't bothered looking for description thus far):

            http://paste.dollyfish.net.nz/20ebea

            Show
            mjollnir Penny Leach added a comment - Luke - I just went to merge this and I don't think your patch covers all the cases. Here's where the title needs to be replaced for example (I haven't bothered looking for description thus far): http://paste.dollyfish.net.nz/20ebea
            Hide
            lukehudson Luke Hudson added a comment -

            Patches lib/authlib to add the functions, and makes changes to various places, so the new functions are called.

            Show
            lukehudson Luke Hudson added a comment - Patches lib/authlib to add the functions, and makes changes to various places, so the new functions are called.
            Hide
            mjollnir Penny Leach added a comment -

            ok, I put in head & stable. thanks luke!

            Show
            mjollnir Penny Leach added a comment - ok, I put in head & stable. thanks luke!

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Mar/08