From 5684275c03036ed5d10b8391dcef736db48f080a Mon Sep 17 00:00:00 2001 From: Katy Sadowski Date: Sat, 4 Nov 2023 16:35:31 -0400 Subject: [PATCH 1/2] fix bugs --- R/writeJsonResultsTo.R | 2 +- inst/sql/sql_server/field_fk_class.sql | 6 +++--- inst/sql/sql_server/field_fk_domain.sql | 6 +++--- inst/sql/sql_server/field_plausible_temporal_after.sql | 10 +++++----- tests/testthat/test-writeDBResultsTo.R | 2 +- tests/testthat/test-writeJsonResultsTo.R | 6 +++--- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/R/writeJsonResultsTo.R b/R/writeJsonResultsTo.R index a5a70b20..869539b6 100644 --- a/R/writeJsonResultsTo.R +++ b/R/writeJsonResultsTo.R @@ -47,7 +47,7 @@ writeJsonResultsToTable <- function(connectionDetails, ParallelLogger::logInfo(sprintf("Writing results to table %s", tableName)) - if ("unitConceptId" %in% colnames(df)) { + if ("conceptId" %in% colnames(df)) { ddl <- SqlRender::loadRenderTranslateSql( sqlFilename = "result_table_ddl_concept.sql", packageName = "DataQualityDashboard", diff --git a/inst/sql/sql_server/field_fk_class.sql b/inst/sql/sql_server/field_fk_class.sql index e9d6871e..2505fefc 100755 --- a/inst/sql/sql_server/field_fk_class.sql +++ b/inst/sql/sql_server/field_fk_class.sql @@ -4,7 +4,7 @@ FK_CLASS Drug era standard concepts, ingredients only Parameters used in this template: -cdmDatabaseSchema = @cdmDatabaseSchema +schema = @schema vocabDatabaseSchema = @vocabDatabaseSchema cdmTableName = @cdmTableName cdmFieldName = @cdmFieldName @@ -32,7 +32,7 @@ FROM ( SELECT '@cdmTableName.@cdmFieldName' AS violating_field, cdmTable.* - FROM @cdmDatabaseSchema.@cdmTableName cdmTable + FROM @schema.@cdmTableName cdmTable LEFT JOIN @vocabDatabaseSchema.concept co ON cdmTable.@cdmFieldName = co.concept_id {@cohort & '@runForCohort' == 'Yes'}?{ @@ -48,7 +48,7 @@ FROM ( ( SELECT COUNT_BIG(*) AS num_rows - FROM @cdmDatabaseSchema.@cdmTableName cdmTable + FROM @schema.@cdmTableName cdmTable {@cohort & '@runForCohort' == 'Yes'}?{ JOIN @cohortDatabaseSchema.@cohortTableName c ON cdmTable.person_id = c.subject_id diff --git a/inst/sql/sql_server/field_fk_domain.sql b/inst/sql/sql_server/field_fk_domain.sql index ce596c47..a3e188f8 100755 --- a/inst/sql/sql_server/field_fk_domain.sql +++ b/inst/sql/sql_server/field_fk_domain.sql @@ -5,7 +5,7 @@ FIELD_FK_DOMAIN all standard concept ids are part of specified domain Parameters used in this template: -cdmDatabaseSchema = @cdmDatabaseSchema +schema = @schema vocabDatabaseSchema = @vocabDatabaseSchema cdmTableName = @cdmTableName cdmFieldName = @cdmFieldName @@ -32,7 +32,7 @@ FROM ( SELECT '@cdmTableName.@cdmFieldName' AS violating_field, cdmTable.* - FROM @cdmDatabaseSchema.@cdmTableName cdmTable + FROM @schema.@cdmTableName cdmTable LEFT JOIN @vocabDatabaseSchema.concept co ON cdmTable.@cdmFieldName = co.concept_id {@cohort & '@runForCohort' == 'Yes'}?{ @@ -48,7 +48,7 @@ FROM ( ( SELECT COUNT_BIG(*) AS num_rows - FROM @cdmDatabaseSchema.@cdmTableName cdmTable + FROM @schema.@cdmTableName cdmTable {@cohort & '@runForCohort' == 'Yes'}?{ JOIN @cohortDatabaseSchema.@cohortTableName c ON cdmTable.PERSON_ID = c.SUBJECT_ID diff --git a/inst/sql/sql_server/field_plausible_temporal_after.sql b/inst/sql/sql_server/field_plausible_temporal_after.sql index 8dd28eab..034ce0b8 100755 --- a/inst/sql/sql_server/field_plausible_temporal_after.sql +++ b/inst/sql/sql_server/field_plausible_temporal_after.sql @@ -4,7 +4,7 @@ PLAUSIBLE_TEMPORAL_AFTER get number of records and the proportion to total number of eligible records with datetimes that do not occur on or after their corresponding datetimes Parameters used in this template: -cdmDatabaseSchema = @cdmDatabaseSchema +schema = @schema cdmTableName = @cdmTableName cdmFieldName = @cdmFieldName plausibleTemporalAfterTableName = @plausibleTemporalAfterTableName @@ -33,9 +33,9 @@ FROM SELECT '@cdmTableName.@cdmFieldName' AS violating_field, cdmTable.* - FROM @cdmDatabaseSchema.@cdmTableName cdmTable - {@cdmDatabaseSchema.@cdmTableName != @cdmDatabaseSchema.@plausibleTemporalAfterTableName}?{ - JOIN @cdmDatabaseSchema.@plausibleTemporalAfterTableName plausibleTable ON cdmTable.person_id = plausibleTable.person_id} + FROM @schema.@cdmTableName cdmTable + {@schema.@cdmTableName != @schema.@plausibleTemporalAfterTableName}?{ + JOIN @schema.@plausibleTemporalAfterTableName plausibleTable ON cdmTable.person_id = plausibleTable.person_id} {@cohort & '@runForCohort' == 'Yes'}?{ JOIN @cohortDatabaseSchema.@cohortTableName c ON cdmTable.person_id = c.subject_id AND c.cohort_definition_id = @cohortDefinitionId @@ -55,7 +55,7 @@ FROM ( SELECT COUNT_BIG(*) AS num_rows - FROM @cdmDatabaseSchema.@cdmTableName cdmTable + FROM @schema.@cdmTableName cdmTable {@cohort & '@runForCohort' == 'Yes'}?{ JOIN @cohortDatabaseSchema.@cohortTableName c ON cdmTable.person_id = c.subject_id AND c.cohort_definition_id = @cohortDefinitionId diff --git a/tests/testthat/test-writeDBResultsTo.R b/tests/testthat/test-writeDBResultsTo.R index 312f4607..9cc11808 100644 --- a/tests/testthat/test-writeDBResultsTo.R +++ b/tests/testthat/test-writeDBResultsTo.R @@ -6,7 +6,7 @@ test_that("Write DB results to json", { connectionDetailsEunomia <- Eunomia::getEunomiaConnectionDetails() cdmDatabaseSchemaEunomia <- "main" resultsDatabaseSchemaEunomia <- "main" - writeTableName <- "dqdashboard_results" + writeTableName <- "dqd_db_results" expect_warning( results <- DataQualityDashboard::executeDqChecks( diff --git a/tests/testthat/test-writeJsonResultsTo.R b/tests/testthat/test-writeJsonResultsTo.R index 7aff258f..c9df7f0d 100644 --- a/tests/testthat/test-writeJsonResultsTo.R +++ b/tests/testthat/test-writeJsonResultsTo.R @@ -29,11 +29,11 @@ test_that("Write JSON results", { connectionDetails = connectionDetailsEunomia, resultsDatabaseSchema = resultsDatabaseSchemaEunomia, jsonFilePath = jsonPath, - writeTableName = "dqd_results" + writeTableName = "dqd_json_results" ) connection <- DatabaseConnector::connect(connectionDetailsEunomia) on.exit(DatabaseConnector::disconnect(connection), add = TRUE) tableNames <- DatabaseConnector::getTableNames(connection = connection, databaseSchema = resultsDatabaseSchemaEunomia) - expect_true("dqd_results" %in% tolower(tableNames)) - DatabaseConnector::renderTranslateExecuteSql(connection, "DROP TABLE @database_schema.dqd_results;", database_schema = resultsDatabaseSchemaEunomia) + expect_true("dqd_json_results" %in% tolower(tableNames)) + DatabaseConnector::renderTranslateExecuteSql(connection, "DROP TABLE @database_schema.dqd_json_results;", database_schema = resultsDatabaseSchemaEunomia) }) From 16990b4731c334c3978d1c105f703c93a6053ca0 Mon Sep 17 00:00:00 2001 From: Katy Sadowski Date: Sat, 4 Nov 2023 17:26:48 -0400 Subject: [PATCH 2/2] test for cdm_source warning --- tests/testthat/test-executeDqChecks.R | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/testthat/test-executeDqChecks.R b/tests/testthat/test-executeDqChecks.R index 01cb11b3..35fbff93 100644 --- a/tests/testthat/test-executeDqChecks.R +++ b/tests/testthat/test-executeDqChecks.R @@ -328,3 +328,30 @@ test_that("Incremental insert SQL is valid.", { DatabaseConnector::renderTranslateExecuteSql(connection, "DROP TABLE @database_schema.dqd_results;", database_schema = resultsDatabaseSchemaEunomia) }) + +test_that("Multiple cdm_source rows triggers warning.", { + outputFolder <- tempfile("dqd_") + on.exit(unlink(outputFolder, recursive = TRUE)) + + connectionDetailsEunomiaCS <- Eunomia::getEunomiaConnectionDetails() + connection <- DatabaseConnector::connect(connectionDetailsEunomiaCS) + on.exit(DatabaseConnector::disconnect(connection), add = TRUE) + DatabaseConnector::renderTranslateExecuteSql(connection, "INSERT INTO @database_schema.cdm_source VALUES ('foo',NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL);", database_schema = cdmDatabaseSchemaEunomia) + + w <- capture_warnings( + results <- executeDqChecks( + connectionDetails = connectionDetailsEunomiaCS, + cdmDatabaseSchema = cdmDatabaseSchemaEunomia, + resultsDatabaseSchema = resultsDatabaseSchemaEunomia, + cdmSourceName = "Eunomia", + checkNames = "measurePersonCompleteness", + outputFolder = outputFolder, + writeToTable = F + )) + + expect_match(w, "Missing check names", all = FALSE) + expect_match(w, "The cdm_source table has", all = FALSE) + + expect_true(nrow(results$CheckResults) > 1) +}) +