fix: update get_content ptype and docs

[toph-allen]

Intent

Update the ptype and documentation for get_content() to include recently-added columns. The ptype update means that the columns will be parsed with proper types.

Fixes #461

Checklist

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

[jonkeane]

I'm fine with us moving forward with this, though I'm surprised that there are no test changes needed here.

But that might also signal a separate, bigger, and IMO more important question: what is the value that we (or our users) get out of maintaining this structure and mapping? It seems like a lot of work, not adding much value (the descriptions are just copy/pasted from the API docs, yeah?) and a lot of work for supporting the pattern of the package that we are moving away from. And if we're not even testing that this does the thing we are expecting, do we really care about it that much?

[toph-allen]

I'm fine with us moving forward with this, though I'm surprised that there are no test changes needed here.

But that might also signal a separate, bigger, and IMO more important question: what is the value that we (or our users) get out of maintaining this structure and mapping? It seems like a lot of work, not adding much value (the descriptions are just copy/pasted from the API docs, yeah?) and a lot of work for supporting the pattern of the package that we are moving away from. And if we're not even testing that this does the thing we are expecting, do we really care about it that much?

Test or code changes apparently are needed — in integration tests. 😅

I think I see the point you're driving at. (Fwiw, updating this wasn't much work. But to your point, the need to remember to update these is a burden.)

The value in the ptype usage is that the data frames produced by connectapi have columns are parsed properly with appropriate types, which is really nice as a user of those data frames — from my perspective, that benefit is worth the tradeoff. It adds a layer of polish and ease of use.

I see us dropping this in the future if we totally deprecate converting to data frames! But I could also see an argument that it's worth keeping, or approaching in a different way — and I don't think I know enough about how that would unfold to really have that discussion yet.

[karawoo]

I'm somewhat on team drop the type mapping. Looking at these updates, many of the field names being added have been present in API responses since early 2023. So we're clearly not keeping up with mapping these fields as it is. That said, it's not a hill I'm going to die on, so @toph-allen if you want to keep it and own it, it's ok with me.

[toph-allen]

I'm somewhat on team drop the type mapping. Looking at these updates, many of the field names being added have been present in API responses since early 2023. So we're clearly not keeping up with mapping these fields as it is. That said, it's not a hill I'm going to die on, so @toph-allen if you want to keep it and own it, it's ok with me.

@karawoo Yeah, wrote up #473 to track this. Between this and discussion on #470, it feels like parsing should happen either during the actual JSON parse from httr or during conversion to an object (probably the former — I just don't know how that works and what work would be needed to handle dates correctly there).

I def agree that, as connectapi moves towards lists & R objects as representations of server objects, parsing/munging during the conversion to a data frame is the wrong place for it. (I also think the current approach is also a big performance bottleneck.) I have some open questions about how we should handle certain data types (mostly dates) from the server, but… yeah, I'm sure there are better ways to do that than manually maintaining a set of prototypes that fall wildly out of sync.