refactor(statmodel): Set up a constants file for all namespace IDs

[tonywu1999]

Summary by CodeRabbit

• Refactor

  • Centralized and standardized identifiers for comparison controls and panels to improve consistency; behavior and user-facing UI remain unchanged.

• Tests

  • Updated test suite to use the new centralized identifiers and added a guard for invalid comparison mode inputs.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[github-actions]

Failed to generate code suggestions for PR

[coderabbitai]

Walkthrough

Adds two centralized constant maps (NAMESPACE_STATMODEL, CONSTANTS_STATMODEL) and updates the statmodel UI, server wiring, styles, and tests to use these constants instead of hard-coded IDs/values; server branching and reactive handlers are rewired to reference the new namespaced identifiers.

Changes

Cohort / File(s)	Summary
Constants Definition R/constants.R	Adds NAMESPACE_STATMODEL (namespaced UI/output/input IDs) and CONSTANTS_STATMODEL (comparison-mode keys).
Server Module Logic R/module-statmodel-server.R	Replaces literal ID checks and switch dispatch with constant-driven if/else branches; rewires all input/output references and reactive/observer hooks to use NAMESPACE_STATMODEL and CONSTANTS_STATMODEL.
UI Headers & Styling R/statmodel-ui-headers.R	create_custom_styles() updated to generate CSS selectors dynamically using NAMESPACE_STATMODEL constants.
UI Option Builders R/statmodel-ui-options-contrasts.R	All UI outputs, radio values, and actionButton IDs replaced with NAMESPACE_STATMODEL / CONSTANTS_STATMODEL constants across contrast panels.
Server Tests tests/testthat/test-module-statmodel-server.R	Test input setting refactored to assemble a named list and call do.call(session$setInputs, ...), using namespaced constants for mode and fields.
UI Module Tests tests/testthat/test-module-statmodel-ui.R	Replaces hard-coded element IDs/values with NAMESPACE_STATMODEL / CONSTANTS_STATMODEL constants in assertions.
Utility & Options Tests tests/testthat/test-statmodel-ui-options-contrasts.R, tests/testthat/test-utils-statmodel-server.R	Tests updated to call get_contrast_panel_ui() with CONSTANTS_STATMODEL values and to assert presence of UI elements using NAMESPACE_STATMODEL; adds null-check for invalid mode.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI as StatModel UI (ns)
    participant Server as StatModel Server
    participant Builder as Contrast Builder

    User->>UI: select comparison mode (radio)
    UI->>Server: input[[NAMESPACE_STATMODEL$comparison_mode]]
    Server->>Server: map mode via CONSTANTS_STATMODEL
    Server->>UI: render panel (uiOutput id from NAMESPACE_STATMODEL)
    User->>UI: fill choices & click submit (namespaced submit)
    UI->>Server: input[[NAMESPACE_STATMODEL$..._submit]]
    Server->>Builder: call build_*_contrast() based on CONSTANTS_STATMODEL
    Builder-->>Server: contrast matrix
    Server->>UI: enable calculate / show matrix (namespaced outputs)
    note right of Server: Observers and validators use namespaced inputs\n(submit, clear, mode) for flow control

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Pay special attention to:
  • R/module-statmodel-server.R — verify all input/output keys are correctly mapped and that conditional branches preserve previous behavior.
  • R/constants.R — confirm completeness and naming consistency with calls across files.
  • Tests — ensure test refactors still exercise identical code paths and that the new null-check is correct.

Possibly related PRs

• refactor(statmodel): Put conditional logic in backend for comparison matrices #129 — Refactors contrast-panel backend and UI wiring; this PR centralizes IDs and references those changes.
• refactor(statsmodel): Refactor statsmodel server into sub-functions #119 — Introduced/modified contrast-building helpers referenced now via constants.
• tests(stat-model): Add unit tests to stat model server #118 — Related refactor of server/module scoping and wiring that intersects with the namespacing changes here.

Suggested labels

Review effort 3/5

Poem

🐰 I hopped through code, no strings in sight,
Namespaced and tidy, every ID right.
Constants for modes, buttons in line,
Tests now follow one map so fine.
Hooray — a clean, consistent design! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: introducing a constants file to centralize namespace IDs used throughout the statmodel module.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor-statmodel-comparisons

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

R/constants.R (1)

2-25: Centralizing statmodel IDs/modes via constants looks solid

NAMESPACE_STATMODEL and CONSTANTS_STATMODEL are internally consistent with their later usages (UI, server, tests) and provide a clear single source of truth for IDs and mode keys. You might consider, in a follow-up, documenting these in package-level docs if they’re intended as part of the public API, but nothing here blocks this PR.

R/statmodel-ui-headers.R (1)

9-48: Namespaced CSS selectors correctly aligned with new constants

Using '#statmodel-' plus NAMESPACE_STATMODEL$… for the contrast buttons keeps styling tied to the same IDs used in the UI builders, reducing drift risk. If you ever want to trim duplication, these eight tags$style() calls could be generated in a small helper over a vector of the relevant constants, but the current form is perfectly acceptable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8761e39 and 2d2bb9d.

