8675 - user group invites by adiloztaser · Pull Request #15 · buddypress/buddypress

adiloztaser · 2022-04-05T22:34:41Z

Trac ticket: https://buddypress.trac.wordpress.org/ticket/8675

This Pull Request is for code review only. Please keep all other discussion in the BuddyPress Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the WordPress Core Handbook for more details.

…hing group invites.

imath

Thanks a lot for this PR. I agree it's visually fixing the issue. But if an Admin clicks on the button to accept the invite for the user then:

we have an Ajax error in Nouveau and the invite is not accepted.
we have a regular error in Legacy but the invite is accepted.

So changing this behavior needs more changes probably:

But it might not just be a matter of switching from logged in user ID to displayed user ID, it's probably a broader issue.

src/bp-templates/bp-nouveau/buddypress/members/single/groups.php

src/bp-templates/bp-legacy/buddypress/members/single/groups.php

Subnavs only register if user can see the page so it looks we don't need these extra controls.

adiloztaser · 2022-04-09T09:53:16Z

Hey @imath, thanks for the review. Actually I've never tried to use Accept/Reject buttons, I'll look asap.

…domain. This looks like more safer then previous for custom themes and plugins.

adiloztaser · 2022-04-11T20:32:26Z

Hi again @imath,

I believe accept/reject buttons should work for admins after my last commits. Can you look again?

I also agree about your concern to inform users but I'm not sure where to generate it.

imath

Hi @adiloztaser

Thanks a lot for your changes 👍. I've just tested, it works as expected. About emailing the user, i'd use the 'groups_reject_invite' & 'groups_accept_invite' hook to check if ( $user_id !== bp_loggedin_user_id() && bp_current_user_can( 'bp_moderate' ) to decide whether to send a notification email.

src/bp-groups/bp-groups-template.php

adiloztaser · 2022-04-13T21:15:37Z

Hi again @imath,

I've added an email notification based on your suggestion.
I have two questions:
1-) Should we create a new email or just use existing ones?
2-) Should we add unsubscribe link to email or we always inform users after that action?

I'm also open to any suggestion about current version.

imath · 2022-04-14T19:54:05Z

Thanks a lot @adiloztaser for these improvements.

I think we should have two specific emails eg: "An administrator accepted an invitation to join {{group.name}} on your behalf. If you disagree with this, you can leave the group at anytime using this link: {{{leave-group.url}}}

In this case we don't need the unsubscribe link.

The new code you added looks great.

adiloztaser · 2022-04-15T09:30:25Z

Thank you for feedback @imath, I'll keep working on it.

src/bp-groups/bp-groups-notifications.php

adiloztaser · 2022-04-15T20:25:52Z

Hi there.

I've added new emails, as you guess to do that I needed to increase the db_version. I don't know is there any rule for this, if I did anything wrong please let me know.

I couldn't add leave url to accepted invate email because we have nonce check and I didn't create a valid url. I've added member groups page, I think it's good enough for navigating user.

imath

It's looking great thanks a lot for you work on it @adiloztaser

I just think we should do the small changes I suggested in my review 😉

src/bp-core/bp-core-functions.php

src/bp-core/bp-core-update.php

src/bp-groups/screens/user/invites.php

src/class-buddypress.php

Co-authored-by: imath <imathi.eu@outlook.fr>

adiloztaser · 2022-04-17T11:36:09Z

Thank you for review and suggestions @imath. I hope its ready to go.

imath

Looks good! Thanks a lot @adiloztaser I'll commit it to our SVN Trunk asap.

As a site admin can view the displayed user groups invites, listed invites have to be the one of this user and not the ones of the site admin. Adapt the Group Invites feature so that site admins can accept or reject on behalf of the displayed user the listed invites. These two actions made by an admin will generate a specific BP Email informing the user of it. Props oztaser, dcavins, espellcaste Closes buddypress/buddypress#15 Fixes #8675 git-svn-id: https://buddypress.svn.wordpress.org/trunk@13273 cdf35c40-ae34-48e0-9cc9-0c9da1808c22

As a site admin can view the displayed user groups invites, listed invites have to be the one of this user and not the ones of the site admin. Adapt the Group Invites feature so that site admins can accept or reject on behalf of the displayed user the listed invites. These two actions made by an admin will generate a specific BP Email informing the user of it. Props oztaser, dcavins, espellcaste Closes buddypress/buddypress#15 Fixes #8675 git-svn-id: http://buddypress.svn.wordpress.org/trunk@13273 cdf35c40-ae34-48e0-9cc9-0c9da1808c22

