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

          Attachments

            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