📒 Files selected for processing (8)

• R/constants.R (1 hunks)
• R/module-statmodel-server.R (3 hunks)
• R/statmodel-ui-headers.R (1 hunks)
• R/statmodel-ui-options-contrasts.R (5 hunks)
• tests/testthat/test-module-statmodel-server.R (9 hunks)
• tests/testthat/test-module-statmodel-ui.R (4 hunks)
• tests/testthat/test-statmodel-ui-options-contrasts.R (4 hunks)
• tests/testthat/test-utils-statmodel-server.R (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build

🔇 Additional comments (4)

tests/testthat/test-statmodel-ui-options-contrasts.R (1)

5-9: UI option-panel tests now correctly keyed off shared namespace constants

The tests for all four contrast option panels now assert against NAMESPACE_STATMODEL$comparisons_* IDs, which is consistent with the refactored UI code using the same constants via ns(). This centralization should make future ID changes far less brittle across the suite.

Also applies to: 21-23, 34-35, 47-50

R/statmodel-ui-options-contrasts.R (1)

8-15: Contrast UI successfully migrated to constants-based IDs and mode keys

create_contrast_section(), create_contrast_radio_buttons(), and all four panel builders now consistently use NAMESPACE_STATMODEL and CONSTANTS_STATMODEL for IDs and mode values. This matches the server’s use of the same constants and the updated tests, so the contrast UI is now cleanly centralized around a single set of identifiers.

Also applies to: 20-33, 40-47, 52-58, 63-67, 72-79

tests/testthat/test-utils-statmodel-server.R (1)

334-371: get_contrast_panel_ui tests correctly track constant-based modes and namespace IDs

The updated tests for get_contrast_panel_ui() now:

• Call the helper with CONSTANTS_STATMODEL$comparison_mode_* rather than string literals, and
• Assert on the presence of NAMESPACE_STATMODEL$comparisons_* IDs in the returned HTML,

plus they cover the new behavior of returning NULL for NULL, empty, or invalid modes. This keeps the helper tightly coupled to the centralized constants and validates the new guard logic.

R/module-statmodel-server.R (1)

22-38: Server-side refactor is correct; shinyjs calls properly target namespaced elements

Verification confirms:

• get_contrast_panel_ui() cleanly dispatches on CONSTANTS_STATMODEL$comparison_mode_* and returns NULL for NULL/empty/invalid modes, matching updated tests.
• All dynamic contrast inputs/outputs (comparisons_*) are rendered and observed via NAMESPACE_STATMODEL keys, in sync with R/statmodel-ui-options-contrasts.R and the test suite.
• Rownames, check_cond, and matrix_build are consistently triggered off the four namespaced submit inputs, and validate_contrast_inputs() centralizes validation logic.
• shinyjs enable("calculate") and disable("calculate") calls correctly target the namespaced button: shinyjs state functions automatically apply module namespacing by default, so plain names work as written without requiring explicit session$ns().

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/testthat/test-module-statmodel-server.R (1)

192-193: Original review comment's concern is valid but overstated; production code is correct.

The inputs ARE properly namespaced in production: selectInput(ns("group1"), ...) and numericInput(ns(paste0("weight", i)), ...) in the renderUI functions correctly apply namespacing via the ns() wrapper. When accessed in module server context (e.g., input$group1), they work correctly.

The test pattern (inputs$group1 <- "Group1") is acceptable when testing helper functions with mock data, not the Shiny module reactivity itself.

Actual inconsistency: Output slots use NAMESPACE_STATMODEL constants (e.g., comparisons_custom_pairwise_choice1) but input IDs themselves ("group1", "group2", "comp_name", "weight1"-"weightN") are hardcoded strings in renderUI calls. For consistency and maintainability, consider adding constants for input IDs similar to the existing output slot constants—this would allow referencing them throughout the codebase (server functions, tests) consistently.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2d2bb9d and 8030fb7.

📒 Files selected for processing (2)

• tests/testthat/test-module-statmodel-server.R (9 hunks)
• tests/testthat/test-module-statmodel-ui.R (4 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/testthat/test-module-statmodel-ui.R

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build

🔇 Additional comments (2)

tests/testthat/test-module-statmodel-server.R (2)

189-196: Past issue resolved: Namespaced submit inputs now correctly configured.

All test cases now properly use the namespaced submit constants (e.g., NAMESPACE_STATMODEL$comparisons_custom_pairwise_submit) instead of the bare submit field. This ensures tests correctly trigger the refactored server event handlers.

Also applies to: 239-243, 281-285, 323-331, 375-380, 413-420, 453-458, 498-503, 506-507, 541-544

---

191-191: ****

The constants NAMESPACE_STATMODEL and CONSTANTS_STATMODEL are properly available in the test environment. These are internal objects defined in R/constants.R and are automatically loaded as part of the package namespace during testing. R packages load all objects from R/ files when the package is initialized, making them available to test code even without explicit @export tags or NAMESPACE entries. This is standard R package structure and requires no changes.

Likely an incorrect or invalid review comment.