Project

General

Profile

Bug #14194

Viewing Group Invitations

Added by Chris Stein 10 months ago. Updated 10 months ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Category name:
Group Invitations
Target version:
Start date:
2021-03-17
Due date:
% Done:

0%

Estimated time:

Description

This has to do with viewing group invitations.

There are two places to view invitations on your Commons Profile:

1. Invites: commons.gc.cuny.edu/members/<username>/invites/

2. Groups > Invitations: commons.gc.cuny.edu/members/<username>/groups/invites/

In the Invites page I did see the invitation but there was no type, it did not let me know it was a group invite and what group. See attached Invitations-received.png

In the Groups > Invitations page I did not see the invitation at all. See attached Groups-invitations.png

A third place is in the Notifications you get in the Admin bar. I received a notification in the bar when membership request was accepted but there was not a notification about the invitation. I can't remember if the notification of the invitation is accepted behavior or not. See attached Notifications.png

The email with the group invitation was sent. Everything seems to be working for that.

Groups-invitations.png (36.4 KB) Groups-invitations.png Chris Stein, 2021-03-17 04:09 PM
invitations-received.png (40.8 KB) invitations-received.png Chris Stein, 2021-03-17 04:09 PM
Notifications.png (26.1 KB) Notifications.png Chris Stein, 2021-03-17 04:09 PM

History

#1 Updated by Boone Gorges 10 months ago

  • Assignee set to Raymond Hoh

Thanks for the detailed report, Chris.

Ray, could you have a look?

For one thing, it seems like we should not allow users to visit Groups > Invitations at all.

The blank "type" column seems like a bug with the way the Invitations panel is rendered. The invitation object (27430) exists in the DB and seems OK at a glance.

#2 Updated by Raymond Hoh 10 months ago

The blank line stems due to Chris already being a member of the invited group: https://github.com/cuny-academic-commons/cac-onboarding/blob/ea1d18b2f2851894fc9566bc1a7f44be7e54eb96/src/Invitation/Invitation.php#L327-L332

Since Chris is already a member of the group, this bypasses how the group line is rendered on the "My Invites > Received" page: https://github.com/cuny-academic-commons/cac-onboarding/blob/ea1d18b2f2851894fc9566bc1a7f44be7e54eb96/src/Frontend/Helpers.php#L62

According to the wp_bp_groups_members DB table, Chris joined the group yesterday at 2021-03-17 19:34:30, but the invite in the wp_1_cac_invites DB table was not removed. This explains how Chris probably used the "My Groups > Invitations" page to accept the invite instead of through "My Invites".

For one thing, it seems like we should not allow users to visit Groups > Invitations at all.

Boone, do we want to hide the "Groups > Invitations" subnav item or remove it entirely with bp_core_remove_subnav_item()?

I think hiding might be the best approach since removing entirely might conflict with any BP invite action functionality.

Secondly, when we fetch a CACO invite's group memberships here: https://github.com/cuny-academic-commons/cac-onboarding/blob/ea1d18b2f2851894fc9566bc1a7f44be7e54eb96/src/Invitation/Invitation.php#L327-L332

We should probably delete the CACO invite if there are no further groups or sites to join.

Or we could write a script to remove any orphaned group CACO invites instead.

#3 Updated by Matt Gold 10 months ago

One note on the sequence of events here:

1. I invited Chris to the group.
2. He had not yet joined, but visited the group home page and requested membership
3. I approved the request.

So, I'm not certain that his being a member of the group was an issue at the time he reported this problem

#4 Updated by Raymond Hoh 10 months ago

So, I'm not certain that his being a member of the group was an issue at the time he reported this problem

That's correct. The issue occurred when the "My Groups > Invitations" page was used to accept the invite instead of the main "My Invites" page. We need to do what Chris recommended in #14195 to solve this half of the issue.


Boone, I'm currently writing a script to go through all pending invites with group memberships and no site memberships to see if there are any invites that should be marked as accepted.

Does this logic look correct to you? The reason I added a clause for no site memberships is because this issue is likely to be only for group invites.

$results = $GLOBALS['wpdb']->get_results( "SELECT id from wp_1_cac_invites WHERE status = 'pending' AND group_memberships != '[]' AND site_memberships = '[]'" );

