Moodle
  1. Moodle
  2. MDL-16796

Assignment plugins is not completely plugable

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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
    • Rank:
      30352

      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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Miroslav Fikar added a comment -

          my zip file

          Show
          Miroslav Fikar added a comment - my zip file
          Hide
          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
          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
          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
          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
          Anthony Borrow added a comment -

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

          Show
          Anthony Borrow added a comment - Good catch Miroslav. I am attaching an updated patch which addresses the issue you raised. Peace - Anthony
          Hide
          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
          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
          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
          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
          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
          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
          Anthony Borrow added a comment -

          Thanks Dan for the more complete patch. Peace - Anthony

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

          Thanks Dan, you are correct. Best, Miroslav

          Show
          Miroslav Fikar added a comment - Thanks Dan, you are correct. Best, Miroslav
          Hide
          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
          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
          Dan Poltawski added a comment -

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

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

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

          Show
          Petr Škoda added a comment - is your fix going to be compatible with current subplugin design in 2.0?
          Hide
          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
          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
          Petr Škoda added a comment -

          looks like it should be compatible

          Show
          Petr Škoda added a comment - looks like it should be compatible
          Hide
          Dan Poltawski added a comment -

          I have applied this fix in CVS

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

          Thanks Dan!

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

          Finally!

          Show
          Miroslav Fikar added a comment - Finally!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: