Migrate mailing list functions to XM Directory API#388
Conversation
all_mailinglists() and fetch_mailinglist() previously used the Research Core Contacts endpoints which Qualtrics is retiring on June 30, 2026. This migrates both functions to the XM Directory API endpoints, with automatic discovery of the user's directory ID. Fixes #386.
|
The Windows CI failure is a 429 rate-limit error from the Qualtrics API on pre-existing fetch_survey() tests — not related to this PR's changes. |
juliasilge
left a comment
There was a problem hiding this comment.
Thanks for taking this on so much! 🙌 I'm really glad that we might be able to sneak this XM Directory migration in ahead of or very near the June 30, 2026 retirement.
All of this is really great:
- the automatic directory discovery with a
QUALTRICS_DIRECTORY_IDoverride - session caching that follows the package's existing pattern (cleared in
zzz.Ralongside the API key and base URL) - really nice
generate_urlrefactor
Tests pass locally for me, and the Windows CI passed when I clicked to re-run it. I do notice that both tests set QUALTRICS_DIRECTORY_ID, which short-circuits fetch_directory_id() before it calls the API, so the discovery branch and the "No XM Directories found" abort has no coverage right now. What do you think about a small test that mocks qualtrics_api_request to exercise both paths?
| @@ -1,5 +1,11 @@ | |||
| # qualtRics (development version) | |||
|
|
|||
| - Migrated `all_mailinglists()` and `fetch_mailinglist()` from the deprecated | |||
There was a problem hiding this comment.
The new endpoints return a different shape, which you can see from the updated tests:
all_mailinglists():id➡️mailingListId;libraryId/category/folderdropped;contactCount/ownerId/dates added.fetch_mailinglist():id➡️contactId;externalDataReference➡️extRef.
Existing code that references x$id or x$externalDataReference will silently get NULL. The NEWS line here "existing code requires no changes" is accurate only about directory discovery, but readers will probably take it as full backward compatibility. Could you update NEWS to call out the renamed and changed columns explicitly?
There was a problem hiding this comment.
Good call — updated. I reworded the "no changes" line so it's clearly scoped to the directory discovery, and added explicit bullets listing the renamed/dropped/added columns for both functions (id → mailingListId, externalDataReference → extRef, etc.). Commit 0139d04.
There was a problem hiding this comment.
Can you update the roxygen docs in this file?
- The
@param mailinglistIDdescription says it is "Returned asidby all_mailinglists"; that field is nowmailingListId. - The example
fetch_mailinglist(mailinglists$id[1])now passesNULL. It should bemailinglists$mailingListId[1].
There was a problem hiding this comment.
Fixed — @param mailinglistID now says mailingListId, and the example uses mailinglists$mailingListId[1]. Commit 0139d04.
| #' to discover it and caches the result for the session. | ||
| #' | ||
| #' @keywords internal | ||
| fetch_directory_id <- function() { |
There was a problem hiding this comment.
IIUC fetch_directory_id() silently picks the first directory here, and also IIUC accounts can have more than one XM Directory (although I'm not an expert on this). If that's right, dirs[[1]] could quietly target the wrong one with no signal to the user. The QUALTRICS_DIRECTORY_ID escape hatch is really nice, but it is not documented anywhere user-facing. Could we document that env var in the user-facing function (not the internal one), and warn (is error too much?) when multiple directories are returned so users know to set it?
There was a problem hiding this comment.
You're right that an account can have more than one directory. fetch_directory_id() now emits a warning when it finds multiple, naming the one it picked and pointing to QUALTRICS_DIRECTORY_ID. I went with a warning rather than an error so existing single-directory setups aren't interrupted — happy to make it an abort if you'd prefer. I also documented the env var in the @details of both user-facing functions. Commit 0139d04.
There was a problem hiding this comment.
That's a good argument for a warning; makes sense!
| paginate_api_request = function(fetch_url) { | ||
| list(list( | ||
| contactCount = 5L, | ||
| mailingListId = "CG_abc123", | ||
| name = "Test List", | ||
| lastModifiedDate = 1700000000000, | ||
| creationDate = 1700000000000, | ||
| ownerId = "GR_abc123" | ||
| )) |
There was a problem hiding this comment.
These tests now mock paginate_api_request with hand-crafted data rather than the stoplight mock. Is this because of network issues? You confirmed these against a live response?
There was a problem hiding this comment.
Good question. These two tests stub paginate_api_request with inline data rather than going through the stoplight mock, because the mock doesn't have the new directory-scoped route. Hitting /directories/{id}/mailinglists on the mock returns Prism's "Route not resolved, no path matched" (as a 422). The only route
that still resolves is the old flat /mailinglists, and it returns the deprecated Research Core schema (libraryId/id/category/folder) — i.e. exactly what we're migrating away from. So there's no way to exercise the new endpoints through that mock; stubbing paginate_api_request also drops the network dependency.
The stub data isn't guessed, though — I confirmed the response shape against a live call to my own Qualtrics account (214 mailing lists and 509 contacts) and copied the actual field names and types into the tests. That live run is where the schema changes (id → mailingListId, externalDataReference → extRef, etc.) surfaced.
I kept these as inline stubs rather than a recorded vcr cassette on purpose: fetch_mailinglist() returns live contact records (my test call pulled 509 real contacts), and I didn't want real names/emails landing in a cassette in a public repo. If you'd prefer a cassette, I'm happy to record one against a small throwaway mailing list with synthetic contacts so there's nothing sensitive to scrub — just say the word.
There was a problem hiding this comment.
That makes a lot of sense to me; no need to change!
- Document the changed mailing-list column names in NEWS - Fix fetch_mailinglist roxygen param/example to use mailingListId - Warn when multiple XM Directories exist and document QUALTRICS_DIRECTORY_ID in the user-facing functions - Add tests covering directory discovery, the no-directory abort, and the multiple-directory warning
|
On the test coverage — added a dedicated |
juliasilge
left a comment
There was a problem hiding this comment.
Thank you so much for working on this! 🚀
Summary
all_mailinglists()andfetch_mailinglist()previously used the legacy Research Core Contacts endpoints which Qualtrics is retiring on June 30, 2026 (Migrateall_mailinglists()andfetch_mailinglist()to new Qualtrics endpoints #386)/directories/{directoryId}/mailinglists)Test plan
test-all-mailinglists.R,test-fetch-mailinglist.R)all_mailinglists()returns expected resultsfetch_mailinglist()returns contacts for a given list