Fix behaviour when submitting non locking pages twice

[hughdavenport]

Issue #2 describes an issue where if you have your plugin set up in a
non locking manner, and submit the same page to multiple assignments,
then only the latest link will work.

The reason for this is the Mahara generates a token in an mt parameter
that changes on every submit. The non locking behaviour allows multiple
submissions, but to do that, it submits then releases, hence generating
a new token.

This patch, along with a patch in Mahara, allows sending a lock
parameter to Mahara, and then appending the url with the moodle
assignment and the mahara view/collection. Mahara will then send a
request back to Moodle to test whether you have permission to view that
view, and if so, stores the result in the session. This allows all
submission urls to work.

For upgrade paths, as this changes the get_view_url method, no change to
existing submissions is needed.

This patch makes a requirement on either a 4 line core Moodle patch, or
a local plugin, as there are issues with subplugin MNet publishers.

fixes #2

[hughdavenport]

4 line Moodle core patch would be:

diff --git a/lib/upgradelib.php b/lib/upgradelib.php
index bf22448..414c35e 100644
--- a/lib/upgradelib.php
+++ b/lib/upgradelib.php
@@ -1736,7 +1736,11 @@ function upgrade_plugin_mnet_functions($component) {
                 $dataobject->classname     = $method['classname'];
                 $dataobject->filename      = $method['filename'];
             }
-            $dataobject->xmlrpcpath = $type.'/'.$plugin.'/'.$dataobject->filename.'/'.$method;
+            $xmlrpcpath = $path;
+            if (strstr($xmlrpcpath, $CFG->dirroot) == 0) {
+                $xmlrpcpath = substr($xmlrpcpath, strlen($CFG->dirroot) + 1);
+            }
+            $dataobject->xmlrpcpath = $xmlrpcpath.'/'.$dataobject->filename.'/'.$method;
             $dataobject->static = false;

             require_once($path . '/' . $dataobject->filename);

[hughdavenport]

Local plugin required: catalyst/moodle-local_mahara@177a8b8

[kabalin]

Hmm, moodle-local_plugin is de-facto deprecated and not required for any version of Moodle above 2.5 (see requirements section https://moodle.org/plugins/view/assignsubmission_mahara). I am not sure if we can easily respawn it, as in current implementation may conflict with what moodle-assignsubmission_mahara delivers.

[kabalin]

I see, you removed all old stuff from moodle-local_plugin. Might worth then creating a master branch for moodle-local_plugin and keeping 2.3 branch as it is. Have you submitted a patch to Moodle core as well?

[agwells]

Hi Ruslan,

I told Hugh to start his local repo by forking the old one, because it'll make the code management strategy easier, particularly in the Moodle Plugin Database site.

I figured it would be less confusing in the Moodle Plugin site if I add these as later releases to the existing Mahara local plugin rather than creating a second Mahara local plugin. Even though the two plugins are really completely unrelated.

But, the Moodle Plugin site only allows a plugin to have one source control URL. So, my plan was:

1. Hugh forks from the old local plugin repo
2. I fork from Hugh's repo into a new local plugin repo in the MaharaProject github
3. I set the MaharaProject local plugin repo (which now contains source code for the old local plugin and the new one) as the source control for the local plugin, in the Moodle Plugin site.

We will just have to make sure that if you're updating directly from Moodle 2.5 -> 2.8, the old local plugin gets properly done away with.

[agwells]

But you're probably right that it should be on a different branch in the repo. :)

[kabalin]

Can we avoid local plugin completely? Either by making the change into Moodle core and releasing the feature with corresponding plugin version update (along with Moodle version release), or by reimplementing the same change in the moodle-assignsubmission_mahara?

[agwells]

The problem Hugh ran into, is that Moodle's code for allowing a plugin to publish an MNet function, does not support "subplugins" like assign/submission. It only works correctly for top-level plugin types. So, bringing the "local" plugin back to life seems like the best workaround for right now. Literally the only thing it does is publish the existence of that MNet function.

Hugh also experimented with inserting the necessary records directly into the appropriate Moodle tables, but he found that didn't work. Quoting him from an IRC discussion we had yesterday: "ok, can't be done with upgrade/install path, cos they are both called before the mnet function, and that does a require_once on mnet.php to load methodnames (so won't load it second time around), and if it isn't there, it disables that function"

Getting Moodle to upstream the patch would be ideal, but we're somewhat doubtful of whether they'll accept any new MNet-related patches, or of how quickly we'd be able to make that happen, or whether they'd be willing to backport it to the current stable versions of Moodle.

[kabalin]

I see, thanks for detailed answer. Ok, local plugin back to life then. And start rewriting plugin to use webservices instead of MNet, hehe :)

BTW, thanks for your work on this @hughdavenport!

[agwells]

Indeed!

[agwells]

This code change also requires a corresponding code change in Mahara (which I'm reviewing at the same time): https://reviews.mahara.org/#/c/5704/

[agwells]

And here's the Moodle patch request: https://tracker.moodle.org/browse/MDL-52172

[agwells]

Just an update on this one. It has passed testing, so I'll try to merge it in and clean up the documentation and all, in the next couple of days.

I would like to wait on publishing the new version until the Mahara patch for it gets released in the next Mahara minor release, though. So that we don't have to distribute a patch file with it, and complicate the README with patching instructions again.

https://reviews.mahara.org/#/c/5704/

The code in issue2-fix branch is meant to be 100% back-compatible with non-upgraded Mahara sites, though. In the case that it's interfacing with a non-upgraded site, it'll just fall back to using the old "mt=" access URLs. Likewise, the Mahara patch is 100% back-compatible with a non-upgraded Moodle assignment submission plugin.

In Hugh's original implementation, the Moodle & Mahara sides used somewhat indirect methods to determine whether they were dealing with an upgraded or non-upgraded site on the opposite end. I wound up rewriting that to instead include an "apilevel" parameter, which should also help make it easier to implement future changes to the API between these plugins.

[agwells]

Oh dear, I never got around to merging this one! Well, no time like the present I guess.

[agwells]

Hm, shoot, the Moodle compatibility patch (https://tracker.moodle.org/browse/MDL-52172) still hasn't been merged yet. Well, I'll have to give this some more thought then. :( I really don't want to have to go back to bundling a local plugin again, or requiring a patch. Maybe I can get some traction on the Moodle bug.