Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#6646 closed defect (bug) (fixed)

Ensure the /groups/single-groups/members url renders regardless of activity component

Reported by: jmarx75 Owned by: imath
Milestone: 2.4 Priority: normal
Severity: normal Version:
Component: Groups Keywords: has-patch commit
Cc: jeffrey_marx@…

Description

One of the unfortunate side effects of #6388 is that if you deactivate the activity component, groups/single-group/members ends up throwing a 404. This is my first BuddyPress ticket, so it took me some time to figure out that these inner-group pages will not work unless they are placed in the groups nav. It appears there is a callback function for each page that fires when the user clicks the nav item. Example, when the user clicks the members link ('or if it exists in the nav already'), the groups_screen_group_members() function fires and takes care of loading in the member’s view. If 'members' does not exist in the nav, then that function never fires.

My solution may be over simplistic so I am looking forward to feedback. I am hooking into the bp_template_redirect hook and loading that callback function if the current action is "members". I realize this may be considered an edge case but I think the /members url should always work regardless of the status of the activity component or whether "members" is in the group nav or not. I feel like there would be the possibility of a community having an activity feed and deciding at some point to deactivate it. If that happens, then this side effect could potentially break a lot of users’ bookmarks.

Attachments (6)

404.png (366.6 KB) - added by jmarx75 5 years ago.
work.png (284.5 KB) - added by jmarx75 5 years ago.
membersload.patch (867 bytes) - added by jmarx75 5 years ago.
The patch
6646.patch (857 bytes) - added by imath 5 years ago.
6646.02.patch (1.1 KB) - added by imath 5 years ago.
6646.03.patch (1.0 KB) - added by imath 5 years ago.

Download all attachments as: .zip

Change History (20)

@jmarx75
5 years ago

@jmarx75
5 years ago

@jmarx75
5 years ago

The patch

#1 @imath
5 years ago

Thanks for your feedback and for the patch.

I guess if we do it for members, we should also do it for activity. As you said this is an edge case, because you are not clicking on a link but manually adding the 'members' to the url.

I'll look at it.

#2 @imath
5 years ago

  • Component changed from Component - Members to Component - Groups
  • Keywords reporter-feedback added
  • Milestone changed from Awaiting Review to 2.4
  • Version 2.3.3 deleted

@jmarx75 could you test 6646.patch, i think it's fixing the issue. But it would be great to have your confirmation about it.

@imath
5 years ago

#3 @jmarx75
5 years ago

Hey @imath I definitely tested it.
I apologize as this IS my first rodeo :)
To demonstrate I tested it, is there a standard procedure?
Here is a screenshot of that url without the patch https://buddypress.trac.wordpress.org/attachment/ticket/6646/work.png
And one with the patch https://buddypress.trac.wordpress.org/attachment/ticket/6646/work.png

#4 @jmarx75
5 years ago

Hey @imath I definitely tested it.
I apologize as this IS my first rodeo :)
To demonstrate I tested it, is there a standard procedure?
Here is a screenshot of that url without the patch https://buddypress.trac.wordpress.org/attachment/ticket/6646/404.png
And one with the patch https://buddypress.trac.wordpress.org/attachment/ticket/6646/work.png

#5 @jmarx75
5 years ago

  • Cc jeffrey_marx@… added
  • Keywords reporter-feedback removed

Never mind. I see now that you added a new patch.
Thanks!

#6 @jmarx75
5 years ago

Just tested your patch @imath and it works great. I don't think any compensation needs to be made for /groups/single-group/activity because 'activity' never appears in the nav, right? it's either slapped on the group's home page or it's deactivated I believe.

#7 @imath
5 years ago

Thanks a lot for your tests and for confirming patch is fixing the issue :)

I think even if 'activity' never appeared in the nav, it will when the group's has a custom front page.

And the case could be similar: Let's imagine there's a buddypress/groups/single/front-status-public.php template and that the group is public, then the activity nav is appearing. Let's say a member of the group is bookmarking the link. Then the administrator changes the visibility of his group and there's no buddypress/groups/single/front-status-private.php template, then the activity is available at the home level, and if we don't include the activity in the canonical check, then going to site.url/groups/mygroup/activity > 404.

So i say: let's do it for both :)

#8 @imath
5 years ago

Ah! My fix wasn't good. It apparently worked because by unseting the action members and activity was redirected to home. But as soon as you add some action variables to the url it fails.

The right fix is to reset the action to home, as in your case members is at the home level, and in my case activity is at the home level. I'll run some other tests to be completely sure :)

You can test 6646.02.patch if you want :)

@imath
5 years ago

#9 @imath
5 years ago

  • Keywords commit added

Ok i got it fixed!

6646.03.patch is 6646.02.patch improved.

Will commit 6646.03.patch in a few hours :)

@imath
5 years ago

#10 @jmarx75
5 years ago

Oh cool, thanks. I was just about to start testing :)
I will test this anyways.

#11 @jmarx75
5 years ago

Ah, nice. I see the urls redirect to home now if that is where the missing content exists.
Patch tested and works for me :)

#12 @imath
5 years ago

Great! thanks a lot for your confirmation @jmarx75 :)

#13 @jmarx75
5 years ago

Any time @imath.
I'm looking forward to helping out some more!

#14 @imath
5 years ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 10205:

Groups Homes: make sure the action parameter of the canonical stack is reset to home in two specific cases.

When the Group does not use a custom front.php template:
(1) If the Activity component is not active, the home page displays the members of the Group.
(2) If the Activity component is active, the home page displays the activities of the Group.

In case (1), a user trying to directly access to site.url/groups/single/members must be redirected
to the home page.
In case (2), a user trying to directly access to site.url/groups/single/activity must be redirected to the home page.

Props jmarx75 (Congrats for your first ticket and patch, welcome on board!)

Fixes #6646

Note: See TracTickets for help on using tickets.