foreach ( $results as $result ) {
    $invite = new CAC\Onboarding\Invitation\Invitation( $result->id );

    // Use 'filtered' to grab groups that the user is not a member of yet.
    $group_invites = $invite->get_group_memberships( 'filtered' );

    // Bail if we still have a group invite.
    if ( ! empty( $group_invites ) ) {
        continue;
    }

    // No more group invites, so mark as accepted.
    /*
    $invite->set_status( 'accepted' );

    // @todo use last group membership date as date_modified?
    //$invite->set_date_modified( '' );
    $invite->save();
    */
}

There are approximately 4000 pending group invites to parse through.

#5 Updated by Boone Gorges 10 months ago

Thanks for having a look, Ray.

We should probably delete the CACO invite if there are no further groups or sites to join.

Yes. I've made this change in #14195.

Does this logic look correct to you? The reason I added a clause for no site memberships is because this issue is likely to be only for group invites.

I think it mostly looks right, though I can imagine a situation where the same issue might come up for sites. And I think we need to account for the scenario where an invitation might include more than one group membership. See https://github.com/cuny-academic-commons/cac-onboarding/commit/4de95e5b31b7e7db6b27abb2e17cc0687ae1c549 for how I handle this: only mark the item as accepted if the other type (sites in the case of groups, and vice versa) is empty, and there's only a single membership attached to the group.

#6 Updated by Raymond Hoh 10 months ago

And I think we need to account for the scenario where an invitation might include more than one group membership.

$invite->get_group_memberships( 'filtered' ) already handles checking if the user is already a member of all invited groups so we should be okay here. If there are outstanding pending group invites, that method will let us know. Same thing for sites with $invite->get_site_memberships( 'filtered' ).

(sites in the case of groups, and vice versa)

I'll handle doing the same thing for sites.

Do we care much about the date_modified timestamp for sites? I don't think it's possible to determine when a user joined a site retroactively.

#7 Updated by Boone Gorges 10 months ago

Do we care much about the date_modified timestamp for sites? I don't think it's possible to determine when a user joined a site retroactively.

Nah, I don't think we care about this.

$invite->get_group_memberships( 'filtered' ) already handles checking if the user is already a member of all invited groups so we should be okay here. If there are outstanding pending group invites, that method will let us know. Same thing for sites with $invite->get_site_memberships( 'filtered' ).

If I invite you to groups A, B, and C; and you later join group C; then we should keep the invitation, but it should only reference groups A and C. The invitation should only be marked as accepted if your group join "clears out" the invitation. Right?

#8 Updated by Raymond Hoh 10 months ago

If I invite you to groups A, B, and C; and you later join group C; then we should keep the invitation, but it should only reference groups A and C. The invitation should only be marked as accepted if your group join "clears out" the invitation. Right?

Yes, that's right. In this scenario, I believe $invite->get_group_memberships( 'filtered' ) would return groups A and B: https://github.com/cuny-academic-commons/cac-onboarding/blob/ea1d18b2f2851894fc9566bc1a7f44be7e54eb96/src/Invitation/Invitation.php#L327-L332

So in my script above, we don't go forward with accepting the CACO invite because we still have pending invites for group A and B.

#9 Updated by Boone Gorges 10 months ago

Right, but in that case we want to remove the membership: $invite->remove_group_membership( B );

#10 Updated by Raymond Hoh 10 months ago

$invite->remove_group_membership( B )

You mean C right?

I get what you mean now. I don't think it's entirely necessary to remove the group membership since the user is already a member of the group and $invite->accept() only looks at filtered groups: https://github.com/cuny-academic-commons/cac-onboarding/blob/ea1d18b2f2851894fc9566bc1a7f44be7e54eb96/src/Invitation/Invitation.php#L180

It might be good to keep the initial groups that the user was invited to.

#11 Updated by Boone Gorges 10 months ago

Ah, I see what you mean now too. Yes, you're probably right that it's unnecessary to remove the membership from the invitation because of the 'filtered' status.

#12 Updated by Boone Gorges 10 months ago

  • Target version set to 1.18.8

#13 Updated by Raymond Hoh 10 months ago

  • Status changed from New to Resolved

This is done.

Just ran the CLI script to go through all pending invitations that were already completed through an alternative join action, but not marked as accepted in the cac_invites table.

Also available in: Atom PDF