Details

You could start by following the replication steps in the description and ensure that you don't get the same problem.

Make sure that you have at lease two groups for this course.

Create a collaborative wiki activity with visible groups.

As the admin create the first page for all participants - put content like 'all participants' in the body so that you can recognise which group you are in.

Now change over to another group. Make the first page for them (make sure that you can also identify this page).

Continue to switch between groups. Create new pages.

log in as students in the course and create new pages.

[What to look out for]
Ensure that the group drop down box is the same as the page that you are viewing.
Make sure that even if you are view a page in a different group, that when you create a new page it's for your group.
Try creating a wiki with separate groups and check that there are no problems there either.

You could start by following the replication steps in the description and ensure that you don't get the same problem.
Make sure that you have at lease two groups for this course.
Create a collaborative wiki activity with visible groups.
As the admin create the first page for all participants - put content like 'all participants' in the body so that you can recognise which group you are in.
Now change over to another group. Make the first page for them (make sure that you can also identify this page).
Continue to switch between groups. Create new pages.
log in as students in the course and create new pages.
[What to look out for]
Ensure that the group drop down box is the same as the page that you are viewing.
Make sure that even if you are view a page in a different group, that when you create a new page it's for your group.
Try creating a wiki with separate groups and check that there are no problems there either.

Description

When wiki page group mode is set to either 'Visible groups' or 'Separate groups'. The groups selector displayed on wiki/view.php page (a) does not reflect the current group (b) changing the groups does not bring you to correct group page.

The investigation identified the following issues:

1. The current activity group in wiki view page is not updated to selected
group in most cases, making dropdown menu displaying the wrong group. Having
said that, the displayed page may also belong to the the wrong group, I have seen a
situation that after selecting group, both dropdown menu and page do not match
neither each other nor the requested page.

2. I guess the wiki title is sometimes malformed for groups (spaces in the original title are replaced with +), which makes it redirecting to the
'first page', and even causes 'redirect loop' or prompting for group page creation (while it has been created already).

3. The title used as hidden parameter in group dropdown always reflect the title of the previously selected page, so the attempt to switch group ends up with non-existing page and redirect user to 'first page' (which might even be not the main page either).

Get back to moodle from error page (just open the url and the course page), click on wiki activity we created.

(observation 4) Wiki prompts you to create the new page 'First page name'

Proceed creating it, the surprising thing is that it will belong to 'Group 1' as well under different name, so name it 'Group 1 new'.

Now try switching between 'Group 1' and 'All participants' several time, you will be able to spot all three pages one after other, but in most cases the displayed page and selector will not be reflecting the one you have chosen.

If you like to observe even more weirdness, you may introduce a second page by selecting 'Group 2' in the menu, and play switching between them.

Michael de Raadt
added a comment - 28/Nov/11 11:20 AM Thanks for reporting this.
I've put that on the backlog.
In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.

Michael de Raadt
added a comment - 28/Nov/11 4:44 PM There has been some recent work on Wiki group functionality that may be relevant here. It might have resolved the problem in this issue, but I'm not sure.

I found this same problem occurring only with groups set visible. There is some inconsistency in the code, the other cases in the switch statement in the renderer.php file don't try to urlencode the title (hence the problem). One solution to this, the one that makes the renderer.php most consistent is attached. Tested under 2.1.3+ diffed against MOODLE_22_STABLE (the patch is equivalent).

Matt Kolb
added a comment - 20/Dec/11 9:56 AM I found this same problem occurring only with groups set visible. There is some inconsistency in the code, the other cases in the switch statement in the renderer.php file don't try to urlencode the title (hence the problem). One solution to this, the one that makes the renderer.php most consistent is attached. Tested under 2.1.3+ diffed against MOODLE_22_STABLE (the patch is equivalent).

Matt Kolb
added a comment - 20/Dec/11 10:23 PM The patch is also available here (applies cleanly to 21_STABLE, 22_STABLE and master):
https://github.com/mkolb/moodle/commit/884a1b3e8110cf7bf0a2a78cb299324cbb400fff
or on the branch (against current):
https://github.com/mkolb/moodle/tree/MDL-30478

Sorry, I have missed the patch somehow and did not check it earlier. I have verified it, the suggested patch partly solves the problem , e.g. it definitely fixes the page naming, and you no longer get 'This web page has a redirect loop' error, but some issues are still there:

Switching between groups and 'All participants' is still broken. The content of the page is always the last one when you switch from any group to 'All participants', there seems no way to see 'All participants' page content now.

Once above is fixed,

The mechanism of fixing pages that have already been created with the wrong name should be added in upgrade function

Ruslan Kabalin
added a comment - 23/Feb/12 6:19 PM Sorry, I have missed the patch somehow and did not check it earlier. I have verified it, the suggested patch partly solves the problem , e.g. it definitely fixes the page naming, and you no longer get 'This web page has a redirect loop' error, but some issues are still there:
Switching between groups and 'All participants' is still broken. The content of the page is always the last one when you switch from any group to 'All participants', there seems no way to see 'All participants' page content now.
Once above is fixed,
The mechanism of fixing pages that have already been created with the wrong name should be added in upgrade function

I removed 'gid' as a parameter being passed via the url to view.php so that group id is only passed through 'group'. There is an addition to the grouplib.php functions groups_get_activity_group and groups_print_activity_menu.
This should fix all of the issues mentioned in the description.

Adrian Greeve
added a comment - 29/Mar/12 8:22 AM I removed 'gid' as a parameter being passed via the url to view.php so that group id is only passed through 'group'. There is an addition to the grouplib.php functions groups_get_activity_group and groups_print_activity_menu.
This should fix all of the issues mentioned in the description.

I am suspicious of the need for changing the groups_get_activity_group behaviour. All the other activity modules manage by passing around the parameter in the URL. Can you not pass the group parameter in the url?

You are doing 'database' things in pagelib.php, my understanding of how that the wiki is structured is that this is the 'renderer' which does 'view' things in an MVC sense. I'm not sure how accurate I am on that - but the lack of $DB in that file suggests this is true. This logic should almost certainly be moved into create.php which I think calls this.

Dan Poltawski
added a comment - 02/Apr/12 2:13 PM Hi Adrian,
Comments having looks through this:
I am suspicious of the need for changing the groups_get_activity_group behaviour. All the other activity modules manage by passing around the parameter in the URL. Can you not pass the group parameter in the url?
You are doing 'database' things in pagelib.php, my understanding of how that the wiki is structured is that this is the 'renderer' which does 'view' things in an MVC sense. I'm not sure how accurate I am on that - but the lack of $DB in that file suggests this is true. This logic should almost certainly be moved into create.php which I think calls this.
This code:
if (!$cm = get_coursemodule_from_instance('wiki', $wikiid)) {
die ('grrr');
Probably wants to use the MUST_EXIST $strictness parmeter?

Adrian Greeve
added a comment - 03/Apr/12 10:09 AM Thanks Dan,
I removed the changes to the grouplib functions. It seems to be working okay now with out that. I also moved the stuff that you were talking about in point 2 to create.php

Sorry for these comments coming in drips - i'm learning about wiki/groups as I review (this is a tricky piece of code which is not well understood).

The changes you have made now make it much easier for me to follow, thanks.

Are you sure that your selecting of the group to display is accurate (does wiki support seperate groups mode?). Looking at other activity modules they explicitly check for SEPERATEGROUPS. (I am not intimately familiar with this, best to see what other modules do)

The pagelib function you've added set_currentgroups() - this doesn't sound accurate to me, the current group is the one set by the set_gid call no? Would this be better described as available groups?

Also in pagelib: this is changing the $this->subwiki in every case, is that intentional? if (!$this->subwiki = wiki_get_subwiki_by_group($this->wid, $groupid)) {

mod/wiki/lib.php - Might be clearer to remove $gid alltogether and set to 0 (I am not sure why we are setting that to 0 there)

mod/wiki/create_form.php groupid and groupinfo element seem to be the wrong way round (groupinfo should be the hidden field with groupid?) Also rather than doing a for loop you could use an array function to get the first element as we are only expecting one?

Dan Poltawski
added a comment - 03/Apr/12 10:56 AM Hi Adrain,
Sorry for these comments coming in drips - i'm learning about wiki/groups as I review (this is a tricky piece of code which is not well understood).
The changes you have made now make it much easier for me to follow, thanks.
Are you sure that your selecting of the group to display is accurate (does wiki support seperate groups mode?). Looking at other activity modules they explicitly check for SEPERATEGROUPS. (I am not intimately familiar with this, best to see what other modules do)
The pagelib function you've added set_currentgroups() - this doesn't sound accurate to me, the current group is the one set by the set_gid call no? Would this be better described as available groups?
Also in pagelib: this is changing the $this->subwiki in every case, is that intentional? if (!$this->subwiki = wiki_get_subwiki_by_group($this->wid, $groupid)) {
mod/wiki/lib.php - Might be clearer to remove $gid alltogether and set to 0 (I am not sure why we are setting that to 0 there)
mod/wiki/create_form.php groupid and groupinfo element seem to be the wrong way round (groupinfo should be the hidden field with groupid?) Also rather than doing a for loop you could use an array function to get the first element as we are only expecting one?

1) I'm pretty sure that I am selecting the group to display accurately. Wiki does support groups mode. I tested this using separate groups and it behaves correctly. The calls that I make to the group API has checks in them for which group mode is being used.
2) I've changed the function to set_availablegroups.
3) This is intentional. Removing either line that starts with $this->subwiki will stop the module from working properly. The first line makes sure that the page is created for the right group. The second '$this->subwiki' if removed will result in errors when creating a page.
4) I don't know why I edited this file. I've reverted it back to what it was and it doesn't break anything.
5) I've changed this to just get the first item of the array.
6) I've removed this optional parameter as it isn't used in this file.

Adrian Greeve
added a comment - 03/Apr/12 3:04 PM Hi Dan,
1) I'm pretty sure that I am selecting the group to display accurately. Wiki does support groups mode. I tested this using separate groups and it behaves correctly. The calls that I make to the group API has checks in them for which group mode is being used.
2) I've changed the function to set_availablegroups.
3) This is intentional. Removing either line that starts with $this->subwiki will stop the module from working properly. The first line makes sure that the page is created for the right group. The second '$this->subwiki' if removed will result in errors when creating a page.
4) I don't know why I edited this file. I've reverted it back to what it was and it doesn't break anything.
5) I've changed this to just get the first item of the array.
6) I've removed this optional parameter as it isn't used in this file.

Hi Dan,
Thanks for your help with this issue.
I re-based my patch and made a couple of changes to the code. They are:
#I changed the name of the element name from 'groupinfo' to 'groupdescription'
#I have now included a check to see if $this->_customdata['groups']->availablegroups is empty or not, to remove a notice that I was getting.

Adrian Greeve
added a comment - 18/Apr/12 3:10 PM Hi Dan,
Thanks for your help with this issue.
I re-based my patch and made a couple of changes to the code. They are:
#I changed the name of the element name from 'groupinfo' to 'groupdescription'
#I have now included a check to see if $this->_customdata ['groups'] ->availablegroups is empty or not, to remove a notice that I was getting.

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.

Dan Poltawski
added a comment - 19/Apr/12 9:57 PM 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

Andrew Davis
added a comment - 26/Apr/12 4:46 PM Testing in both stable branches went fine. I did raise MDL-32647 but that's a pre-existing issue.
Also Adrian, could you take a look at MDL-31188 ? Sounds like a probable duplicate to me but you'd know better than I would. If it was resolved by this issue then assign it to yourself and close it

Eloy Lafuente (stronk7)
added a comment - 27/Apr/12 11:13 PM This has been near becoming rejected, because it's not the best code you are able to produce.
But, luckily, at the end, it has landed and has been spread to all repos out there.
Many thanks and, don't forget it, keep improving your skills, you can!
Closing, ciao