ISSUE-16 Fix problem when deleting a discussion when filters with conten...

[nitzo]

...t and headings are enabled

[nitzo]

To reproduce:

Enable a filter on content and headings (Site wide).
Create a new discussion
Try to the delete the new discussion.

The problem occurs because the filter system initializes the THEME before a course_module is set on the forum.

I suggest calling $PAGE->set_cm() before calling $PAGE->set_title() in mod_forumng::init_page() (Line 3672 of mod_forumng.php)

[jason-platts]

I haven't been able to replicate this problem (Moodle 2.5). Can I ask:

• What version of Moodle are you using?
• What filter is enabled, does this happen with any filter?
• Is there content in the discussion title that will be picked up by the filter?
• Does this happen with any theme?

It does seem very strange that the problem occurs when you delete a discussion as init_page() is called on pretty much every page in forumng, do you get that error elsewhere?

[nitzo]

Hi,

We are using Moodle 2.4 currently.

I tried this on Moodle 2.5, fresh installation with only forumNG installed - The problem does not happen. On Moodle 2.4 (tried even on 2.4.7), fresh installation with only forumNG installed - The problem occurs.

Forgot to check 2.5 Sorry for that...

I checked with "Activity names auto-linking" - just change "Apply to" to "content and headings" (I think is happens with every filter). The discussion's title does not have to have content that is picked up by the filter. It happens on every theme. (I checked with standard and with our custom theme).

I guess it does not happen on 2.5 because something in the filter system has changed. (I went briefly over ForumNG's code in init_page() and delete.php and the code looks that same for 2.4 and 2.5).

Would it be possible to merge this into the 2.4 branch? (For the sake of users who did not migrate to 2.5 yet)?

Thanks!

[jason-platts]

Does the error happen on many pages in forumng, or just when deleting the discussion?

Essentially we've stopped support for 2.4 now as the 2.5 version of forumng is due to be released in the next week or so.

If I can work out why the problem is happening there may be a fix to correct it that is valid for later versions also. I'm not really happy making your proposed fix as I don't really see why that makes a difference - though I don't think it will have any negative impact, it just seems a bit of a 'hack'. You could always implement the code change locally anyway as there won't be any more 2.4 branch updates, so there is no danger of a future conflict.

[nitzo]

Hi !

It happens only when deleting the discussion. (As far as I know right now). All the common functionality works (viewing, creating discussions).

I agree that that patch is a hack so I investigated some more and it seems that the difference between discuss.php and delete.php is that the permissions checking ( $discussion->require_view() ) in discuss.php are checked before calling init_page().

Apparently $forum->require_view() eventually calls moodle's require_login() which sets up the $PAGE before the filters set it up.

That is why this happens in delete.php and no where else. Again this is probably a bug in moodle 2.4 (Filters unintentionally cause init of $PAGE and it seems to be solved in 2.5).

I still suggest you maybe move the $discussion->require_edit() a few lines above in delete.php. If you need I will create a PULL request.

Look at the proposed change here:

https://github.com/nitzo/moodle-mod_forumng/commit/4a5e77ba29c3bbf53d63518b97a40c4818198071

[jason-platts]

Thanks, require_edit() should be before the init_page() call so I'll make that change to the Master branch.

[jason-platts]

I've integrated your fix into our Master branch - this will turn up in GitHub on our next refresh (which should be around the end of this week). Thanks again.

[nitzo]

Thanks !