As a site admin can view the displayed user groups invites, listed invites have to be the one of this user and not the ones of the site admin. Adapt the Group Invites feature so that site admins can accept or reject on behalf of the displayed user the listed invites. These two actions made by an admin will generate a specific BP Email informing the user of it. Props oztaser, dcavins, espellcaste Closes buddypress/buddypress#15 Fixes #8675 git-svn-id: https://buddypress.svn.wordpress.org/trunk@13273 cdf35c40-ae34-48e0-9cc9-0c9da1808c22

adiloztaser added 2 commits April 6, 2022 01:30

Use bp_displayed_user_id instead of bp_loggedin_user_id when fect…

0424921

…hing group invites.

Show groups subnav if user can edit settings.

3627515

imath suggested changes Apr 9, 2022

View reviewed changes

src/bp-templates/bp-nouveau/buddypress/members/single/groups.php Outdated Show resolved Hide resolved

src/bp-templates/bp-legacy/buddypress/members/single/groups.php Outdated Show resolved Hide resolved

Remove if statements for displaying subnav.

c4e4bad

Subnavs only register if user can see the page so it looks we don't need these extra controls.

adiloztaser added 2 commits April 9, 2022 22:59

Fix actions when admin accept/reject group invites for other users.

2256f23

Use displayed user domain if it exists, otherwise use logged-in user …

c5ae0f2

…domain. This looks like more safer then previous for custom themes and plugins.

adiloztaser requested a review from imath April 11, 2022 20:32

imath suggested changes Apr 13, 2022

View reviewed changes

src/bp-groups/bp-groups-template.php Outdated Show resolved Hide resolved

src/bp-groups/bp-groups-template.php Outdated Show resolved Hide resolved

adiloztaser added 2 commits April 13, 2022 22:48

Use bp_is_user to check if user page.

9e36a21

Send email notification to user when admin reject/accept invite.

6cf1b5b

renatonascalves reviewed Apr 15, 2022

View reviewed changes

src/bp-groups/bp-groups-notifications.php Show resolved Hide resolved

src/bp-groups/bp-groups-notifications.php Outdated Show resolved Hide resolved

adiloztaser added 3 commits April 15, 2022 18:57

Bail if group is not exists.

5dcaf0f

Cast $user_id to integer for better check.

3c55b72

Add new emails for group memembership intives.

0fdbc7e

adiloztaser requested review from imath and renatonascalves April 15, 2022 20:26

Fix indentation.

b20bf30

imath suggested changes Apr 17, 2022

View reviewed changes

adiloztaser and others added 6 commits April 17, 2022 14:25

Update src/bp-core/bp-core-functions.php

e2929f5

Co-authored-by: imath <imathi.eu@outlook.fr>

Improve accepted email message.

f4f45d5

Co-authored-by: imath <imathi.eu@outlook.fr>

Update src/bp-core/bp-core-functions.php

4b14cc2

Co-authored-by: imath <imathi.eu@outlook.fr>

Remove leading tab.

b67906e

Co-authored-by: imath <imathi.eu@outlook.fr>

Remove leading tab.

8b28f8c

Co-authored-by: imath <imathi.eu@outlook.fr>

Improve email content,

c4af754

Co-authored-by: imath <imathi.eu@outlook.fr>

adiloztaser and others added 6 commits April 17, 2022 14:30

Use latest revision number as db_version.

4df5568

Co-authored-by: imath <imathi.eu@outlook.fr>

Use latest revision number as db_version.

9a5880f

Co-authored-by: imath <imathi.eu@outlook.fr>

Add space after !.

8a23d97

Co-authored-by: imath <imathi.eu@outlook.fr>

Add spave after !

ce55e38

Co-authored-by: imath <imathi.eu@outlook.fr>

Remove leading tab.

7e01455

Co-authored-by: imath <imathi.eu@outlook.fr>

Remove leading tab.

0299e04

Co-authored-by: imath <imathi.eu@outlook.fr>

adiloztaser requested a review from imath April 17, 2022 11:36

imath approved these changes Apr 20, 2022

View reviewed changes

boonebgorges closed this in 883c934 Apr 22, 2022

adiloztaser deleted the 8675_user_group_invites branch April 22, 2022 06:20

Conversation

adiloztaser commented Apr 5, 2022

Uh oh!

imath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adiloztaser commented Apr 9, 2022

Uh oh!

adiloztaser commented Apr 11, 2022

Uh oh!

imath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adiloztaser commented Apr 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imath commented Apr 14, 2022

Uh oh!

adiloztaser commented Apr 15, 2022

Uh oh!

Uh oh!

Uh oh!

adiloztaser commented Apr 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adiloztaser commented Apr 17, 2022

Uh oh!

imath left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adiloztaser commented Apr 13, 2022 •

edited

Loading

adiloztaser commented Apr 15, 2022 •

edited

Loading