Moodle
  1. Moodle
  2. MDL-31657

Strings imsroleadmin and imsroleinstructor are unused

    Details

    • Rank:
      38230

      Description

      mod/lti/lang/en/lti.php contains references to imsroleadmin and imsroleinstructor which do not appear to be used anywhere. If this is in fact the case we should remove them in 2.3.

      It is probably a bad idea to remove them in current versions as this is more of an improvement than a fix and there is the small possibility that somebody could be calling these in code external to Moodle.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Up to you if you want to do this or close this issue, Chris.

          Show
          Michael de Raadt added a comment - Up to you if you want to do this or close this issue, Chris.
          Hide
          Gerard Caulfield added a comment -

          Just so it's clear, I was requested to create this issue here: http://tracker.moodle.org/browse/MDL-30935?focusedCommentId=144618#comment-144618

          Show
          Gerard Caulfield added a comment - Just so it's clear, I was requested to create this issue here: http://tracker.moodle.org/browse/MDL-30935?focusedCommentId=144618#comment-144618
          Hide
          Ralf Hilgenstock added a comment -

          Can you please also check the [setdefault,mod_lti]. Its using the term professor, that is never used in Moodle. Please replace by teacher.

          Show
          Ralf Hilgenstock added a comment - Can you please also check the [setdefault,mod_lti] . Its using the term professor, that is never used in Moodle. Please replace by teacher.
          Hide
          Chris Scribner added a comment -

          Fix is here:

          https://github.com/scriby/moodle/commits/MDL-31657

          I removed a couple other strings nearby that also weren't in use. I suspect there are a number more in that file that aren't used.

          Testing instructions:

          I did a code grep to make sure the localization strings weren't in use. Just running through the LTI screens and confirming it's not trying to use one of the localization strings is all I can think of on testing.

          Show
          Chris Scribner added a comment - Fix is here: https://github.com/scriby/moodle/commits/MDL-31657 I removed a couple other strings nearby that also weren't in use. I suspect there are a number more in that file that aren't used. Testing instructions: I did a code grep to make sure the localization strings weren't in use. Just running through the LTI screens and confirming it's not trying to use one of the localization strings is all I can think of on testing.
          Hide
          Dan Poltawski added a comment -

          Looks good:

          (M=cf487 ?3) danp@marge ~/git/integration> git grep imsroleadmin
          mod/lti/lang/en/lti.php:$string['imsroleadmin'] = 'Instructor,Administrator';
          (M=cf487 ?3) danp@marge ~/git/integration> git grep imsroleinstructor
          mod/lti/lang/en/lti.php:$string['imsroleinstructor'] = 'Instructor';
          (M=cf487 ?3) danp@marge ~/git/integration> git grep imsrolelearner
          mod/lti/lang/en/lti.php:$string['imsrolelearner'] = 'Learner';
          (M=cf487 ?3) danp@marge ~/git/integration> git grep setupbox | grep lti
          mod/lti/lang/en/lti.php:$string['setupbox'] = 'LTI Tool Setup Box';
          (M=cf487 ?3) danp@marge ~/git/integration> git grep setdefault | grep lti
          mod/lti/lang/en/lti.php:$string['setdefault'] = 'Set a default value for the professor if delegating';
          
          Show
          Dan Poltawski added a comment - Looks good: (M=cf487 ?3) danp@marge ~/git/integration> git grep imsroleadmin mod/lti/lang/en/lti.php:$string['imsroleadmin'] = 'Instructor,Administrator'; (M=cf487 ?3) danp@marge ~/git/integration> git grep imsroleinstructor mod/lti/lang/en/lti.php:$string['imsroleinstructor'] = 'Instructor'; (M=cf487 ?3) danp@marge ~/git/integration> git grep imsrolelearner mod/lti/lang/en/lti.php:$string['imsrolelearner'] = 'Learner'; (M=cf487 ?3) danp@marge ~/git/integration> git grep setupbox | grep lti mod/lti/lang/en/lti.php:$string['setupbox'] = 'LTI Tool Setup Box'; (M=cf487 ?3) danp@marge ~/git/integration> git grep setdefault | grep lti mod/lti/lang/en/lti.php:$string['setdefault'] = 'Set a default value for the professor if delegating';
          Hide
          Dan Poltawski added a comment -

          I'm submitting thsi for integration now, thanks!

          Chris: it'd be great if you could fill out the git repository/branch/diff fields when submititng these as its necessary for submitting to integration.

          Thanks!

          Show
          Dan Poltawski added a comment - I'm submitting thsi for integration now, thanks! Chris: it'd be great if you could fill out the git repository/branch/diff fields when submititng these as its necessary for submitting to integration. Thanks!
          Hide
          Eloy Lafuente (stronk7) 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
          Eloy Lafuente (stronk7) 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
          Aparup Banerjee added a comment -

          cool, integrated into master.

          Show
          Aparup Banerjee added a comment - cool, integrated into master.
          Hide
          Adrian Greeve added a comment -

          No testing instructions present. I did a grep for the different strings in the code to see if it was being called from anywhere. The result turned up negative.
          Passing.
          Thanks.

          Show
          Adrian Greeve added a comment - No testing instructions present. I did a grep for the different strings in the code to see if it was being called from anywhere. The result turned up negative. Passing. Thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          U P S T R E A M I Z E D !

          Many thanks for the hard work, closing this as fixed.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: