feat: connect_user S3 class

[karawoo]

Creates new S3 classes connect_user and connect_users. Functions that previously took a user GUID as an argument now can take either the GUID or the connect_user object.

This is still a draft while we talk through the right approach for phasing in this new object and moving away from the data frame-centric return object of get_users().

• For now get_users() still returns a data frame.
• get_users_list() returns a list of connect_user objects; get_users() just calls get_users_list() and converts to a data frame. I don't love this naming at all.
• Arguments that were previously named user_guid, owner_guid, etc. are now user/owner to reflect the fact that they can be either the GUID or object. This may be a pretty significant breaking change but it also doesn't seem right to keep _guid in there when it might not be a guid.

I'm a little tempted to deprecate passing the GUID strings with a warning, and then a few releases down the line remove support for passing the GUID. At the same time we can maybe swap get_users() to returning a list and remove/deprecate get_users_list(). Would love others' thoughts as they look at this though, I think there are a lot of potential options.

Checklist

• Does this change update NEWS.md (referencing the connected issue if necessary)?
• Does this change need documentation? Have you run devtools::document()?

[toph-allen]

Thanks for working on this! Added some comments thinking through different aspects of this & adding context.

[toph-allen]

Naming! @karawoo and I talked about this yesterday and there are a few options available, including what's used here, just calling it users(), etc.

I was thinking back to a similar discussion with @jonkeane at the start of the year about job-related functions, particularly during #356 (the comment threads are hard to follow and not particularly worth mining).

If we wanted to follow the pattern used with get_job_list() exactly, we'd call this get_user_list() (singular). I think in the abstract I prefer get_users_list(), but if we have a _list() naming scheme, I think it should be consistent in the part of speech used.

I also think it would be fine to call it users().

Calling it get_user_list() feels like a choice that prioritizes clarity in a mixed-paradigm period where it coexists alongside get_users() (which returns a data frame).

Calling it users() would be more confusing when in a mixed-paradigm period ("How is it different from get_users()?"), but would be much nicer for people using the newer object-based system. Perhaps deprecating get_users() would help clarify that the new way to do things is call users() |> as_tibble().

[toph-allen]

Just a note — the list-of-integrations object's class is connect_integration_list (see here).

Not saying that that's a better paradigm, and these class names are internal — there isn't much sunk cost here, so if connect_{object_plural} feels better and more comprehensible to future code-readers, we could do that.

[toph-allen]

💟

[toph-allen]

Thinking ahead in this line of work to functions that act on a single user. We'd like to be able to something like lock_user(user), without having to also pass in the client as the first argument.

Different objects in the package achieve this in different ways. The R6 classes have the client as a property. The jobs (just a list, no class) have the client as an added list item. Integrations (an S3 class, which is the most similar to what we're doing here) have the client as an attribute — the thing I like about that is that it keeps the list fields as the data returned by the server, which feels neat and tidy to me.

That doesn't need to happen in this PR — it looks like most of the user-facing functions that are being updated are ones that also operate on content, and so already have access to a client.