Moodle
  1. Moodle
  2. MDL-29007

YUI: moodle-enrol-notification should be generalised as its already being used widely outside of enrol

    Details

    • Testing Instructions:
      Hide
      • Open a course page
      • Turn Editing on
      • Ensure that the dialogues work (e.g. modchooser)
      • Open the filepicker from somewhere
      • Ensure that it opens

      For brownie points:

      • open lib/yui/chooserdialogue/chooserdialogue.js in your editor
      • change the line 'moodle-core-notification' to 'moodle-enrol-notification'
      • open the course page
      • open your js console
      • turn editing on
      • confirm that a warning message is shown suggesting that the enrol module has been deprecated
      Show
      Open a course page Turn Editing on Ensure that the dialogues work (e.g. modchooser) Open the filepicker from somewhere Ensure that it opens For brownie points: open lib/yui/chooserdialogue/chooserdialogue.js in your editor change the line 'moodle-core-notification' to 'moodle-enrol-notification' open the course page open your js console turn editing on confirm that a warning message is shown suggesting that the enrol module has been deprecated
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-29007-master-3
    • Rank:
      18550

      Description

      In fact I create this bug because we are using it in our private plugins as its so useful

      $ git grep moodle-enrol-notification
      backup/util/ui/yui/confirmcancel/confirmcancel.js:}, '@VERSION@', {'requires':['base','node','node-event-simulate','moodle-enrol-notification']});
      blocks/community/yui/comments/comments.js:    requires:['base','overlay', 'moodle-enrol-notification']
      blocks/community/yui/imagegallery/imagegallery.js:    requires:['base','node','overlay', 'moodle-enrol-notification']
      enrol/cohort/yui/quickenrolment/quickenrolment.js:}, '@VERSION@', {requires:['base','node', 'overlay', 'io', 'test', 'json-parse', 'event-delegate', 'dd-plugin', 'event-key', 'moodle-enrol-notification']});
      enrol/manual/yui/quickenrolment/quickenrolment.js:}, '@VERSION@', {requires:['base','node', 'overlay', 'io', 'test', 'json-parse', 'event-delegate', 'dd-plugin', 'event-key', 'moodle-enrol-notification']});
      enrol/yui/notification/notification.js:YUI.add('moodle-enrol-notification', function(Y) {
      enrol/yui/notification/notification.js:}, '@VERSION@', {requires:['base','node','overlay','event-key', 'moodle-enrol-notification-skin']});
      enrol/yui/otherusersmanager/otherusersmanager.js:}, '@VERSION@', {requires:['base','node', 'overlay', 'io', 'test', 'json-parse', 'event-delegate', 'dd-plugin', 'event-key', 'moodle-enrol-notification']});
      enrol/yui/rolemanager/rolemanager.js:}, '@VERSION@', {requires:['base','node','io','json-parse','test','moodle-enrol-notification']});
      mod/glossary/yui/autolinker/autolinker.js:}, '@VERSION@', {requires:['base','node','event-delegate','overlay','moodle-enrol-notification']});
      

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for suggesting this.

          Feel free to help us work on this issue.

          Show
          Michael de Raadt added a comment - Thanks for suggesting this. Feel free to help us work on this issue.
          Hide
          Dan Poltawski added a comment -

          No code to review there govna.

          Show
          Dan Poltawski added a comment - No code to review there govna.
          Hide
          Andrew Nicols added a comment -

          Apologies for that - pushed to our private repo by mistake.

          Show
          Andrew Nicols added a comment - Apologies for that - pushed to our private repo by mistake.
          Hide
          Dan Poltawski added a comment -

          Hi Andrew,

          This looks good, thanks for working on it. Only comment is that we'd want to document this change for third party developers which were also relying on it (as mentioned in the issue we used it ourself).

          The jury is out as to where we could put this notice, maybe in enrol/upgrade.txt and lib/upgrade.txt to be absolutely clear.

          Show
          Dan Poltawski added a comment - Hi Andrew, This looks good, thanks for working on it. Only comment is that we'd want to document this change for third party developers which were also relying on it (as mentioned in the issue we used it ourself). The jury is out as to where we could put this notice, maybe in enrol/upgrade.txt and lib/upgrade.txt to be absolutely clear.
          Hide
          Andrew Nicols added a comment -

          Hi Dan,

          Thanks for the review. Agreed this should be noted somewhere in addition to the console.log output.
          Personally, I think that the best place is in lib/upgrade.txt as a majority of developers won't think to look in enrol/upgrade.txt if their plugin has nothing (else) to do with notifications, but they may well look in lib/upgrade.txt.

          I've updated lib/upgrade.txt and pushed a new branch. If you're happy and agree, I'll put it up for IR.

          Cheers,

          Andrew

          Show
          Andrew Nicols added a comment - Hi Dan, Thanks for the review. Agreed this should be noted somewhere in addition to the console.log output. Personally, I think that the best place is in lib/upgrade.txt as a majority of developers won't think to look in enrol/upgrade.txt if their plugin has nothing (else) to do with notifications, but they may well look in lib/upgrade.txt. I've updated lib/upgrade.txt and pushed a new branch. If you're happy and agree, I'll put it up for IR. Cheers, Andrew
          Hide
          Dan Poltawski added a comment - - edited

          Seems OK, but I know for a fact this is going to conflict (http://git.moodle.org/gw?p=integration.git;a=blob;f=lib/upgrade.txt;h=58682c5673fd07550fddb5dcd520c2ce2f74c51b;hb=master ) so please rebase when the weekly build becomes available

          Show
          Dan Poltawski added a comment - - edited Seems OK, but I know for a fact this is going to conflict ( http://git.moodle.org/gw?p=integration.git;a=blob;f=lib/upgrade.txt;h=58682c5673fd07550fddb5dcd520c2ce2f74c51b;hb=master ) so please rebase when the weekly build becomes available
          Hide
          Andrew Nicols added a comment -

          Rebased against integration/master for your convenience

          Show
          Andrew Nicols added a comment - Rebased against integration/master for your convenience
          Hide
          Aparup Banerjee 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
          Aparup Banerjee 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 Hemelryk added a comment -

          Absolute legend!

          a. Because you took the time to move this to a much better location.
          b. Because you use it!

          (this has been integrated now btw )

          Show
          Sam Hemelryk added a comment - Absolute legend! a. Because you took the time to move this to a much better location. b. Because you use it! (this has been integrated now btw )
          Hide
          Adrian Greeve added a comment -

          Tested in master. dialogues work as does the file picker.
          No problems encountered.
          Test passed

          Show
          Adrian Greeve added a comment - Tested in master. dialogues work as does the file picker. No problems encountered. Test passed
          Hide
          Dan Poltawski added a comment -

          asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ!

          Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!

          Show
          Dan Poltawski added a comment - asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ! Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: