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

String invalidcourseid - missing content for placeholder

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.3
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Language
    • Labels:
    • Testing Instructions:
      Hide

      Code review should be sufficient, else pass id invalid params to course/delete.php, course/externallib.php, enrol/authorize/index.php and nnotes/externallib.php and examine the string.

      Show
      Code review should be sufficient, else pass id invalid params to course/delete.php, course/externallib.php, enrol/authorize/index.php and nnotes/externallib.php and examine the string.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      The language strings
      [invalidcourseid,core_error]
      You are trying to use an invalid course ID: ({$a})
      contains a placeholder. In the Moodle interface, when entering an invalid course ID, the placeholder is not filled in. It just shows ({$a})

      Steps to reproduce: enter an invalid course number in the URL moodle/course/view.php?id=427 and you'll see the string appear.

      Suggested fix: alter the English string: it's clear which is the invalid course ID and I think the information is useless.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            poltawski Dan Poltawski added a comment -

            For these issues we tend to use the database call with MUST_EXIST these days rather than specific error messsages like this.

            But i've converted the few calls which were using the placeholder (vast majority were not).

            Show
            poltawski Dan Poltawski added a comment - For these issues we tend to use the database call with MUST_EXIST these days rather than specific error messsages like this. But i've converted the few calls which were using the placeholder (vast majority were not).
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi Dan,
            A few things:-

            1. there is also lang/en/notes.php:$string['invalidcourseid'] = 'Invalid course id: {$a}'; shouldn't we change this as well?
            2. Following needs to be fixed as well?

              course/externallib.php:            throw new moodle_exception('invalidcourseid', 'error', '', $params['courseid']);
              notes/externallib.php:                $errormessage = get_string('invalidcourseid', 'notes', $note['courseid']);

            3. In stables I guess we have a lot more instances of this (so I assume this cannot be integrated untill we are done with 2.3.1?)
            4. Testing instructions are needed.

            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi Dan, A few things:- there is also lang/en/notes.php:$string ['invalidcourseid'] = 'Invalid course id: {$a}'; shouldn't we change this as well? Following needs to be fixed as well? course/externallib.php: throw new moodle_exception('invalidcourseid', 'error', '', $params['courseid']); notes/externallib.php: $errormessage = get_string('invalidcourseid', 'notes', $note['courseid']); In stables I guess we have a lot more instances of this (so I assume this cannot be integrated untill we are done with 2.3.1?) Testing instructions are needed. Thanks
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Ankit,

            1. Well, we probably should get rid of that string, and same for notes/externalib
            2. True about course/externallib.php
            3. Don't understand?

            Show
            poltawski Dan Poltawski added a comment - Hi Ankit, 1. Well, we probably should get rid of that string, and same for notes/externalib 2. True about course/externallib.php 3. Don't understand?
            Hide
            ankit_frenz Ankit Agarwal added a comment - - edited

            Ignore 3 please.
            Just confirmed the use of string is exactly the same in 2.2

            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - - edited Ignore 3 please. Just confirmed the use of string is exactly the same in 2.2 Thanks
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Linking a duplicate of this issue.

            Show
            ankit_frenz Ankit Agarwal added a comment - Linking a duplicate of this issue.
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Dan,

            Changes look spot on here for master.
            Just noted one thing, we don't remove strings from stable branches so the 23 and 22 branches should leave notes.invalidcourseid there.
            Could you please update those branches and ping me when done so I can integrate this.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Dan, Changes look spot on here for master. Just noted one thing, we don't remove strings from stable branches so the 23 and 22 branches should leave notes.invalidcourseid there. Could you please update those branches and ping me when done so I can integrate this. Cheers Sam
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Sam - i've updated the branches now.

            Show
            poltawski Dan Poltawski added a comment - Thanks Sam - i've updated the branches now.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Awesome thanks Dan, has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Awesome thanks Dan, has been integrated now
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            In course/externallib.php, integration master has two more calls at line 948 and 952. Introduced by http://tracker.moodle.org/browse/MDL-32919 (Thanks to Dan for tracker issue)

            Waiting for further feedback, before passing/failing it.

            Show
            rajeshtaneja Rajesh Taneja added a comment - In course/externallib.php, integration master has two more calls at line 948 and 952. Introduced by http://tracker.moodle.org/browse/MDL-32919 (Thanks to Dan for tracker issue) Waiting for further feedback, before passing/failing it.
            Hide
            poltawski Dan Poltawski added a comment -

            I've created a patch to the issue on a branch off integration:

            git pull git://github.com/danpoltawski/moodle.git MDL-33624-integrationfix

            Note, with the associated issue it would've been better to use MUST_EXIST (perhaps we should become more militant about that ) because specifying the ID will probably be useful for webservices.

            Show
            poltawski Dan Poltawski added a comment - I've created a patch to the issue on a branch off integration: git pull git://github.com/danpoltawski/moodle.git MDL-33624 -integrationfix Note, with the associated issue it would've been better to use MUST_EXIST (perhaps we should become more militant about that ) because specifying the ID will probably be useful for webservices.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Good spotting and thanks for the quick fix! I've integrated the quick patch now

            Show
            samhemelryk Sam Hemelryk added a comment - Good spotting and thanks for the quick fix! I've integrated the quick patch now
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Dan and Sam,

            All good, passing test.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Dan and Sam, All good, passing test.
            Hide
            poltawski Dan Poltawski added a comment -

            *Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

            Congratulations

            {tracker.user.name}

            !

            You've made into Moodle

            {tracker.fixversion-1}

            +

            I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

            cheers!

            {tracker.friendlyintegrator}
            Show
            poltawski Dan Poltawski added a comment - * Notice *: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26 Congratulations {tracker.user.name} ! You've made into Moodle {tracker.fixversion-1} + I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world. cheers! {tracker.friendlyintegrator}

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Sep/12