Skip to content

refactor: use shared exclusion utilities in general_exclusions#1127

Draft
js3110 wants to merge 1 commit into1110-feat/pp-exclusions-in-settingsfrom
1126-refactor/general-exclusions-shared-utils
Draft

refactor: use shared exclusion utilities in general_exclusions#1127
js3110 wants to merge 1 commit into1110-feat/pp-exclusions-in-settingsfrom
1126-refactor/general-exclusions-shared-utils

Conversation

@js3110
Copy link
Copy Markdown
Collaborator

@js3110 js3110 commented Mar 26, 2026

Issue

Closes #1126

Description

Refactors general_exclusions.R to use the shared utilities from exclusion-utils.R (introduced in #1125), replacing duplicated inline logic. Net -23 lines, no behavior change.

Definition of Done

  • general_exclusions.R uses rehydrate_exclusions(), observe_remove_buttons(), and clean_exclusion_list() instead of inline implementations
  • Existing general exclusion functionality (add, remove, persist, restore) works identically

How to test

  1. Upload data, go to General Exclusions, add exclusions with reasons
  2. Verify remove buttons work (click X to remove an exclusion)
  3. Download settings YAML, reload, upload settings — verify general exclusions are restored
  4. Verify no regressions in NCA results when exclusions are active

Contributor checklist

  • Code passes lintr checks
  • Code passes all unit tests
  • New logic covered by unit tests
  • New logic is documented
  • App or package changes are reflected in NEWS
  • Package version is incremented
  • R script works with the new implementation (if applicable)
  • Settings upload works with the new implementation (if applicable)

Notes to reviewer

Replace inline rehydration, remove-button observers, and clean-list
logic with calls to shared functions from exclusion-utils.R.

Closes #1126

Co-authored-by: Ona <no-reply@ona.com>
@js3110 js3110 force-pushed the 1126-refactor/general-exclusions-shared-utils branch from 653fbcc to 24d9dc6 Compare March 26, 2026 10:16
@js3110 js3110 changed the base branch from 1019-feat/parameter-exclusions-anl01fl to 1110-feat/pp-exclusions-in-settings March 26, 2026 10:17
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.

1 participant