Moodle
  1. Moodle
  2. MDL-13000

moodle should support local/ to define its own capabilities.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9, 2.0
    • Fix Version/s: 1.9, 2.0
    • Component/s: Roles / Access
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Rank:
      30722

      Description

      by providing local/db/access.php, devs should be able to create custom capabilities that moodle picks up and installs.

      1. localcapabilitesOU.patch.txt
        2 kB
        Tim Hunt
      2. localcaps.diff
        3 kB
        Penny Leach
      3. MDL-13000.diff
        4 kB
        Penny Leach

        Activity

        Hide
        Penny Leach added a comment -

        patch against 1.9 (very similar to patch against head)

        • diff in lib/locallib.php which contains instructions also has some notes on other local/ stuff I'm working on (lang (almost ready to commit) and admin tree <-- already committed)
        Show
        Penny Leach added a comment - patch against 1.9 (very similar to patch against head) diff in lib/locallib.php which contains instructions also has some notes on other local/ stuff I'm working on (lang (almost ready to commit) and admin tree <-- already committed)
        Hide
        Penny Leach added a comment -

        new version of patch that namespaces by moodle/local:

        Show
        Penny Leach added a comment - new version of patch that namespaces by moodle/local:
        Hide
        Penny Leach added a comment -

        Fixed in HEAD & 19_STABLE

        (using the second patch, with a couple extra notes)

        Show
        Penny Leach added a comment - Fixed in HEAD & 19_STABLE (using the second patch, with a couple extra notes)
        Hide
        Tim Hunt added a comment -

        Was reading back through the chat about this on Moodle HQ.

        We implemented this at the OU, but assumed that no-one else would want it in core. Anyway, if this is going in to core, it would be really good if it could be done in a way that is compatible with what we are already using.

        We went for capabilites with names like local/something:somethingmorespecific.

        I guess I had better make a patch.

        Show
        Tim Hunt added a comment - Was reading back through the chat about this on Moodle HQ. We implemented this at the OU, but assumed that no-one else would want it in core. Anyway, if this is going in to core, it would be really good if it could be done in a way that is compatible with what we are already using. We went for capabilites with names like local/something:somethingmorespecific. I guess I had better make a patch.
        Hide
        Tim Hunt added a comment -

        The language string for capability local/something:somethingmorespecific would be get_string('somethingmorespecific', 'something');, which we would put in lang/en_utf8_local

        I think the attached patch is all the code we added to make this work. It's suprisingly small.

        Show
        Tim Hunt added a comment - The language string for capability local/something:somethingmorespecific would be get_string('somethingmorespecific', 'something');, which we would put in lang/en_utf8_local I think the attached patch is all the code we added to make this work. It's suprisingly small.
        Hide
        Tim Hunt added a comment -

        Drat! I am being really unobservant I now notice you have already committed your changes, which are incompatible with what we have been doing for ages. RANT! Shouldn't you have posted about this in the General Developer Forum and given people a few days to comment? As Martin Langhoff would say, we need to hold these discussions in public, not just in Moodle HQ chat.

        Show
        Tim Hunt added a comment - Drat! I am being really unobservant I now notice you have already committed your changes, which are incompatible with what we have been doing for ages. RANT! Shouldn't you have posted about this in the General Developer Forum and given people a few days to comment? As Martin Langhoff would say, we need to hold these discussions in public, not just in Moodle HQ chat.
        Hide
        Penny Leach added a comment -

        Tim -

        About the 'more specific' stuff, did you read the long conversation about whether local/ should be a plugin itself or a plugin system? I actually started off with a patch very similar to yours, but changed it so that local/ ended up not having 'sub' components.

        Regarding the "shouldn't I have posted about this in GDF" - tbh, I really didn't think it was necessary given that it was such a tiny patch and only affected local/ not core.

        I can't really keep up on whether we should discuss these things in bugs, forums or skype chat either. All three seems a bit excessive. Admittedly I created the bug but then MD and I discussed it in skype rather than here, which was probably incorrect. I'm not convinced GDF is the best place though - surely this is what the tracker is for ?

        Show
        Penny Leach added a comment - Tim - About the 'more specific' stuff, did you read the long conversation about whether local/ should be a plugin itself or a plugin system? I actually started off with a patch very similar to yours, but changed it so that local/ ended up not having 'sub' components. Regarding the "shouldn't I have posted about this in GDF" - tbh, I really didn't think it was necessary given that it was such a tiny patch and only affected local/ not core. I can't really keep up on whether we should discuss these things in bugs, forums or skype chat either. All three seems a bit excessive. Admittedly I created the bug but then MD and I discussed it in skype rather than here, which was probably incorrect. I'm not convinced GDF is the best place though - surely this is what the tracker is for ?
        Hide
        Penny Leach added a comment -

        Also, just to piss you off even more You should probably take a look at MDL-13001 and MDL-11561 which add support for local/lang and local/settings.php respectively.

        Show
        Penny Leach added a comment - Also, just to piss you off even more You should probably take a look at MDL-13001 and MDL-11561 which add support for local/lang and local/settings.php respectively.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Apart from all this... finally, the "local" stuff is going to be only one stuff, correct? no "submodules" under "local" (i.e. ONE install.xml, upgrade.php, access.php, lang...).

        Yep? (I'd voted +1 for that)

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Apart from all this... finally, the "local" stuff is going to be only one stuff, correct? no "submodules" under "local" (i.e. ONE install.xml, upgrade.php, access.php, lang...). Yep? (I'd voted +1 for that) Ciao
        Hide
        Shane Elliott added a comment -

        Noticed that there was no call to update_capabilities in lib/locallib.php to actually process local/db/access.php - have submitted a fix to CVS for 1.9 and HEAD

        Show
        Shane Elliott added a comment - Noticed that there was no call to update_capabilities in lib/locallib.php to actually process local/db/access.php - have submitted a fix to CVS for 1.9 and HEAD
        Hide
        David Monllaó added a comment -

        Sorry for comment a closed issue, but I've followed the 1.9.3 lib/locallib.php instructions to add new local capabilities; it creates the capabilities, but the installation script enters to update_capabilities function every time upgrade_local_db is called, and it crashes the second time trying to insert the same defined local capabilities to db.

        In your attached patch, update_capabilities is called before local_version config var is set, I think that's the right way, but in 1.10.2.4 lib/locallib.php revision, update_capabilities call is in another place

        Show
        David Monllaó added a comment - Sorry for comment a closed issue, but I've followed the 1.9.3 lib/locallib.php instructions to add new local capabilities; it creates the capabilities, but the installation script enters to update_capabilities function every time upgrade_local_db is called, and it crashes the second time trying to insert the same defined local capabilities to db. In your attached patch, update_capabilities is called before local_version config var is set, I think that's the right way, but in 1.10.2.4 lib/locallib.php revision, update_capabilities call is in another place
        Hide
        Penny Leach added a comment -

        Hi David,

        When you say 'in your attached patch' - which patch ? I see 3 attached to this bug

        Show
        Penny Leach added a comment - Hi David, When you say 'in your attached patch' - which patch ? I see 3 attached to this bug
        Hide
        David Monllaó added a comment -

        Ups, I forget...

        "localcapabilitesOU.patch.txt" or "MDL-13000.diff", after the upgrade of xmldb_local_upgrade

        Show
        David Monllaó added a comment - Ups, I forget... "localcapabilitesOU.patch.txt" or " MDL-13000 .diff", after the upgrade of xmldb_local_upgrade
        Hide
        Penny Leach added a comment -

        > and it crashes the second time trying to insert the same defined local capabilities to db.

        What do you mean by 'crashes'- can you give me exact errors?

        The capabilities should be safe to add multiple times because assign_capability is duplicate-safe. Else you would get the same problem everytime you bumped the version number.

        Show
        Penny Leach added a comment - > and it crashes the second time trying to insert the same defined local capabilities to db. What do you mean by 'crashes'- can you give me exact errors? The capabilities should be safe to add multiple times because assign_capability is duplicate-safe. Else you would get the same problem everytime you bumped the version number.
        Hide
        David Monllaó added a comment -

        error -> "Could not set up the capabilities for local!"

        It shows error trying to insert another capability with the same name

        Show
        David Monllaó added a comment - error -> "Could not set up the capabilities for local!" It shows error trying to insert another capability with the same name
        Hide
        Penny Leach added a comment -

        Can you paste the entire error ?

        Show
        Penny Leach added a comment - Can you paste the entire error ?
        Hide
        David Monllaó added a comment -

        My fault, sorry for make you to lose your time Penny...

        I forget to change the capability name path to moodle/local:

        Show
        David Monllaó added a comment - My fault, sorry for make you to lose your time Penny... I forget to change the capability name path to moodle/local:
        Hide
        Penny Leach added a comment -

        Hehe! No worries, happy you got it working

        Show
        Penny Leach added a comment - Hehe! No worries, happy you got it working

          People

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

            Dates

            • Created:
              Updated:
              Resolved: