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

Strings imsroleadmin and imsroleinstructor are unused

    Details

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

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

            Show
            salvetore Michael de Raadt added a comment - Up to you if you want to do this or close this issue, Chris.
            Hide
            gerry 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
            gerry 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
            ralfh 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
            ralfh 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
            scriby 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
            scriby 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
            poltawski 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
            poltawski 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
            poltawski 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
            poltawski 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
            stronk7 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
            stronk7 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
            nebgor Aparup Banerjee added a comment -

            cool, integrated into master.

            Show
            nebgor Aparup Banerjee added a comment - cool, integrated into master.
            Hide
            abgreeve 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
            abgreeve 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
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  25/Jun/12