-
Notifications
You must be signed in to change notification settings - Fork 26
feat: get content that uses a specified integration #441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -286,6 +286,57 @@ set_integrations <- function(content, integrations) { | |
| invisible(NULL) | ||
| } | ||
|
|
||
|
|
||
| #' Get content items using an OAuth integration | ||
| #' | ||
| #' @description | ||
| #' Retrieves a list of content items that are associated with a specific OAuth | ||
| #' integration. | ||
| #' | ||
| #' You must have administrator privileges to use this function. | ||
| #' | ||
| #' @param x A `connect_integration` object (as returned by [get_integrations()], | ||
| #' [get_integration()], [create_integration()], or [update_integration()]). | ||
| #' | ||
| #' @return A list of `Content` R6 objects representing all pieces of content | ||
| #' that use the specified OAuth integration. See [content_item()] for details | ||
| #' on the object. | ||
| #' | ||
| #' @seealso | ||
| #' [get_integrations()], [get_integration()], [get_associations()], | ||
| #' [content_item()] | ||
| #' | ||
| #' @examples | ||
| #' \dontrun{ | ||
| #' client <- connect() | ||
| #' | ||
| #' # Get an integration | ||
| #' integration <- get_integration(client, "12345678-90ab-cdef-1234-567890abcdef") | ||
| #' | ||
| #' # Find all content using this integration | ||
| #' content_using_integration <- get_content_list(integration) | ||
| #' } | ||
| #' | ||
| #' @family oauth integration functions | ||
| #' @export | ||
| get_content_list <- function(x) { | ||
| if (!inherits(x, "connect_integration")) { | ||
| stop("'x' must be a 'connect_integration' object") | ||
| } | ||
| client <- attr(x, "client") | ||
| error_if_less_than(client$version, "2024.12.0") | ||
| assoc <- client$GET(v1_url( | ||
| "oauth", | ||
| "integrations", | ||
| x$guid, | ||
| "associations" | ||
| )) | ||
| purrr::map( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be a potentially large list, so I'm not sure how wise this. I wonder if a better way to expose this would be through the content search, adding an Alternatively, we could expose a different endpoint off of
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, I'll give that a go. |
||
| assoc, | ||
| ~ content_item(client, .x$content_guid) | ||
| ) | ||
| } | ||
|
|
||
| #' Get OAuth associations for a piece of content | ||
| #' | ||
| #' @description | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| [ | ||
| { | ||
| "app_guid": "12345678", | ||
| "content_guid": "12345678", | ||
| "oauth_integration_guid": "0000001", | ||
| "oauth_integration_name": "Integration 1", | ||
| "oauth_integration_description": "This is the first description", | ||
| "oauth_integration_template": "template1", | ||
| "oauth_integration_auth_type": "Viewer", | ||
| "created_time": "2025-08-05T20:02:39Z" | ||
| } | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing something specific to an oauth integration is not what I would expect a function called
get_content_list()to do.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i would agree here. i think naming could be an issue. I almost always prefer explicit naming conventions like:
get_associated_content_for_integration()or something like that.get_content_listis too similar to what I would expected to get a list of content objects.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this point of view — I would definitely be open to a more verbose name. If I did change it to something like
get_content_for_integration(), then I should also change theget_integration.Content()method added here to something more descriptive.For context: The way #437 PR works,
get_integrations()returns a list of integration objects but uses a different method based on what it's called with:get_integrations.Connect()gets all integration on the server, andget_integrations.Content()gets all integrations used by the piece of content you provide. Following that naming scheme exactly,get_content.integration()(so called withget_content(integration)) would be how this is implemented, but there's an existingget_content()function that returns a data frame, and we want to keep return types consistent within a given function. The idea, though, is that (in a future PR) you'd callget_content_list(client)to get a list of Content objects — kinda like howget_job_list()works. So the idea is that it is a more general name, but uses method dispatch to get you the list of content objects that are determined by the object you pass in.Yesterday @jonkeane and I talked about a few different approaches, and this is the one I decided to move forward with. But having a more descriptive name and not using method dispatch (I'd probably go with
get_content_for_integration()and the converseget_integrations_for_content()) is another option we looked at, and I'd be happy to go with that if folks think it's a better option.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if you implement the search keyword for this like we discussed in the other comment, maybe you don't even need a function for it, it's just
search_content(client, integration=)or however you call search. So maybe we don't need to worry what to name this.