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

Assignment plugins is not completely plugable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.2
    • Fix Version/s: 1.9.7
    • Component/s: Assignment (2.2)
    • Labels:
      None
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE

      Description

      When developing a custom assignment plugin (random), its name cannot be found in directories searched. More explicitly, function assignment_types in assignment/lib.php calls function get_string('type'.$name, 'assignment'). All official plugins get string name typeonline, typeoffline, etc from official language file asignment.php in lang/en_utf8 (or other appropriate language). However, custom plugin should get its strings from mod/assignment/type/random/lang/en_utf8. Although language file assignment_random.php in this directory is browsed for other strings, string typerandom is not read from it. This results in [[typerandom]] messages in list of assignments and at the assignment update pages.

        Gliffy Diagrams

        1. 0001-mod-assignment-MDL-16796-Support-non-core-assignmen.patch
          3 kB
          Dan Poltawski
        2. assignment_type.patch
          2 kB
          Anthony Borrow
        3. assignment_type.patch
          1 kB
          Anthony Borrow

          Issue Links

            Activity

            Hide
            aborrow Anthony Borrow added a comment -

            Petr - It looks like this was intended; however, the code in assignment lib.php does not follow what get_string expects. The following patch should resolve this. I have also added a todo to look at another area where we attempt to get assignment type name using get_string. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - Petr - It looks like this was intended; however, the code in assignment lib.php does not follow what get_string expects. The following patch should resolve this. I have also added a todo to look at another area where we attempt to get assignment type name using get_string. Peace - Anthony
            Hide
            aborrow Anthony Borrow added a comment -

            adding fix version 1.9.5 as I think we should get this in there for folks who are using custom assignment types. I came across this when I was working on CONTRIB-1175. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - adding fix version 1.9.5 as I think we should get this in there for folks who are using custom assignment types. I came across this when I was working on CONTRIB-1175 . Peace - Anthony
            Hide
            fikar Miroslav Fikar added a comment -

            I have tried the patch and changed the line in lib.php (line 3010) corespondingly. However, I still get [[typerandom]] as before. Maybe I do something wrong. Here are the location of strings 'typerandom':

            grep -ir typerandom moodle/*
            moodle/mod/assignment/type/random/lang/sk_utf8/assignment_random.php:$string['typerandom'] = 'Náhodné zadanie';
            moodle/mod/assignment/type/random/lang/en_utf8/assignment_random.php:$string['typerandom'] = 'Random assignment';

            and this is now around line 3010 of my lib.php
            grep -n -C 2 assignment_ moodle/mod/assignment/lib.php
            ...
            3008- $type->type = "assignment&type=$assignmenttype";
            3009- //$type->typestr = get_string("type$assignmenttype", 'assignment');
            3010: $type->typestr = get_string("type$assignmenttype", 'assignment_'.$assignmenttype);
            3011- $types[] = $type;
            3012- }
            ...

            Show
            fikar Miroslav Fikar added a comment - I have tried the patch and changed the line in lib.php (line 3010) corespondingly. However, I still get [ [typerandom] ] as before. Maybe I do something wrong. Here are the location of strings 'typerandom': grep -ir typerandom moodle/* moodle/mod/assignment/type/random/lang/sk_utf8/assignment_random.php:$string ['typerandom'] = 'Náhodné zadanie'; moodle/mod/assignment/type/random/lang/en_utf8/assignment_random.php:$string ['typerandom'] = 'Random assignment'; and this is now around line 3010 of my lib.php grep -n -C 2 assignment_ moodle/mod/assignment/lib.php ... 3008- $type->type = "assignment&type=$assignmenttype"; 3009- //$type->typestr = get_string("type$assignmenttype", 'assignment'); 3010: $type->typestr = get_string("type$assignmenttype", 'assignment_'.$assignmenttype); 3011- $types[] = $type; 3012- } ...
            Hide
            aborrow Anthony Borrow added a comment -

            Miroslav - Looking at your README file, I think you made some old assumptions about get_string that has been fixed up quite a bit. I downloaded the latest version of the random assignment type. I put lang into /mod/assignment/type/random and renamed the file from assignment.php to assignment_random.php and it worked as expected. I'll attach a zip file so that you can see what I did. Also, if you are interested in adding your code to Moodle's CVS server have a look at http://docs.moodle.org/en/Development:Guidelines_for_contributed_code. I would be happy to give your code a look and make some suggestions but would like to do that under a separate tracker issue. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - Miroslav - Looking at your README file, I think you made some old assumptions about get_string that has been fixed up quite a bit. I downloaded the latest version of the random assignment type. I put lang into /mod/assignment/type/random and renamed the file from assignment.php to assignment_random.php and it worked as expected. I'll attach a zip file so that you can see what I did. Also, if you are interested in adding your code to Moodle's CVS server have a look at http://docs.moodle.org/en/Development:Guidelines_for_contributed_code . I would be happy to give your code a look and make some suggestions but would like to do that under a separate tracker issue. Peace - Anthony
            Hide
            aborrow Anthony Borrow added a comment -

            Miroslav - Here is the zip file I mentioned earlier. Let me know if you have any questions. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - Miroslav - Here is the zip file I mentioned earlier. Let me know if you have any questions. Peace - Anthony
            Hide
            fikar Miroslav Fikar added a comment -

            Antony, thank you very much for your help. I have been aware of some of the changes you propose in the zip file, but they have not been in the package yet - due to this bug.

            I have tested your suggestions over the weekend. It does not work at my development machine, but works on testing server. But it is possible that the development machine has some problems. I still have a problem that only english strings are shown, but I will work on it..

            Show
            fikar Miroslav Fikar added a comment - Antony, thank you very much for your help. I have been aware of some of the changes you propose in the zip file, but they have not been in the package yet - due to this bug. I have tested your suggestions over the weekend. It does not work at my development machine, but works on testing server. But it is possible that the development machine has some problems. I still have a problem that only english strings are shown, but I will work on it..
            Hide
            fikar Miroslav Fikar added a comment -

            my zip file

            Show
            fikar Miroslav Fikar added a comment - my zip file
            Hide
            aborrow Anthony Borrow added a comment -

            Miroslav - If I had to guess why it is not working on the production site I would say that get_string is finding the local file and trying to use that. I would move/rename that folder/file so that it uses what is in /mod/assignment/type/random/lang/en_utf8. get_string can be a little picky at times. Once you verify it is working you can delete the local lang directory/file. If I recall the code it looks for the existence of the directory and if it is there that is what it uses; however, it is not finding the correctly named file (assignment_random.php). Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - Miroslav - If I had to guess why it is not working on the production site I would say that get_string is finding the local file and trying to use that. I would move/rename that folder/file so that it uses what is in /mod/assignment/type/random/lang/en_utf8. get_string can be a little picky at times. Once you verify it is working you can delete the local lang directory/file. If I recall the code it looks for the existence of the directory and if it is there that is what it uses; however, it is not finding the correctly named file (assignment_random.php). Peace - Anthony
            Hide
            fikar Miroslav Fikar added a comment -

            Antony,

            with your patch, I got it almost working after I have replaced all my get_string calls
            get_string('...', 'assignment')
            by:
            get_string('...', 'assignment_random')

            There is one issue remaining. It can be seen in the list of all assignments in a course (moodle/mod/assignment/index.php?id=...). The assignment name is still in double brackets as [[typerandom]]. It is due to incorrect function assignment_types in assignment/lib.php. It only checks string typerandom in assignment.php and not in assignment_random.php. See here:
            2816 function assignment_types() {
            2817 $types = array();
            2818 $names = get_list_of_plugins('mod/assignment/type');
            2819 foreach ($names as $name) {
            2820 $types[$name] = get_string('type'.$name, 'assignment');

            Best,
            Miroslav

            Show
            fikar Miroslav Fikar added a comment - Antony, with your patch, I got it almost working after I have replaced all my get_string calls get_string('...', 'assignment') by: get_string('...', 'assignment_random') There is one issue remaining. It can be seen in the list of all assignments in a course (moodle/mod/assignment/index.php?id=...). The assignment name is still in double brackets as [ [typerandom] ]. It is due to incorrect function assignment_types in assignment/lib.php. It only checks string typerandom in assignment.php and not in assignment_random.php. See here: 2816 function assignment_types() { 2817 $types = array(); 2818 $names = get_list_of_plugins('mod/assignment/type'); 2819 foreach ($names as $name) { 2820 $types [$name] = get_string('type'.$name, 'assignment'); Best, Miroslav
            Hide
            aborrow Anthony Borrow added a comment -

            Good catch Miroslav. I am attaching an updated patch which addresses the issue you raised. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - Good catch Miroslav. I am attaching an updated patch which addresses the issue you raised. Peace - Anthony
            Hide
            fikar Miroslav Fikar added a comment -

            Thanks Antony for your time and help. It works now. We can close this issue if your last patch is applied.. Best, Miroslav

            Show
            fikar Miroslav Fikar added a comment - Thanks Antony for your time and help. It works now. We can close this issue if your last patch is applied.. Best, Miroslav
            Hide
            aborrow Anthony Borrow added a comment -

            Thanks Miroslav for testing. Since the issue is assigned to Petr, I will leave it to him to evaluate the patch and commit. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - Thanks Miroslav for testing. Since the issue is assigned to Petr, I will leave it to him to evaluate the patch and commit. Peace - Anthony
            Hide
            poltawski Dan Poltawski added a comment -

            The latest patch from anthony misses the assignment actvitiy form when adding a non-standard assignment type, i've addressed this in this patch.

            Show
            poltawski Dan Poltawski added a comment - The latest patch from anthony misses the assignment actvitiy form when adding a non-standard assignment type, i've addressed this in this patch.
            Hide
            aborrow Anthony Borrow added a comment -

            Thanks Dan for the more complete patch. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - Thanks Dan for the more complete patch. Peace - Anthony
            Hide
            fikar Miroslav Fikar added a comment -

            Thanks Dan, you are correct. Best, Miroslav

            Show
            fikar Miroslav Fikar added a comment - Thanks Dan, you are correct. Best, Miroslav
            Hide
            aborrow Anthony Borrow added a comment -

            Petr - I am reviewing an upcoming book about Moodle development and this issue is mentioned. It would be good if we could get this patch applied. Since I am not doing full time coding, I always like to have such things reviewed first but feel free to assign to me if it looks good and I will apply it. Peace - Anthony

            Show
            aborrow Anthony Borrow added a comment - Petr - I am reviewing an upcoming book about Moodle development and this issue is mentioned. It would be good if we could get this patch applied. Since I am not doing full time coding, I always like to have such things reviewed first but feel free to assign to me if it looks good and I will apply it. Peace - Anthony
            Hide
            poltawski Dan Poltawski added a comment -

            I am going to commit this fix unless anyone can come up with a compelling reason not to.

            Show
            poltawski Dan Poltawski added a comment - I am going to commit this fix unless anyone can come up with a compelling reason not to.
            Hide
            skodak Petr Skoda added a comment -

            is your fix going to be compatible with current subplugin design in 2.0?

            Show
            skodak Petr Skoda added a comment - is your fix going to be compatible with current subplugin design in 2.0?
            Hide
            poltawski Dan Poltawski added a comment -

            Where is this design?

            The fix hopefully wont be (as its a bodge), but hopefully the ability for assignment types to have language strings will be comptabible since that is a fundamental of all our plugins.

            Show
            poltawski Dan Poltawski added a comment - Where is this design? The fix hopefully wont be (as its a bodge), but hopefully the ability for assignment types to have language strings will be comptabible since that is a fundamental of all our plugins.
            Hide
            skodak Petr Skoda added a comment -

            looks like it should be compatible

            Show
            skodak Petr Skoda added a comment - looks like it should be compatible
            Hide
            poltawski Dan Poltawski added a comment -

            I have applied this fix in CVS

            Show
            poltawski Dan Poltawski added a comment - I have applied this fix in CVS
            Hide
            aborrow Anthony Borrow added a comment -

            Thanks Dan!

            Show
            aborrow Anthony Borrow added a comment - Thanks Dan!
            Hide
            fikar Miroslav Fikar added a comment -

            Finally!

            Show
            fikar Miroslav Fikar added a comment - Finally!

              People

              • Votes:
                6 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Nov/09