Moodle
  1. Moodle
  2. MDL-33624

String invalidcourseid - missing content for placeholder

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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:
    • Rank:
      41569

      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.

        Issue Links

          Activity

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

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

          Thanks

          Show
          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 Agarwal added a comment -

          Linking a duplicate of this issue.

          Show
          Ankit Agarwal added a comment - Linking a duplicate of this issue.
          Hide
          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
          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
          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
          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
          Dan Poltawski added a comment -

          Thanks Sam - i've updated the branches now.

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

          Awesome thanks Dan, has been integrated now

          Show
          Sam Hemelryk added a comment - Awesome thanks Dan, has been integrated now
          Hide
          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
          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
          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
          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
          Sam Hemelryk added a comment -

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

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

          Thanks Dan and Sam,

          All good, passing test.

          Show
          Rajesh Taneja added a comment - Thanks Dan and Sam, All good, passing test.
          Hide
          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
          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: