Skip to content

clean up user import form validation#475

Merged
spgarbet merged 4 commits intomainfrom
issue-474-forms
Nov 24, 2025
Merged

clean up user import form validation#475
spgarbet merged 4 commits intomainfrom
issue-474-forms

Conversation

@couthcommander
Copy link
Copy Markdown
Contributor

This PR may address #465 and #474.

@spgarbet
Copy link
Copy Markdown
Member

Are there any tests that can check this?

@obregos
Copy link
Copy Markdown
Collaborator

obregos commented Nov 19, 2025

── Failed tests ──────────────────────────────────────────────────────────────
Error (test-101-userMethods-ArgumentValidation.R:5:3): (code run outside of `test_that()`)
Error in `devtools::test()`: 2 assertions failed:
 * At least one user is missing an entry for the form(s): record_id
 * At least one user is missing an export entry for the form(s): record_id
Backtrace:
    ▆
 1. ├─redcapAPI::importUsers(...) at test-101-userMethods-ArgumentValidation.R:5:3
 2. └─redcapAPI:::importUsers.redcapApiConnection(...) at redcapAPI/R/importUsers.R:6:3
 3.   └─redcapAPI::prepUserImportData(data, rcon = rcon, consolidate = consolidate) at redcapAPI/R/importUsers.R:51:3
 4.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Error (test-101-userMethods-Functionality.R:13:5): Import / Export Users
Error in `devtools::test()`: 2 assertions failed:
 * At least one user is missing an entry for the form(s): record_id
 * At least one user is missing an export entry for the form(s): record_id
Backtrace:
    ▆
 1. ├─redcapAPI::importUsers(rcon, data = data.frame(username = EXPENDABLE_USER)) at test-101-userMethods-Functionality.R:13:5
 2. └─redcapAPI:::importUsers.redcapApiConnection(rcon, data = data.frame(username = EXPENDABLE_USER)) at redcapAPI/R/importUsers.R:6:3
 3.   └─redcapAPI::prepUserImportData(data, rcon = rcon, consolidate = consolidate) at redcapAPI/R/importUsers.R:51:3
 4.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Error (test-101-userMethods-Functionality.R:43:5): Import Users Options
Error in `devtools::test()`: 2 assertions failed:
 * At least one user is missing an entry for the form(s): record_id
 * At least one user is missing an export entry for the form(s): record_id
Backtrace:
    ▆
 1. ├─redcapAPI::importUsers(rcon, data = data.frame(username = EXPENDABLE_USER)) at test-101-userMethods-Functionality.R:43:5
 2. └─redcapAPI:::importUsers.redcapApiConnection(rcon, data = data.frame(username = EXPENDABLE_USER)) at redcapAPI/R/importUsers.R:6:3
 3.   └─redcapAPI::prepUserImportData(data, rcon = rcon, consolidate = consolidate) at redcapAPI/R/importUsers.R:51:3
 4.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Error (test-101-userMethods-Functionality.R:102:5): Export User Options
Error in `devtools::test()`: 2 assertions failed:
 * At least one user is missing an entry for the form(s): record_id
 * At least one user is missing an export entry for the form(s): record_id
Backtrace:
    ▆
 1. ├─redcapAPI::importUsers(...) at test-101-userMethods-Functionality.R:102:5
 2. └─redcapAPI:::importUsers.redcapApiConnection(...) at redcapAPI/R/importUsers.R:6:3
 3.   └─redcapAPI::prepUserImportData(data, rcon = rcon, consolidate = consolidate) at redcapAPI/R/importUsers.R:51:3
 4.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Error (test-101-userMethods-Functionality.R:150:5): Delete User Functionality
Error in `devtools::test()`: 1 assertions failed:
 * Variable 'users': Must be a subset of {'beckca','benjamin_nutter','garbetsp','obregos','tanh3'}, but has additional elements {'bstat_api_user'}.
Backtrace:
    ▆
 1. ├─redcapAPI::deleteUsers(rcon, users = EXPENDABLE_USER) at test-101-userMethods-Functionality.R:150:5
 2. └─redcapAPI:::deleteUsers.redcapApiConnection(rcon, users = EXPENDABLE_USER) at redcapAPI/R/deleteUsers.R:8:3
 3.   └─checkmate::reportAssertions(coll) at redcapAPI/R/deleteUsers.R:41:3

Failure (test-102-userRoleMethods-ArgumentValidation.R:108:5): Validate config, api_param
`importUserRoles(...)` threw an error with unexpected message.
Expected match: "'config': Must have names"
Actual message: "2 assertions failed:\n * At least one user is missing an entry for the form(s): record_id\n * At least one user is missing an export entry for the form(s): record_id"
Backtrace:
    ▆
 1. ├─testthat::expect_error(...) at test-102-userRoleMethods-ArgumentValidation.R:108:5
 2. │ └─testthat:::quasi_capture(...)
 3. │   ├─testthat (local) .capture(...)
 4. │   │ └─base::withCallingHandlers(...)
 5. │   └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
 6. ├─redcapAPI::importUserRoles(...)
 7. └─redcapAPI:::importUserRoles.redcapApiConnection(...) at redcapAPI/R/importUserRoles.R:8:3
 8.   └─redcapAPI::prepUserImportData(...) at redcapAPI/R/importUserRoles.R:48:3
 9.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Failure (test-102-userRoleMethods-ArgumentValidation.R:112:5): Validate config, api_param
`importUserRoles(...)` threw an error with unexpected message.
Expected match: "'config': Must be of type 'list'"
Actual message: "2 assertions failed:\n * At least one user is missing an entry for the form(s): record_id\n * At least one user is missing an export entry for the form(s): record_id"
Backtrace:
    ▆
 1. ├─testthat::expect_error(...) at test-102-userRoleMethods-ArgumentValidation.R:112:5
 2. │ └─testthat:::quasi_capture(...)
 3. │   ├─testthat (local) .capture(...)
 4. │   │ └─base::withCallingHandlers(...)
 5. │   └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
 6. ├─redcapAPI::importUserRoles(...)
 7. └─redcapAPI:::importUserRoles.redcapApiConnection(...) at redcapAPI/R/importUserRoles.R:8:3
 8.   └─redcapAPI::prepUserImportData(...) at redcapAPI/R/importUserRoles.R:48:3
 9.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Failure (test-102-userRoleMethods-ArgumentValidation.R:117:5): Validate config, api_param
`importUserRoles(...)` threw an error with unexpected message.
Expected match: "'api_param': Must have names"
Actual message: "2 assertions failed:\n * At least one user is missing an entry for the form(s): record_id\n * At least one user is missing an export entry for the form(s): record_id"
Backtrace:
    ▆
 1. ├─testthat::expect_error(...) at test-102-userRoleMethods-ArgumentValidation.R:117:5
 2. │ └─testthat:::quasi_capture(...)
 3. │   ├─testthat (local) .capture(...)
 4. │   │ └─base::withCallingHandlers(...)
 5. │   └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
 6. ├─redcapAPI::importUserRoles(...)
 7. └─redcapAPI:::importUserRoles.redcapApiConnection(...) at redcapAPI/R/importUserRoles.R:8:3
 8.   └─redcapAPI::prepUserImportData(...) at redcapAPI/R/importUserRoles.R:48:3
 9.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Failure (test-102-userRoleMethods-ArgumentValidation.R:121:5): Validate config, api_param
`importUserRoles(...)` threw an error with unexpected message.
Expected match: "'api_param': Must be of type 'list'"
Actual message: "2 assertions failed:\n * At least one user is missing an entry for the form(s): record_id\n * At least one user is missing an export entry for the form(s): record_id"
Backtrace:
    ▆
 1. ├─testthat::expect_error(...) at test-102-userRoleMethods-ArgumentValidation.R:121:5
 2. │ └─testthat:::quasi_capture(...)
 3. │   ├─testthat (local) .capture(...)
 4. │   │ └─base::withCallingHandlers(...)
 5. │   └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
 6. ├─redcapAPI::importUserRoles(...)
 7. └─redcapAPI:::importUserRoles.redcapApiConnection(...) at redcapAPI/R/importUserRoles.R:8:3
 8.   └─redcapAPI::prepUserImportData(...) at redcapAPI/R/importUserRoles.R:48:3
 9.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Error (test-102-userRoleMethods-Functionality.R:9:5): User Role Methods Functionality
Error in `devtools::test()`: 2 assertions failed:
 * At least one user is missing an entry for the form(s): record_id
 * At least one user is missing an export entry for the form(s): record_id
Backtrace:
    ▆
 1. ├─redcapAPI::importUserRoles(rcon, data = NewRole) at test-102-userRoleMethods-Functionality.R:9:5
 2. └─redcapAPI:::importUserRoles.redcapApiConnection(rcon, data = NewRole) at redcapAPI/R/importUserRoles.R:8:3
 3.   └─redcapAPI::prepUserImportData(...) at redcapAPI/R/importUserRoles.R:48:3
 4.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Error (test-103-userRoleAssignmentMethods-ArgumentValidation.R:80:5): Return an error if there are duplicate usernames
Error in `devtools::test()`: 2 assertions failed:
 * At least one user is missing an entry for the form(s): record_id
 * At least one user is missing an export entry for the form(s): record_id
Backtrace:
    ▆
 1. ├─redcapAPI::importUserRoles(rcon, NewRole) at test-103-userRoleAssignmentMethods-ArgumentValidation.R:80:5
 2. └─redcapAPI:::importUserRoles.redcapApiConnection(rcon, NewRole) at redcapAPI/R/importUserRoles.R:8:3
 3.   └─redcapAPI::prepUserImportData(...) at redcapAPI/R/importUserRoles.R:48:3
 4.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Error (test-103-userRoleAssignmentMethods-Functionality.R:1:1): (code run outside of `test_that()`)
Error in `devtools::test()`: 2 assertions failed:
 * At least one user is missing an entry for the form(s): record_id
 * At least one user is missing an export entry for the form(s): record_id
Backtrace:
    ▆
 1. ├─redcapAPI::importUsers(rcon, data = data.frame(username = EXPENDABLE_USER)) at test-103-userRoleAssignmentMethods-Functionality.R:1:1
 2. └─redcapAPI:::importUsers.redcapApiConnection(rcon, data = data.frame(username = EXPENDABLE_USER)) at redcapAPI/R/importUsers.R:6:3
 3.   └─redcapAPI::prepUserImportData(data, rcon = rcon, consolidate = consolidate) at redcapAPI/R/importUsers.R:51:3
 4.     └─checkmate::reportAssertions(coll) at redcapAPI/R/prepUserImportData.R:105:3

Error (test-105-dagAssignment-Functionality.R:25:5): Import and Export DAG Assignments
Error in `devtools::test()`: 1 assertions failed:
 * Variable 'data$username': Must be a subset of {'beckca','benjamin_nutter','garbetsp','obregos','tanh3'}, but has additional elements {'bstat_api_user'}.
Backtrace:
    ▆
 1. ├─redcapAPI::importUserDagAssignments(rcon, data = AddDagAssign) at test-105-dagAssignment-Functionality.R:25:5
 2. └─redcapAPI:::importUserDagAssignments.redcapApiConnection(...) at redcapAPI/R/importUserDagAssignments.R:9:3
 3.   └─checkmate::reportAssertions(coll) at redcapAPI/R/importUserDagAssignments.R:51:3

[ FAIL 13 | WARN 0 | SKIP 13 | PASS 1961 ]

@couthcommander
Copy link
Copy Markdown
Contributor Author

The failed tests are helpful and the code is working as intended. This is a design issue, and we'll need to make a determination on if we need different tests, or if we need to change the design.

@obregos
Copy link
Copy Markdown
Collaborator

obregos commented Nov 21, 2025

All tests pass
[ FAIL 0 | WARN 0 | SKIP 12 | PASS 2047 ]

@spgarbet
Copy link
Copy Markdown
Member

@jubilee2 This seems ready to me. Does it work for you?

Copy link
Copy Markdown
Collaborator

@jubilee2 jubilee2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning this up and addressing #474.

The change to drop the custom form-validation helpers and let REDCap handle missing instruments (with “No Access”) makes sense, and the new test for implicit permission removal is helpful.

@spgarbet spgarbet merged commit 1e36533 into main Nov 24, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants