Skip to content

feat(replication): option to exclude reviewer names from the exported correction log - #347

Open
iabaako wants to merge 3 commits into
mainfrom
feat/332-exclude-reviewer-names
Open

iabaako wants to merge 3 commits into
mainfrom
feat/332-exclude-reviewer-names

Conversation

@iabaako

@iabaako iabaako commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Pull Request Summary 🚀

What does this PR do? 📝

Adds an Exclude reviewer names option to the Export Replication Package page.

  • Page: a checkbox, off by default, sits under the Page Name selector. A caption explains that correction_log.csv records who made each correction.
  • build_replication_package: a new exclude_reviewer_names argument. When set, the user column is blanked in the exported 4_output/3_logs/correction_log.csv only.

Closes #332

Why is this change needed? 🤔

The user column is new in 1.2.0 (#322). It holds the sidebar Reviewer name, or the OS login, which is often the person's full name on Windows. Without this option, reviewers' names go into every replication package, including ones shared outside the team. Promoted to a v1.2.0 blocker (#339) because this release is the one that starts exporting names.

How was this implemented? 🛠️

  • _blank_reviewer_names sets user to null and keeps the column, so the export still matches CORRECTION_LOG_SCHEMA. The stored log is never written.
  • The option is off by default, so the audit trail is kept unless the user opts out.
  • Changing the checkbox discards an already-built zip, so a package built with names can't be downloaded after ticking the box. The session key changed from _replication_page_sel to _replication_build_settings.
  • User Guide: the option is documented under "Correction Audit Trail", next to the User field. The guide has no dedicated replication-package section.

Merge danger: two-way door. Blast radius: narrow, limited to the replication export. The default output is unchanged.

How to test or reproduce ? 🧪

Before: no option; correction_log.csv always contained user.
After: TestExcludeReviewerNames in tests/replication/test_package_builder.py, plus view tests:

  • Default or off: user values are exported.
  • On: the user column is present with all values blank, and other columns are unchanged.
  • An empty log keeps the full header.
  • On: the stored log is unchanged, with no DuckDB writes.
  • View: the checkbox value reaches the builder, and changing it clears a package that was already built.

just test: 3915 passed, 6 skipped.

Manual check: make a correction with a Reviewer name set, build the package with and without the box ticked, and open 4_output/3_logs/correction_log.csv.

Screenshots (if applicable) 📷

N/A

Checklist ✅

  • I have run and tested my changes locally
  • I have limit this PR to less than 1000 lines of code change (if not, explain why)
  • I have updated/added tests to cover my changes (if applicable)
  • I have updated/added requirements to cover my changes (if applicable) (N/A)
  • I have run linting and formatting on any code changes (if applicable)
  • I have updated the documentation (README, etc.) accordingly (CHANGELOG, USER_GUIDE)
  • I have reviewed and resolved any merge conflict

Reviewer Emoji Legend

:code: Meaning
😃👍💯 :smiley: :+1: :100: I like this...

...and I want the author to know it! This is a way to highlight positive parts of a code review.
⭐⭐⭐ :star: :star: :star: Important to fix before PR can be approved...

And I am providing reasons why it needs to be addressed as well as suggested improvements.
⭐⭐ :star: :star: Important to fix but non-blocking for PR approval...

And I am providing suggestions where it could be improved either in this PR or later.
⭐ :star: Give this some thought but non-blocking for PR approval...

...and consider this a suggestion, not a requirement.
❓ :question: I have a question.

This should be a fully formed question with sufficient information and context that requires a response.
📝 :memo: This is an explanatory note, fun fact, or relevant commentary that does not require any action.
⛏ :pick: This is a nitpick.

This does not require any changes and is often better left unsaid. This may include stylistic, formatting, or organization suggestions and should likely be prevented/enforced by linting if they really matter
♻️ :recycle: Suggestion for refactoring.

Should include enough context to be actionable and not be considered a nitpick.

🤖 Generated with Claude Code

iabaako and others added 2 commits October 15, 2026 21:55
… correction log

Closes #332

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@iabaako
iabaako requested a review from a team as a code owner October 15, 2026 21:58
@iabaako iabaako added this to the v1.2.0 milestone Oct 15, 2026
@iabaako iabaako self-assigned this Oct 15, 2026
@iabaako iabaako mentioned this pull request Oct 15, 2026
13 of 54 tasks
@iabaako
iabaako requested a balanced review from Copilot October 15, 2026 22:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The narrow implementation matches the requested behavior, preserves stored data, and includes focused regression coverage with no identified blocking issues.

0 open findings

What changed in this PR

Adds an opt-in privacy control to DataSure’s replication exports, addressing #332 for the 1.2.0 release.

Changes:

  • Blanks reviewer names only in the exported correction log, preserving its schema and stored data.
  • Invalidates built packages when export settings change.
  • Adds regression tests and user documentation.
File Description
tests/​views/​test_replication_view.py Tests checkbox forwarding and package invalidation.
tests/​replication/​test_package_builder.py Tests name exclusion, defaults, schema, and stored-log preservation.
src/​datasure/​views/​replication_view.py Adds the checkbox and tracks export settings.
src/​datasure/​replication/​package_builder.py Supports optional reviewer-name blanking during export.
docs/​USER_GUIDE.md Explains the privacy option and unchanged defaults.
CHANGELOG.md Records the new export option.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@iabaako
iabaako requested a review from dmartinezIPA October 15, 2026 22:58
@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
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.

feat(replication): option to exclude reviewer names from the exported correction log

2 participants