S-137651 Include custom text fields and tags to redaction script - #53
hariharan-digitalai wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new script has a confirmed failure mode in custom-field parsing and risky defaults that can lead to unintended deletions without explicit operator configuration.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR replaces the existing one-off story redaction SQL script with a new script that additionally supports redacting configured custom text/long-text fields and removing story tags, while continuing to delete related commit/webhook event records that reference the redacted fields.
Changes:
- Removed the legacy
redact-story-name-and-description.sqlscript. - Added
redact-story-multiple-fields.sqlto redact Name/Description plus custom text/long-text fields and delete story tags. - Extended the commit/webhook event cleanup filters to include tag and custom-field payload references.
File summaries
| File | Description |
|---|---|
| one-off/redact-story-name-and-description.sql | Removed legacy one-off script in favor of a more capable replacement. |
| one-off/redact-story-multiple-fields.sql | New one-off script to redact additional fields (custom text/long-text) and delete tags, plus expanded event cleanup. |
Review details
Suppressed comments (1)
one-off/redact-story-multiple-fields.sql:78
- Same as above for CustomLongText: use
=instead ofINand guard for NULL so the update is a deliberate opt-in.
update dbo.LongString
set Value=@redacted
from dbo.CustomLongText
where LongString.ID=CustomLongText.Value and CustomLongText.ID=@storyId
and CustomLongText.Definition in (@customLongTextDefinition)
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- updated header comment - used = instead in for definition comparison
| if @error<>0 goto ERR | ||
| raiserror('%d Custom Long Text redacted', 0, 1, @rowcount) with nowait | ||
|
|
||
| delete dbo.BaseAssetTaggedWith |
There was a problem hiding this comment.
This is just deleting all tags for a story. They want to update specific tags to be "redacted" similar to how the rest of the code in this file does. Also, I believe they wanted that to be a completely different script so they could run the tags separately.
There was a problem hiding this comment.
Updated script to redact tags instead of deleting them and moved to separate script file - 4e74fe3
|
|
||
| declare @workitemNumber int=NNNNN | ||
| declare @assetType varchar(100)=NULL -- e.g. 'Story' | ||
| declare @tagValue varchar(440)=NULL -- e.g. 'all' |
There was a problem hiding this comment.
This is a confusing comment. Will @tagValue varchar(440)='all' redact all the workitem's tags?
The instructions in the header block give no guidance on setting @tagValue
| update dbo.BaseAssetTaggedWith | ||
| set Value=@replaceWith | ||
| where ID=@workitemId and Value=@tagValue |
There was a problem hiding this comment.
Any given asset must not have the same tag multiple times at any point in history. We are not protecting against this, which can happen if redacted is a real tag, or if the user runs this script multiple times for different @tagValues
| declare @tagValue varchar(440)=NULL -- e.g. 'all' | ||
| declare @replaceWith varchar(440)='redacted' |
|
|
||
| delete dbo.Commits | ||
| where cast(Payload as varchar(max)) like '%Asset":"'+@workitemOid | ||
| and cast(Payload as varchar(max)) like '%"Name":"TaggedWith"%Value":"'+@tagValue+'"%' |
There was a problem hiding this comment.
Doesn't @tagValue need to be JSON-encoded to match?
Updated the story redaction script to: