31 August 2013, 0:04

Hi,

I'm working on a plugin to replace the watchlist-notifications. The plugin targets two problems:
- the watchlist-notification only tells you which page was changed but nothing about which elements were touched
- when you work on a page for some time modifying different block-instances (or the same one all the time), the system sends a notification about any 5 minutes.

The plugin works fine so far, I hook into the 'blockinstancecommit'-event, store the editing user, view and blockelement, use a cron-job to find the elements that have been changed but which haven't been touched in a while, hand the data to a subclass of ActivityTypeWatchlist and use notify-users() to send out the modified notifications, also listing all the modified elements on that page.

But I can't find a way to disable the built-in notifications from whithin the plugin-code or the database. I tried deleting the watchlist-entry in activity_type, but that results in errors on the frontend whenever I try to edit a blocktype. Also, when you edit a page, the notifications are triggered once you click the edit-icon on one of the blocks (which is an AJAX-request). So there's no way to redirect anything in menu_items(). Neither is changing the target an option as the blocks.json.php-targets are all produced in the /js/views.js

Anyone got any idea how to disable the built-in watchlist-notifications without messing with the core-code?

02 September 2013, 8:01

Hello Tobias,

thank you for tackling this issue. There is already a bug report at https://bugs.launchpad.net/mahara/+bug/1041228 for this. A notification is sent whenever a block was edited and not only once you finish the page and click "Done" because clicking "Done" is not required.

Sorry, can't help with the tech side of things.

Cheers

Kristina

03 September 2013, 0:40

Hello Kristina,

thanks for your reply. I did read the bug report and I understand that the "Done" button is just a link back to the page-list and clicking "Done" is not required. 

The blockinstancecommit-event however works fine. Also it should be easy to rearrange the built-in watchlist-notification system to fire on that event - inside the mahara core. But changes on the core are difficult to maintain whenever I upgrade to another version of mahara, which is why I want to keep the logic in an independent plugin, as much as possible. 

For now I disable the notifications in the core-code (I deleted "activity_occured" in view.php:process_changes). Only other idea I had was deleting the watchlist-events from the activity_queue-table in the database with a cron-job but that feels dirty and even more difficult to maintain. 

03 September 2013, 13:47

Hi Tobias,

I've taken a look at it, and I don't think there is any way you could disable the core Watchlist notifications without modifying core code. Like you said, if you remove the entry from the activity_type table, then activity_locate_typerecord() will just throw an exception.

In addition to commenting it out in lib/view.php, you'll also need to comment out the activity_occurred in the function artefact_watchlist_notification() in artefact/lib.php

It sounds like you're implementing an improved version of the Watchlist Activity that solves some of the problems we've been having with it. Would you be willing to upstream your fixes into Mahara core? It's too late in the release cycle to get it into Mahara 1.8, but we could consider it for 1.9

Cheers,

Aaron

04 September 2013, 0:58

Hi Aaron,

thanks for the help. You're right, I didn't realize that saving an artefact also results in watchlist-notifications for the views. So additionally I register for the deleteartefacts- and saveartefact-event. 

Sure, I'd be happy to share the plugin and the code-changes with you. Do you have a rough plan when you'll be releasing version 1.9? 

04 September 2013, 11:45

We're on a six-monthly release cycle these days. 1.8 is due out around the end of this month, and 1.9 wil be scheduled for April 2014.

If you're comfortable with git & gerrit, the easiest thing for us would be if you could push your patch into our gerrit code review system, https://reviews.mahara.org . Instructions on setting that up are here: https://wiki.mahara.org/index.php/Developer_Area/Contributing_Code

But if that's too much effort, we'd be happy to accept it in any format -- a link to a github branch, or a patch file, or a zip archive, or whatever.

Cheers,

Aaron

05 September 2013, 18:36

Hi Aaron,

April 2014 doesn't sound too bad. 

I'll let you know when the plugin is ready and uploaded. I'm sure I'll get along with gerrit, the explanation behind your link looks straight forward. 

@ Kristina: It's actually the other way round, I'm programming with Mahara 1.6 so we use it right away and port the plugin for 1.9 to upload it into gerrit. 

And yes, I'm looking forward to save time with the upgrades when the plugin is managed in the core code :)

Cheers,

Tobias

12 September 2013, 3:11

Hi Aaron,

I uploaded the plugin and the two changes in the core-code into gerrit. 

There are two things I'm not totally sure about:

first I extended ActivityTypeWatchlistnotification and overwrote get_message to create a meaningful message - but I also had to overwrite get_type() to obfuscate that this is not really a full ActivityType. I think that otherwise ActivityType threw an error. Is that as it should be? 

second I put the number of minutes to wait for changes on the same view into the artefact_config table but I dind't find another way but to use the upgrade.php-script and calling the function in PluginArtefact::postinst(). Is there an easier way to put data into the database? And is there a way to get data from the artefact-config table into the settings-menu from within the plugins?

Cheers

Tobias

16 September 2013, 12:17

Hi Tobias,

Sorry for the slow reply! I don't always have a lot of time to respond to the forum every day, and your question was tricky enough I couldn't answer it off the top of my head.

I see the code you uploaded, here: https://reviews.mahara.org/#/c/2490/ and here https://reviews.mahara.org/#/c/2491/  (For the time being, those tickets have been marked with a -2 code review to make sure we don't accidentally merge them in before the Mahara 1.8 release.) I mentioned a couple of bookkeeping things you can do for the patch, in my comments on the patch (i.e. create a Launchpad bug for the feature, put the bug number in the commit message, and change the formatting for the commit message.)

Anyway, to answer your questions:

1. It looks like you're using an Artefact plugin as just a placeholder to install the Activity and cron tasks? Since we're reviewing this for inclusion in core, that's unnecessary. I was actually thinking that we would fold your ActityTypeWatchlistnotification in as a replacement for the existing ActivityTypeWatchlist in lib/activity.php. The cron task can then be registered as a standard core cron task (search for "activity_process_queue" to see an example of what a core cron task looks like; it needs to be installed via the "core_install_firstcoredata_defaults()" function in lib/upgrade.php, and inserted directly into the "cron" table in lib/db/upgrade.php, and the cron function itself can live in lib/activity.php)

2. For inserting records into the database during the initial installation of Mahara or a plugin, you have to use the preinst and/or postinst functions. You can get data from the artefact_config table using the get_config_plugin() function (and, actually, the preferred way to put artefact_config values into the DB is calling set_config_plugin(), rather than direct DB insert). But since we're going to be eliminating that artefact and just putting this in core, you'll need to make it a core config instead, which means you use get_config() and set_config(). I'd recommend adding it to the "Site Options" page. (admin/site/options.php).

I hope that answers your current questions. Let me know if you've got any others. You can also try the #mahara-dev room on irc.freenode.net

Cheers,

Aaron

20 September 2013, 0:21

Hi Aaron,

thanks for the input. Never mind the delay. I have a couple of (follow up) questions about the implementation:

I put the ActityTypeWatchlistnotification-class into the activity.php, as well as the functions for the cron-job and the event-listener. Getting these registered in the update.php-scripts shouldn't be a problem. Or would you have me merge the class-changes into the ActivityTypeWatchlist-class?
What about the table that I introduced with the plugin, is there a comfortable way to reuse the install.xml-file in the upgrade.php-scripts?

And about the review-tool: I read the comments and found the bugtracking-number that Kristina changed to fit the modifications. But how can I change the comments in the comits that I already uploaded? Do I have to use git rebase and amend and then force push the changes into gerrit? Can I edit the message online?