Details

    • Testing Instructions:
      Hide

      For each changed branch, inspect the file mod/lti/lang/en/lti.php to make sure that none of the string values are instances of the words Instructor or Instructors (including lower case variations). The two exceptions to this are imsroleadmin and imsroleinstructor which will still contain references to Instructor in their strings.

      Show
      For each changed branch, inspect the file mod/lti/lang/en/lti.php to make sure that none of the string values are instances of the words Instructor or Instructors (including lower case variations). The two exceptions to this are imsroleadmin and imsroleinstructor which will still contain references to Instructor in their strings.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      m_MDL-30935_lti_change_instructor_role_to_teacher
    • Rank:
      33945

      Description

      The lag files for LTI module uses the term instructor. I assume that in Moodle role model a teacher is meant. Please change the term 'instructor' to 'teacher' for better understanding or correct me.

      Please also add LTI as component here.

        Issue Links

          Activity

          Hide
          Helen Foster added a comment -

          Ralf, thanks for your report. I agree that teacher, rather than instructor should be used in the language strings.

          Adding component External Tool (IMS-LTI).

          Show
          Helen Foster added a comment - Ralf, thanks for your report. I agree that teacher, rather than instructor should be used in the language strings. Adding component External Tool (IMS-LTI).
          Hide
          Gerard Caulfield added a comment -

          fyi: Patch for 2.1 not submitted as it seems this issue didn't exist in 2.1

          Show
          Gerard Caulfield added a comment - fyi: Patch for 2.1 not submitted as it seems this issue didn't exist in 2.1
          Hide
          Sam Hemelryk added a comment -

          Thanks Gerry - this has been integrated now.
          The LTI module was added in Moodle 2.2 hence no 2.1 changes

          Show
          Sam Hemelryk added a comment - Thanks Gerry - this has been integrated now. The LTI module was added in Moodle 2.2 hence no 2.1 changes
          Hide
          Gerard Caulfield added a comment -

          Ah I see. Thanks Sam.

          Show
          Gerard Caulfield added a comment - Ah I see. Thanks Sam.
          Hide
          Andrew Davis added a comment -

          Hi. There are still 3 instances of "instructor" in the lang file in master. Lower case i. Perhaps you were doing a case sensitive search.

          $string['allowinstructorcustom'] = 'Allow instructors to add custom parameters';

          $string['show_in_course_help'] = 'If selected, this tool configuration will appear in the "External tool type" dropdown when instructors

          $string['typename_help'] = 'The tool name is used to identify the tool provider within Moodle. The name entered will be visible
          to instructors when adding external tools within courses.';

          Should I fail this back to you or am I misunderstanding?

          Show
          Andrew Davis added a comment - Hi. There are still 3 instances of "instructor" in the lang file in master. Lower case i. Perhaps you were doing a case sensitive search. $string ['allowinstructorcustom'] = 'Allow instructors to add custom parameters'; $string ['show_in_course_help'] = 'If selected, this tool configuration will appear in the "External tool type" dropdown when instructors $string ['typename_help'] = 'The tool name is used to identify the tool provider within Moodle. The name entered will be visible to instructors when adding external tools within courses.'; Should I fail this back to you or am I misunderstanding?
          Hide
          Andrew Davis added a comment -

          After a chat with Gerard Im marking this failed.

          Show
          Andrew Davis added a comment - After a chat with Gerard Im marking this failed.
          Hide
          Gerard Caulfield added a comment -

          I spoke with Andrew over chat, but just so things are in the public I've said that these are indeed a fail.

          Show
          Gerard Caulfield added a comment - I spoke with Andrew over chat, but just so things are in the public I've said that these are indeed a fail.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Offtopic: Well, apart than "instructor" is official LTI role-name and that the component lead (Chris) should have been warned / assigned / reviewed this...

          Show
          Eloy Lafuente (stronk7) added a comment - Offtopic: Well, apart than "instructor" is official LTI role-name and that the component lead (Chris) should have been warned / assigned / reviewed this...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          (adding Chris here as watcher to let him know)

          Show
          Eloy Lafuente (stronk7) added a comment - (adding Chris here as watcher to let him know)
          Hide
          Gerard Caulfield added a comment -

          I did not realise this, thanks Eloy.

          Show
          Gerard Caulfield added a comment - I did not realise this, thanks Eloy.
          Hide
          Sam Hemelryk added a comment -

          Two options here guys:

          1. We revert this.
          2. We correct the three strings now so that we are still in time to release.

          What's it to be?

          LTI terminology vs. Moodle terminology, for no reason other than my personal preference I think we should stick with this change from instructor => teacher in strings.
          But I would be very keen to hear your thoughts Chris.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Two options here guys: 1. We revert this. 2. We correct the three strings now so that we are still in time to release. What's it to be? LTI terminology vs. Moodle terminology, for no reason other than my personal preference I think we should stick with this change from instructor => teacher in strings. But I would be very keen to hear your thoughts Chris. Cheers Sam
          Hide
          Chris Scribner added a comment -

          The only one we shouldn't change is this:

          $string['imsroleadmin'] = 'Instructor,Administrator';
          $string['imsroleinstructor'] = 'Instructor';

          Actually, I don't even recall those being used. We can either revert those 2 or verify they're not used and remove completely.

          All the other usages of instructor are referring to the Moodle concept of an educator, so teacher looks correct.

          Show
          Chris Scribner added a comment - The only one we shouldn't change is this: $string ['imsroleadmin'] = 'Instructor,Administrator'; $string ['imsroleinstructor'] = 'Instructor'; Actually, I don't even recall those being used. We can either revert those 2 or verify they're not used and remove completely. All the other usages of instructor are referring to the Moodle concept of an educator, so teacher looks correct.
          Hide
          Sam Hemelryk added a comment -

          Awesome thanks for looking at this Chris.

          From here it looks to me like we should:

          1. Fix the 3 strings identified by Andrew
          2. Revert the two strings you identified imsroleadmin, and imsroleinstructor
          3. Open a new issue to remove those two strings if they are not needed.

          All yours again Gerry.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Awesome thanks for looking at this Chris. From here it looks to me like we should: Fix the 3 strings identified by Andrew Revert the two strings you identified imsroleadmin, and imsroleinstructor Open a new issue to remove those two strings if they are not needed. All yours again Gerry. Cheers Sam
          Hide
          Gerard Caulfield added a comment -

          Thanks Sam, Chris and Eloy.

          Show
          Gerard Caulfield added a comment - Thanks Sam, Chris and Eloy.
          Hide
          Gerard Caulfield added a comment - - edited

          I've made the changes requested in 1 and 2. I've also searched for usages of imsroleadmin and imsroleinstructor in master and 2.2 but could not find any instances other than the language file itself. Is there a possibility that these could be used outside of the Moodle code base though and if so, do we have to be concerned by such a case?

          Show
          Gerard Caulfield added a comment - - edited I've made the changes requested in 1 and 2. I've also searched for usages of imsroleadmin and imsroleinstructor in master and 2.2 but could not find any instances other than the language file itself. Is there a possibility that these could be used outside of the Moodle code base though and if so, do we have to be concerned by such a case?
          Hide
          Sam Hemelryk added a comment -

          Thanks Gerry - this has been integrated now.

          In regards to those two strings I didn't find any reference for them either, we're best to create a new issue for it and then remove them in master only should be find they are not used within core.

          Up for testing again thanks Andrew.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Gerry - this has been integrated now. In regards to those two strings I didn't find any reference for them either, we're best to create a new issue for it and then remove them in master only should be find they are not used within core. Up for testing again thanks Andrew. Cheers Sam
          Hide
          Andrew Davis added a comment -

          Ok. the only strings containing 'instructor' are...

          $string['imsroleadmin'] = 'Instructor,Administrator';
          $string['imsroleinstructor'] = 'Instructor';

          These shouldnt be there according to the test instructions. Reading back on the comments should there be an MDL to remove these or should they have been removed already?

          Show
          Andrew Davis added a comment - Ok. the only strings containing 'instructor' are... $string ['imsroleadmin'] = 'Instructor,Administrator'; $string ['imsroleinstructor'] = 'Instructor'; These shouldnt be there according to the test instructions. Reading back on the comments should there be an MDL to remove these or should they have been removed already?
          Hide
          Gerard Caulfield added a comment -

          I need to update the test... doing so now.

          Show
          Gerard Caulfield added a comment - I need to update the test... doing so now.
          Hide
          Gerard Caulfield added a comment -

          creating the MDL now

          Show
          Gerard Caulfield added a comment - creating the MDL now
          Hide
          Gerard Caulfield added a comment -

          I have created MDL-31657 to address the imsroleadmin, imsroleinstructor issue.

          Show
          Gerard Caulfield added a comment - I have created MDL-31657 to address the imsroleadmin, imsroleinstructor issue.
          Hide
          Andrew Davis added a comment -

          Passing

          Show
          Andrew Davis added a comment - Passing
          Hide
          Eloy Lafuente (stronk7) added a comment -

          It is late here and I'm very tired but I didn't want to go to sleep before expressing my admiration for your amazing collaboration. Thanks!

          Closing as fixed, heading to zzzZZZzzz, niao

          Show
          Eloy Lafuente (stronk7) added a comment - It is late here and I'm very tired but I didn't want to go to sleep before expressing my admiration for your amazing collaboration. Thanks! Closing as fixed, heading to zzzZZZzzz, niao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: