Moodle
  1. Moodle
  2. MDL-30408

API enhancement: Way to prevent modules appearing on 'Add...' dropdowns

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide

      Level: DIFFICULT (needs code access).

      0. If necessary, turn on the restrict modules feature for all courses.

      1. Load a course page. Turn editing on. Select an 'Add a resource...' dropdown.
      + Verify that the URL module is included on the list.

      2. Add a URL (any settings).

      3. Edit mod/url/lib.php. Change the line in url_supports, 'return MOD_ARCHETYPE_RESOURCE' to return MOD_ARCHETYPE_SYSTEM.

      4. Reload the course page.
      + Verify that the URL module now does not appear on the list.
      + Verify that the existing URL still appears and can still be edited.

      5. Edit course settings.
      + Verify that the URL module now is not listed on the 'restrict modules' list (there's no point as you cannot add it there anyhow)
      + Verify that the 'restrict modules' list shows the human-readable names of each module not the lower-case codes (Forum vs. forum)

      Show
      Level: DIFFICULT (needs code access). 0. If necessary, turn on the restrict modules feature for all courses. 1. Load a course page. Turn editing on. Select an 'Add a resource...' dropdown. + Verify that the URL module is included on the list. 2. Add a URL (any settings). 3. Edit mod/url/lib.php. Change the line in url_supports, 'return MOD_ARCHETYPE_RESOURCE' to return MOD_ARCHETYPE_SYSTEM. 4. Reload the course page. + Verify that the URL module now does not appear on the list. + Verify that the existing URL still appears and can still be edited. 5. Edit course settings. + Verify that the URL module now is not listed on the 'restrict modules' list (there's no point as you cannot add it there anyhow) + Verify that the 'restrict modules' list shows the human-readable names of each module not the lower-case codes (Forum vs. forum)
    • Workaround:
      Hide
      function oucontent_get_types() {
          // This prevents it being on the Add Activity menu
          return array();
      }
      
      Show
      function oucontent_get_types() { // This prevents it being on the Add Activity menu return array(); }
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-30408-master
    • Rank:
      33035

      Description

      (Change proposed for 2.3, please ignore until that starts.)

      In several cases at the OU we have modules which you cannot create using the normal 'Add a resource/activity' dropdowns. This is because either:

      a) the creation process is complicated and must be initiated within another system
      or
      b) the module is created by the system automatically independent of user

      To support these I propose adding a fourth ARCHETYPE constant: MOD_ARCHETYPE_SYSTEM for modules which can only be added by the system and do not appear in the list.

      There is a hack to do this in current (including 2.2) system but it is horrible and seems likely to die in future, now that assignment module has done. See workaround below.

      In addition while implementing this feature I observed that the 'restrict modules' list uses the internal names for modules (e.g. 'forum') rather than the translated language strings. This seems bizarre, so I fixed it.

        Issue Links

          Activity

          Hide
          Sam Marshall added a comment -

          This code is ready and is rather simple, but I will submit it for integration after branch for Moodle 2.3 as I am not proposing this for 2.2 and there doesn't seem to be a mechanism for submitting 2.3 stuff yet.

          Show
          Sam Marshall added a comment - This code is ready and is rather simple, but I will submit it for integration after branch for Moodle 2.3 as I am not proposing this for 2.2 and there doesn't seem to be a mechanism for submitting 2.3 stuff yet.
          Hide
          Michael de Raadt added a comment -

          Hi, Sam.

          Thanks for putting this forward.

          Based on your comments above, I assume it is your intention to put this forward for peer review after 2.2 release.

          Please let me know if you'd like us to deal with this as a regular patched issued.

          Show
          Michael de Raadt added a comment - Hi, Sam. Thanks for putting this forward. Based on your comments above, I assume it is your intention to put this forward for peer review after 2.2 release. Please let me know if you'd like us to deal with this as a regular patched issued.
          Hide
          Sam Marshall added a comment -

          Tim, could you review this very minor enhancement please before I submit it for 2.3?

          Show
          Sam Marshall added a comment - Tim, could you review this very minor enhancement please before I submit it for 2.3?
          Hide
          Tim Hunt added a comment -

          Reviewing the patch, the coding style etc. is fine.

          There is, however, the question of whether what sam is trying to do is something we want to allow. Therefore, I would like to see Petr or Eloy's thoughts on this before this is submitted for integration.

          I think it makes sense to allow it, even though the use cases are quite rare, so +1 from me, for what it is worth.

          Show
          Tim Hunt added a comment - Reviewing the patch, the coding style etc. is fine. There is, however, the question of whether what sam is trying to do is something we want to allow. Therefore, I would like to see Petr or Eloy's thoughts on this before this is submitted for integration. I think it makes sense to allow it, even though the use cases are quite rare, so +1 from me, for what it is worth.
          Hide
          Petr Škoda added a comment - - edited

          Hi, I like this proposed change because I agree there are some valid use cases such as your a) and b)

          Do we really have to introduce extra line breaks all over the place? With all the frankenstyle rules and verbose variable names anything below 120 chars makes the code less readable imo.

          +1 to send it to integration, thanks!

          Show
          Petr Škoda added a comment - - edited Hi, I like this proposed change because I agree there are some valid use cases such as your a) and b) Do we really have to introduce extra line breaks all over the place? With all the frankenstyle rules and verbose variable names anything below 120 chars makes the code less readable imo. +1 to send it to integration, thanks!
          Hide
          Sam Marshall added a comment -

          Thanks Petr, submitting.

          Re extra line break - personally I think anything longer than about 100 is hard to follow, and so long as line breaks are not at illogical points I think they don't make it difficult to read (especially thanks to the 8 char indent which makes it pretty clear it's a wrapped line). So I don't find it 'inconvenient' (as per guidelines) to aim for 100 characters This is kind of a borderline case though because I only added like 3 characters to the line so presumably it was like 102 before being wrapped...

          I don't mind if somebody wants to remove that break.

          Show
          Sam Marshall added a comment - Thanks Petr, submitting. Re extra line break - personally I think anything longer than about 100 is hard to follow, and so long as line breaks are not at illogical points I think they don't make it difficult to read (especially thanks to the 8 char indent which makes it pretty clear it's a wrapped line). So I don't find it 'inconvenient' (as per guidelines) to aim for 100 characters This is kind of a borderline case though because I only added like 3 characters to the line so presumably it was like 102 before being wrapped... I don't mind if somebody wants to remove that break.
          Hide
          Petr Škoda added a comment -

          My point is that editors can not "unwrap" the interrupted lines visually, only the opposite. I believe that we should make lines as long as necessary (200 chars max maybe) and instead let people that like them wrapped at 80, 100 configure their editors to just "visually wrap" long lines.

          Visual wrapping:

          In any case this is not related much to this issue, sorry for annoying you with my personal agenda...

          Show
          Petr Škoda added a comment - My point is that editors can not "unwrap" the interrupted lines visually, only the opposite. I believe that we should make lines as long as necessary (200 chars max maybe) and instead let people that like them wrapped at 80, 100 configure their editors to just "visually wrap" long lines. Visual wrapping: vim http://vim.wikia.com/wiki/Word_wrap_without_line_breaks phpStorm - they call it soft wraps Eclipse PDT - I saw only some experimental plugins for this Netbeans - line wrap option In any case this is not related much to this issue, sorry for annoying you with my personal agenda...
          Hide
          Sam Marshall added a comment -

          Good point - I use Eclipse PDT so I don't have this feature... https://bugs.eclipse.org/bugs/show_bug.cgi?id=35779 (doesn't seem to be any near prospect of it, somehow I don't think the argument of the last patch author 'better to have this feature even if it's totally broken, than not have it at all' will go down well with maintainers).

          If Moodle switched to this approach I think there should be no line wrapping at all (i.e. if there is a 200 limit then you must start a completely new statement at that point, not wrap) because hard breaks at 200 will look horrible for somebody soft-wrapping at 120.

          And if we are doing 'let editors handle things' then we should definitely switch to tabs instead of spaces for initial line indent! Finally I can get my 2 char indent back...

          Anyway I understand your point now, sorry for OT.

          Show
          Sam Marshall added a comment - Good point - I use Eclipse PDT so I don't have this feature... https://bugs.eclipse.org/bugs/show_bug.cgi?id=35779 (doesn't seem to be any near prospect of it, somehow I don't think the argument of the last patch author 'better to have this feature even if it's totally broken, than not have it at all' will go down well with maintainers). If Moodle switched to this approach I think there should be no line wrapping at all (i.e. if there is a 200 limit then you must start a completely new statement at that point, not wrap) because hard breaks at 200 will look horrible for somebody soft-wrapping at 120. And if we are doing 'let editors handle things' then we should definitely switch to tabs instead of spaces for initial line indent! Finally I can get my 2 char indent back... Anyway I understand your point now, sorry for OT.
          Hide
          Petr Škoda added a comment -

          hehe, true about the 200 limit, +666 for tabs

          to integrators: ignore whitespace comments

          Show
          Petr Škoda added a comment - hehe, true about the 200 limit, +666 for tabs to integrators: ignore whitespace comments
          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
          Sam Marshall added a comment -

          Rebased

          Show
          Sam Marshall added a comment - Rebased
          Hide
          Aparup Banerjee added a comment -

          updated fix version here to 2.3 only

          Show
          Aparup Banerjee added a comment - updated fix version here to 2.3 only
          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
          Sam Marshall added a comment -

          Rebased against master

          Show
          Sam Marshall added a comment - Rebased against master
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Offtopic comment: plz, don't forget to, also, push the changes to your stable/master branches @ github or else the diff above are 99% useless

          Show
          Eloy Lafuente (stronk7) added a comment - Offtopic comment: plz, don't forget to, also, push the changes to your stable/master branches @ github or else the diff above are 99% useless
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Uhm...

          I like the idea of being able to prevent manually adding modules, so +1 for it. But:

          1) What happens for $CFG->defaultallowedmodules. Should MOD_ARCHETYPE_SYSTEM modules be available there? IMO it should perform 100% the same behavior than the course/edit_form.php one (currently hiding there).

          2) In fact, I'm not sure if that current behavior (currently hiding) is the correct one. Those 'restrictmodules' should be checked always when creating a new activity, no matter if the creation is done from the "add dropdown" or from an external system. So perhaps the MOD_ARCHETYPE_SYSTEM should continue being shown there. I think this needs some clarification to decide if both points (restricted modules and manual adding) are the same thing or no, to know where to apply the MOD_ARCHETYPE_SYSTEM restriction.

          3) There are some other considerations like, what does happen with activity backup & restore, duplication, edition... do we want to prevent any of them just with the MOD_ARCHETYPE_SYSTEM feature or do we have all them under control with other features? Surely backup is under control but... for example... edition?

          4) As new feature this needs documentation and QA tests defined for sure. I think subtasks or new linked issues can be added to cover that. NP.

          So I like it, but I think we must define/separate better when that feature (restriction) is applied and which is its impact in other features/capabilities (backup/edit for example).

          So, I'm going to reopen this for 1 more week of discussion aiming to get it agreed and clarified. Thanks and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Uhm... I like the idea of being able to prevent manually adding modules, so +1 for it. But: 1) What happens for $CFG->defaultallowedmodules. Should MOD_ARCHETYPE_SYSTEM modules be available there? IMO it should perform 100% the same behavior than the course/edit_form.php one (currently hiding there). 2) In fact, I'm not sure if that current behavior (currently hiding) is the correct one. Those 'restrictmodules' should be checked always when creating a new activity, no matter if the creation is done from the "add dropdown" or from an external system. So perhaps the MOD_ARCHETYPE_SYSTEM should continue being shown there. I think this needs some clarification to decide if both points (restricted modules and manual adding) are the same thing or no, to know where to apply the MOD_ARCHETYPE_SYSTEM restriction. 3) There are some other considerations like, what does happen with activity backup & restore, duplication, edition... do we want to prevent any of them just with the MOD_ARCHETYPE_SYSTEM feature or do we have all them under control with other features? Surely backup is under control but... for example... edition? 4) As new feature this needs documentation and QA tests defined for sure. I think subtasks or new linked issues can be added to cover that. NP. So I like it, but I think we must define/separate better when that feature (restriction) is applied and which is its impact in other features/capabilities (backup/edit for example). So, I'm going to reopen this for 1 more week of discussion aiming to get it agreed and clarified. Thanks and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Offtopic: About line lengths...

          I really find 100/140 way to restrictive. And, personally, I find 2 lines of 140 chars far easier to read than 4 lines of 80 chars (my mind is hardly able to wrap, grrr).

          Also, thinking that 100% of editors and screen sizes allow it I'd propose to increment current 100/140 limits to something like 132/180 or similar.

          I just run the complete phpCS here against codebase with 132/180 and it reduced the number of length errors by 2600 (8%) and the number of length warnings by 17956 (80%) !!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Offtopic: About line lengths... I really find 100/140 way to restrictive. And, personally, I find 2 lines of 140 chars far easier to read than 4 lines of 80 chars (my mind is hardly able to wrap, grrr). Also, thinking that 100% of editors and screen sizes allow it I'd propose to increment current 100/140 limits to something like 132/180 or similar. I just run the complete phpCS here against codebase with 132/180 and it reduced the number of length errors by 2600 (8%) and the number of length warnings by 17956 (80%) !! Ciao
          Hide
          Sam Marshall added a comment -

          Thanks for looking into this Eloy, definitely there are things I overlooked here. More complicated than I thought. Not for the first time

          1) Yes you are exactly right, I forgot to update the config setting, can fix this.

          2) I kind of see your point but I don't entirely agree. Restrictmodules only restricts ordinary users not admins (afaik!), and I think system components are pretty much like admin users. So right now, the restrictmodules is sort of for, 'I don't want some idiot to add quiz module to this course because we don't offer support for that module and we only set it up in special cases' - it's strictly about idiot-user-control or enforcing policies. Making it control other system components as well would be sort of a new feature.

          Also system-created modules are likely to be a bit confusing to ordinary managers, in my experience, so it would be nice if we could remove them from that list. That was my real logic.

          Of course I am OK with not hiding these modules from those lists, if you still disagree. It makes the change smaller (and system components are still free to ignore the CFG setting if they like).

          3) You're right there are other issues. Backup and restore should be OK I think (if module wants to not be included in backup, it can already do that, probably just not define backup support feature?).

          About hiding duplicate/edit/delete - I think this would definitely be a good feature but should be controlled by separate feature constants. For example, we have one system-created module which we DO want people to be able to delete, but we don't want people to be able to edit it... (For now I made it print a 'you can't edit this' in its edit form, which is a bit hacky.)

          Maybe I should make this change larger to include adding those extra feature constants as well for a more complete improvement? Something like FEATURE_PREVENT_EDIT, FEATURE_PREVENT_DELETE, FEATURE_PREVENT_DUPLICATE or similar.

          4) I agree, where should I put the documentation (there is no existing page on the dev wiki about current module archetype constants at least that I could find with basic search) - shall I make a new page about the _supports function? Or is there a page about the module lib.php API in general that I could add to, or perhaps make a new one but with only the one section about _supports and others left blank...

          Re QA tests, I couldn't find a page on dev wiki about procedure for creating these, is there one?

          Show
          Sam Marshall added a comment - Thanks for looking into this Eloy, definitely there are things I overlooked here. More complicated than I thought. Not for the first time 1) Yes you are exactly right, I forgot to update the config setting, can fix this. 2) I kind of see your point but I don't entirely agree. Restrictmodules only restricts ordinary users not admins (afaik!), and I think system components are pretty much like admin users. So right now, the restrictmodules is sort of for, 'I don't want some idiot to add quiz module to this course because we don't offer support for that module and we only set it up in special cases' - it's strictly about idiot-user-control or enforcing policies. Making it control other system components as well would be sort of a new feature. Also system-created modules are likely to be a bit confusing to ordinary managers, in my experience, so it would be nice if we could remove them from that list. That was my real logic. Of course I am OK with not hiding these modules from those lists, if you still disagree. It makes the change smaller (and system components are still free to ignore the CFG setting if they like). 3) You're right there are other issues. Backup and restore should be OK I think (if module wants to not be included in backup, it can already do that, probably just not define backup support feature?). About hiding duplicate/edit/delete - I think this would definitely be a good feature but should be controlled by separate feature constants. For example, we have one system-created module which we DO want people to be able to delete, but we don't want people to be able to edit it... (For now I made it print a 'you can't edit this' in its edit form, which is a bit hacky.) Maybe I should make this change larger to include adding those extra feature constants as well for a more complete improvement? Something like FEATURE_PREVENT_EDIT, FEATURE_PREVENT_DELETE, FEATURE_PREVENT_DUPLICATE or similar. 4) I agree, where should I put the documentation (there is no existing page on the dev wiki about current module archetype constants at least that I could find with basic search) - shall I make a new page about the _supports function? Or is there a page about the module lib.php API in general that I could add to, or perhaps make a new one but with only the one section about _supports and others left blank... Re QA tests, I couldn't find a page on dev wiki about procedure for creating these, is there one?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          1) oki
          2) oki, I understand you point, I just had doubts, but seems your way is better.
          3) yup, we have already the backup feature available (and that should control already the dupes apart from backup & restore themselves, but surely will need a bunch more for editing / deleting). My +1 to followup issue to complete this.
          4) no idea, surely when talking about modules (or within NEWMODULE). I've already added the docs_required and qa_test_required labels, so perhaps that's enough for now and can be retaken once documentors arrive here. Perhaps it's not a bad idea to add them to NEWMODULE already.

          Thanks! Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - 1) oki 2) oki, I understand you point, I just had doubts, but seems your way is better. 3) yup, we have already the backup feature available (and that should control already the dupes apart from backup & restore themselves, but surely will need a bunch more for editing / deleting). My +1 to followup issue to complete this. 4) no idea, surely when talking about modules (or within NEWMODULE). I've already added the docs_required and qa_test_required labels, so perhaps that's enough for now and can be retaken once documentors arrive here. Perhaps it's not a bad idea to add them to NEWMODULE already. Thanks! Ciao
          Hide
          Sam Marshall added a comment -

          Thanks for previous review. Resubmitting for integration as I have fixed the issue Eloy identified (it didn't exclude the modules from the config defaultallowedmodules list).

          Note about change: The admin list-of-modules selector used for that list is not used anywhere else on code, but just in case, I put a parameter so you can choose whether it includes the system modules or not.

          I have made a new documentation page (there was an existing link to it but the page didn't exist) and documented this there: http://docs.moodle.org/dev/lib.php

          Show
          Sam Marshall added a comment - Thanks for previous review. Resubmitting for integration as I have fixed the issue Eloy identified (it didn't exclude the modules from the config defaultallowedmodules list). Note about change: The admin list-of-modules selector used for that list is not used anywhere else on code, but just in case, I put a parameter so you can choose whether it includes the system modules or not. I have made a new documentation page (there was an existing link to it but the page didn't exist) and documented this there: http://docs.moodle.org/dev/lib.php
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The 2 calls to plugin_supports() are slightly different (really the OTHER default is not needed), but not reason to reject this. So integrated (master only), thanks!

          Ciao

          PS: I've deleted the docs_required page as far http://docs.moodle.org/dev/lib.php contains the new MOD_ARCHETYPE_SYSTEM thing.

          Show
          Eloy Lafuente (stronk7) added a comment - The 2 calls to plugin_supports() are slightly different (really the OTHER default is not needed), but not reason to reject this. So integrated (master only), thanks! Ciao PS: I've deleted the docs_required page as far http://docs.moodle.org/dev/lib.php contains the new MOD_ARCHETYPE_SYSTEM thing.
          Hide
          Aparup Banerjee added a comment -

          thanks, this test and patch went through without a flaw.

          Show
          Aparup Banerjee added a comment - thanks, this test and patch went through without a flaw.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This is now available in the git and cvs repositories.

          Consider the responsibility of your fingerprints engraved there for future generations!

          Thanks for the work, closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - This is now available in the git and cvs repositories. Consider the responsibility of your fingerprints engraved there for future generations! Thanks for the work, closing, ciao
          Hide
          Dan Poltawski added a comment -

          Hi,

          Now that restricted modules has gone I don't think there is a place for a QA test for this.

          Thoughts?

          Show
          Dan Poltawski added a comment - Hi, Now that restricted modules has gone I don't think there is a place for a QA test for this. Thoughts?
          Hide
          Frédéric Massart added a comment -

          I wrote a QA test for this issue, and actually it fails. The 'module chooser dialogue' does not prevent the URL module from being used, but it appears under 'Activity' instead of 'Resources'. See MDLQA-1760.

          Show
          Frédéric Massart added a comment - I wrote a QA test for this issue, and actually it fails. The 'module chooser dialogue' does not prevent the URL module from being used, but it appears under 'Activity' instead of 'Resources'. See MDLQA-1760 .
          Hide
          Dan Poltawski added a comment -

          Thank you Fred! I've reported this on issue MDL-30617

          Show
          Dan Poltawski added a comment - Thank you Fred! I've reported this on issue MDL-30617
          Hide
          Dan Poltawski added a comment -

          Come across this old issue.. and i'm wondering if this 'API' is now handled by addinstance thing which tim added using capabilities?

          If not, I seem to have missed the use case for this which can't be handled by capabilities. So it'd be good if you could just outline it here for the records?

          Show
          Dan Poltawski added a comment - Come across this old issue.. and i'm wondering if this 'API' is now handled by addinstance thing which tim added using capabilities? If not, I seem to have missed the use case for this which can't be handled by capabilities. So it'd be good if you could just outline it here for the records?
          Hide
          Tim Hunt added a comment -

          This is handled by

          {plugin}

          _supports(FEATURE_MOD_ARCHETYPE) returning MOD_ARCHETYPE_OTHER / MOD_ARCHETYPE_RESOURCE or MOD_ARCHETYPE_SYSTEM. (The last option there make this activity type hidden from the activity selector.)

          Show
          Tim Hunt added a comment - This is handled by {plugin} _supports(FEATURE_MOD_ARCHETYPE) returning MOD_ARCHETYPE_OTHER / MOD_ARCHETYPE_RESOURCE or MOD_ARCHETYPE_SYSTEM. (The last option there make this activity type hidden from the activity selector.)
          Hide
          Sam Marshall added a comment -

          Just to explain what this is for:

          This archetype option (MOD_ARCHETYPE_SYSTEM) would be used for a module that is automatically created by the system and should never be added manually. For example, instances of the module might be created based on integration with an external university system. Creating the instance might require input data that you cannot enter into a form, but has to come from that system. Or creating one manually might cause system errors because it isn't properly linked with the external system.

          (Note: My examples are about an external system, but this can also be useful entirely within Moodle if you want to create a module that has to be constructed in a specific, strange way rather than by using the add form. E.g. if you have a module that can only be created by Moodle itself during cron, or something like that. We actually use this feature both for 'pure Moodle' activities and for those which are created based on external integration.)

          You could prevent adding a module with the new capabilities but:

          a) This is inelegant because it makes a property of the code (this code does not support being added manually) into user control, as users could configure the capability and allow roles to add the module even if the default is for nobody to do it. There should be no user control; modules using that code cannot be added manually, end of.

          b) System administrators have all capabilities so they would have the potential to add the module (perhaps accidentallty), causing errors and confusion and possible bug reports.

          Show
          Sam Marshall added a comment - Just to explain what this is for: This archetype option (MOD_ARCHETYPE_SYSTEM) would be used for a module that is automatically created by the system and should never be added manually. For example, instances of the module might be created based on integration with an external university system. Creating the instance might require input data that you cannot enter into a form, but has to come from that system. Or creating one manually might cause system errors because it isn't properly linked with the external system. (Note: My examples are about an external system, but this can also be useful entirely within Moodle if you want to create a module that has to be constructed in a specific, strange way rather than by using the add form. E.g. if you have a module that can only be created by Moodle itself during cron, or something like that. We actually use this feature both for 'pure Moodle' activities and for those which are created based on external integration.) You could prevent adding a module with the new capabilities but: a) This is inelegant because it makes a property of the code (this code does not support being added manually) into user control, as users could configure the capability and allow roles to add the module even if the default is for nobody to do it. There should be no user control; modules using that code cannot be added manually, end of. b) System administrators have all capabilities so they would have the potential to add the module (perhaps accidentallty), causing errors and confusion and possible bug reports.
          Hide
          Helen Foster added a comment -

          Thanks Tim and Sam. I was just wondering why there is a QA test for this and not for controlling things using capabilities, but I realise it's because nobody got around to writing one for the capabilities.

          Show
          Helen Foster added a comment - Thanks Tim and Sam. I was just wondering why there is a QA test for this and not for controlling things using capabilities, but I realise it's because nobody got around to writing one for the capabilities.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: