Details

    • Testing Instructions:
      Hide

      The ability to test what this does has not yet been implemented.
      Best test for regressions though, in case it breaks something.

      1. Login as a student and post a blog entry about a course, edit it, then delete it.
      2. Login as a admin and post a site-wide blog entry, edit it, then delete it.

      Make sure there are no errors in doing the above steps.

      Show
      The ability to test what this does has not yet been implemented. Best test for regressions though, in case it breaks something. Login as a student and post a blog entry about a course, edit it, then delete it. Login as a admin and post a site-wide blog entry, edit it, then delete it. Make sure there are no errors in doing the above steps.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-34436-master
    • Rank:
      42838

      Description

      I'm currently working on a submission type for the Moodle 2.3 assignment module where students can make submissions by posting associated blog entries to Moodle's internal blog system.

      It would be very helpful for my project if event triggers could be fired when new blog assignments are added, edited or deleted, so a new submission could be created when a student post an entry to an assignment that is using blog submissions.

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for suggesting that. It seems pretty harmless.

        I'm referring this for review by Martin for his thoughts.

        Show
        Michael de Raadt added a comment - Thanks for suggesting that. It seems pretty harmless. I'm referring this for review by Martin for his thoughts.
        Hide
        Jason Fowler added a comment - - edited

        I'll be working on getting this sorted in the next week. It'll probably only land in the upcoming release of 2.5 though.

        Show
        Jason Fowler added a comment - - edited I'll be working on getting this sorted in the next week. It'll probably only land in the upcoming release of 2.5 though.
        Hide
        Martin Dougiamas added a comment -

        Yeah if it's an API change you've missed the 2.4 deadline for this.

        Also just added Damyon (assignment component lead) so he's aware of this.

        http://docs.moodle.org/dev/Events_API is the page to document this on and look for other examples.

        Show
        Martin Dougiamas added a comment - Yeah if it's an API change you've missed the 2.4 deadline for this. Also just added Damyon (assignment component lead) so he's aware of this. http://docs.moodle.org/dev/Events_API is the page to document this on and look for other examples.
        Hide
        Jason Fowler added a comment -

        Updated with a rebased master branch for master and the upcoming 2.4

        Show
        Jason Fowler added a comment - Updated with a rebased master branch for master and the upcoming 2.4
        Hide
        Jason Fowler added a comment -

        Thanks Martin

        Show
        Jason Fowler added a comment - Thanks Martin
        Hide
        Jason Fowler added a comment -

        Quick change to the code to update it to use context::instance_by_id now that get_context_instance_by_id is in the process of being deprecated

        Show
        Jason Fowler added a comment - Quick change to the code to update it to use context::instance_by_id now that get_context_instance_by_id is in the process of being deprecated
        Hide
        Jason Fowler added a comment -

        rebased

        Show
        Jason Fowler added a comment - rebased
        Hide
        Tim Hunt added a comment -

        That mostly makes sense, you are raising evnets in three new situtations where raising an event seems like a good idea. Just some minor points.

        1. Even thought you do not require code in and events.php file for the events you raise (like you do for event handlers). You are supposed to list the events that you raise in there, and the integrators should reject your patch if you have not done that.

        In this case, I suppose lib/db/events.php is the appropriate file. There are plenty of other examples in the code. E.g. mod/quiz/db/events.php.

        2. The diff would be clearer if you had done the whitespace fixes as a different commit. As it is, https://github.com/phalacee/moodle/compare/master...wip-MDL-34436-master?w=1 makes it easier to review.

        3. It is not clear to me what the extra code in the constructor is for. (This is probably just because I don't know the blog code at all.) You may wish to add a comment explaining.

        Show
        Tim Hunt added a comment - That mostly makes sense, you are raising evnets in three new situtations where raising an event seems like a good idea. Just some minor points. 1. Even thought you do not require code in and events.php file for the events you raise (like you do for event handlers). You are supposed to list the events that you raise in there, and the integrators should reject your patch if you have not done that. In this case, I suppose lib/db/events.php is the appropriate file. There are plenty of other examples in the code. E.g. mod/quiz/db/events.php. 2. The diff would be clearer if you had done the whitespace fixes as a different commit. As it is, https://github.com/phalacee/moodle/compare/master...wip-MDL-34436-master?w=1 makes it easier to review. 3. It is not clear to me what the extra code in the constructor is for. (This is probably just because I don't know the blog code at all.) You may wish to add a comment explaining.
        Hide
        Jason Fowler added a comment -

        Created the list of events

        the extra code in the constructor is to provide details about the association of the blog to the assignments that use the triggers.

        thanks for that Tim, pushing for integration now.

        Show
        Jason Fowler added a comment - Created the list of events the extra code in the constructor is to provide details about the association of the blog to the assignments that use the triggers. thanks for that Tim, pushing for integration now.
        Hide
        Tim Hunt added a comment -

        I don't see any changes at the diff URL. Did you remember to push?

        Show
        Tim Hunt added a comment - I don't see any changes at the diff URL. Did you remember to push?
        Hide
        Jason Fowler added a comment -

        I forgot to force the push, thanks for spotting that Tim

        Show
        Jason Fowler added a comment - I forgot to force the push, thanks for spotting that Tim
        Hide
        Dan Poltawski 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
        Dan Poltawski 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 -

        This has been integrated thanks Jason

        Show
        Sam Hemelryk added a comment - This has been integrated thanks Jason
        Hide
        Rossiani Wijaya added a comment -

        Tested this as admin and student, it works fine.

        Test passed.

        Show
        Rossiani Wijaya added a comment - Tested this as admin and student, it works fine. Test passed.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

        Closing as fixed, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: