Fix unconfigured voting options being skipped at 3 votes

R/mod_voting.R

@@ -725,8 +725,10 @@ votingServer <- function(
                 print(paste("vote_col:", vote_col))
                 max_votes <- vote_max_map[[option_key]]
                 print(paste("max_votes:", max_votes))
+                # Skip limit check if option is not in voting_options_max_matching_votes
                 if (is.null(max_votes) || is.na(max_votes)) {
-                  max_votes <- 3
+                  print(paste("Skipping limit check for option:", option_key))
+                  next
                 }
                 print(paste("max_votes after check:", max_votes))
                 max_votes <- as.numeric(max_votes)

tests/testthat/test-voting-module.R

@@ -818,6 +818,12 @@ testthat::test_that("get_mutation returns done when all screenshots have 3+ vote
     }
   )
 })
+testthat::test_that("options not in voting_options_max_matching_votes are never skipped", {
+  # This test validates the fix for the issue where options not configured
+  # in voting_options_max_matching_votes were being skipped with a default limit of 3
+
+  # Set up environment with 2 coordinates
+  env <- setup_voting_env(c("chr1:1000", "chr2:2000"))
 
 testthat::test_that("observations and comments are cleared when switching vote type from none_of_above to yes", {
   # Set up test environment
@@ -826,6 +832,26 @@ testthat::test_that("observations and comments are cleared when switching vote t
   cleanup_db <- setup_test_db(args)
   on.exit(cleanup_db())
 
+  # Set vote_count_none_of_above to a high value (e.g., 10)
+  # Since 'none_of_above' is NOT in voting_options_max_matching_votes,
+  # it should NOT be skipped regardless of vote count
+  DBI::dbExecute(
+    args$db_pool,
+    "UPDATE annotations SET vote_count_none_of_above = ? WHERE coordinates = ?",
+    params = list(10, "chr1:1000")
+  )
+
+  # Verify the vote count was set correctly
+  chr1_votes <- DBI::dbGetQuery(
+    args$db_pool, 
+    "SELECT vote_count_none_of_above FROM annotations WHERE coordinates = ?",
+    params = list("chr1:1000")
+  )
+  testthat::expect_equal(chr1_votes$vote_count_none_of_above, 10)
+
+  my_session <- MockShinySession$new()
+  my_session$clientData <- shiny::reactiveValues(
+    url_search = ""
   my_session <- MockShinySession$new()
   my_session$clientData <- shiny::reactiveValues(
     url_search = "?coordinate=chr1:1000"
@@ -841,11 +867,46 @@ testthat::test_that("observations and comments are cleared when switching vote t
     )
   )
 
+  # Verify that 'none_of_above' is NOT in voting_options_max_matching_votes
+  testthat::expect_false(
+    "none_of_above" %in% names(args$cfg$voting_options_max_matching_votes)
+  )
+
   testServer(
     votingServer,
     session = my_session,
     args = args,
     {
+      session$userData$userAnnotationsFile <- env$annotations_file
+      session$userData$votingInstitute <- cfg$test_institute
+      session$userData$shinyauthr_session_id <- "test_no_limit_session"
+
+      # Trigger the mutation loading
+      session$flushReact()
+
+      # get_mutation should return chr1:1000, NOT skip it
+      # even though vote_count_none_of_above is 10
+      res <- get_mutation()
+      testthat::expect_equal(res$coordinates, "chr1:1000")
+
+      # Verify the annotations file does NOT have a skip reason for chr1:1000
+      annotations <- read.delim(
+        env$annotations_file,
+        stringsAsFactors = FALSE
+      )
+      chr1_row <- annotations[annotations$coordinates == "chr1:1000", ]
+      # The agreement should be NA (not voted yet), not a skip reason
+      testthat::expect_true(is.na(chr1_row$agreement))
+    }
+  )
+})
+
+testthat::test_that("options in voting_options_max_matching_votes ARE skipped when limit reached", {
+  # This test confirms that options explicitly configured in 
+  # voting_options_max_matching_votes still get skipped correctly
+
+  # Set up environment with 2 coordinates
+  env <- setup_voting_env(c("chr1:1000", "chr2:2000"))
       # Set up session userData needed by the observer
       session$userData$userAnnotationsFile <- env$annotations_file
       session$userData$shinyauthr_session_id <- "session_state_persistence"
@@ -923,11 +984,62 @@ testthat::test_that("comment is preserved for diff_var but cleared for yes", {
     )
   )
 
+  # Get the configured max votes for 'yes' option
+  max_yes_votes <- args$cfg$voting_options_max_matching_votes[["yes"]]
+  testthat::expect_false(is.null(max_yes_votes))
+
+  # Set vote_count_correct to the max limit
+  DBI::dbExecute(
+    args$db_pool,
+    "UPDATE annotations SET vote_count_correct = ? WHERE coordinates = ?",
+    params = list(max_yes_votes, "chr1:1000")
+  )
+
+  # Verify the vote count was set correctly
+  chr1_votes <- DBI::dbGetQuery(
+    args$db_pool, 
+    "SELECT vote_count_correct FROM annotations WHERE coordinates = ?",
+    params = list("chr1:1000")
+  )
+  testthat::expect_equal(chr1_votes$vote_count_correct, max_yes_votes)
+
+  my_session <- MockShinySession$new()
+  my_session$clientData <- shiny::reactiveValues(
+    url_search = ""
+  )
+
   testServer(
     votingServer,
     session = my_session,
     args = args,
     {
+      session$userData$userAnnotationsFile <- env$annotations_file
+      session$userData$votingInstitute <- cfg$test_institute
+      session$userData$shinyauthr_session_id <- "test_skip_configured_session"
+
+      # Trigger the mutation loading
+      session$flushReact()
+
+      # get_mutation should skip chr1:1000 and return chr2:2000
+      res <- get_mutation()
+      testthat::expect_equal(res$coordinates, "chr2:2000")
+
+      # Verify the annotations file has a skip reason for chr1:1000
+      annotations <- read.delim(
+        env$annotations_file,
+        stringsAsFactors = FALSE
+      )
+      chr1_row <- annotations[annotations$coordinates == "chr1:1000", ]
+      # Expected format matches the skip_reason in mod_voting.R lines 755-760
+      expected_skip_reason <- paste0(
+        "skipped - max matching votes (",
+        max_yes_votes,
+        ") for option (yes) reached"
+      )
+      testthat::expect_equal(chr1_row$agreement, expected_skip_reason)
+    }
+  )
+})
       # Set up session userData needed by the observer
       session$userData$userAnnotationsFile <- env$annotations_file
       session$userData$shinyauthr_session_id <- "session_comment_test"
@@ -977,4 +1089,4 @@ testthat::test_that("comment is preserved for diff_var but cleared for yes", {
       testthat::expect_true(is.na(annotations$comment[1]))
     }
   )
-})
+})