Assorted changes 2

[adekbadek]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

1. Tweaks to the memberships deduplication CLI command, added in Membership de-duplication CLI command #84
2. A fix to username generation, to ensure too-long usernames are not used
3. Adds handling of syncing of users who already exist on the other site, but have a non-synchronizable role
4. Some refactoring

(Note: this is a part of integrating the changes from the data-integrity-improvements branch (#89))

How to test the changes in this Pull Request:

1. Test membership deduplication command, as described in Membership de-duplication CLI command #84, but instead of a --dry-run flag observe it will dry run by default, making any changes only with --live flag provided. This brings it in line with other CLI command in this repository.
2. On one network site, using CLI (front-end won't let you!), create a Subscriber user with an email address that exceeds 60 characters.
   1. Wait for the user to sync, or trigger a sync
   2. Observe that the user's username on the other network sites is based on the email address, but cut to not exceed 60 characters
3. Create two users on two different network sites, with the same email address, but different roles:
   1. Create an Editor (a non-synchronizable role) using WP Admin or WP CLI
   2. Register using the Registration block (or the auth modal) on the second site
   3. Run the reader-registered-event backfill (wp newspack-network data-backfill reader_registered --start=2020-01-01 --end=2024-12-31 --live) from the second site
   4. Wait for the user to sync, or trigger a sync
   5. Observe that on the first site, the user has both roles now

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

[dkoo]

I'm not sure if this is something wrong with my local Network sites' setup, but for steps 2 and 3, when the node site's users got synced to my hub site, they came in without any role.

Also getting a fatal error in the Users page—see below.

[dkoo]

This new constant needs to be imported in class-users.php, too: https://github.com/Automattic/newspack-network/blob/assorted-changes-2/includes/class-users.php#L78

[adekbadek]

Fixed in 8f91df9, thanks!

[adekbadek]

when the node site's users got synced to my hub site, they came in without any role.

That should not ever happen, can you please list the exact reproduction steps?

Also, I've realized the step 3 instructions were wrong (synced user has to register using RAS, not be created with CLI), so I've updated this step. I don't think this relates to the issue described above, though.

[adekbadek]

@dkoo – I've requested a re-review, but what I meant was pinging you about the reproductions steps, as mentioned in the comment above.

[dkoo]

Got this working. Everything is working as described except for this step:

Observe that the user's username on the other network sites is based on the email address, but cut to not exceed 60 characters

In my testing, the user with the really long email address got propagated across the network, but the username was set to match the username of the account on the original site (not the email address). This username string was already shorter than 60 characters because CLI wouldn't let me create a user with a username longer than that.

[adekbadek]

Ah, that's right – the user_name will be overridden if already set on the origin site:

newspack-network/includes/incoming-events/class-user-manually-synced.php

Line 59 in 3f9755e

'user_login' => $this->data->user_login ?? $email,

[matticbot]

🎉 This PR is included in version 1.9.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[matticbot]

🎉 This PR is included in version 1.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