CUNY Academic Commons

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.

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

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.

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.

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.

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?

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.

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

$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.

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.