Canonicalize owner names in upload payload (LIBTRACK-132)#33
Draft
naarok wants to merge 2 commits into
Draft
Conversation
Captures the proposal, design, spec, and tasks artifacts for the emit-canonical-owner-names change. This change addresses LIBTRACK-132 by canonicalizing owner team names in the upload payload assembled by CheckVersionStatus#server_data so the library_tracking Owners tab no longer shows duplicate rows for variants like :bizops, bizops, and "bizops". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-Authored-By: Amplify 1.6.0 <amplify@getjobber.com>
Add `LibraryVersionAnalysis::Ownership.canonicalize`, a class method that
returns a canonical lowercase owner team name with surrounding
whitespace, quotes, and colons stripped (falling back to "unknown" for
empty/punctuation-only input). The rule mirrors `library_tracking`'s
`Owner.canonicalize_team` byte-for-byte so both sides agree on canonical
form even when versions skew.
Apply the helper at the single payload seam in
`CheckVersionStatus#server_data` where `libraries.push({...})` is
called. `row.owner` (and the in-memory `Versionline.owner` it points to)
is intentionally left untouched so downstream branching such as
`line.owner == :attention_needed` in `#get_mode_summary` keeps working.
The other payload builders (`vulns.push`, `new_versions.push`) do not
include the owner field, so this one edit covers the payload surface.
Add RSpec coverage:
- Unit specs for `Ownership.canonicalize` covering colon stripping,
quote stripping, downcase, the `:attention_needed` symbol, the
empty/`nil`/whitespace/punctuation fallback, and idempotency.
- Payload-level specs on `CheckVersionStatus#server_data` confirming
`:bizops` -> "bizops", `:attention_needed` -> "attention_needed",
and `"bizops"` -> "bizops", plus a regression assertion that
`row.owner` retains its pre-payload value after `#server_data` runs.
All 124 specs pass via `bundle _2.5.22_ exec rspec`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Amplify 1.6.0 <amplify@getjobber.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's in this PR?
Fixes LIBTRACK-132 on the gem side.
library_tracking's Owners tab was showing duplicate rows for the same team because outgoing payloads contained owner names in whatever shape each parser happened to produce —:bizops,bizops,:bizops:,"bizops", and the:attention_neededRuby symbol all flowed through verbatim. This change canonicalizes owner names exactly once, at the single payload seam where upload data is assembled.Changes:
LibraryVersionAnalysis::Ownership.canonicalize(name), a class method that returns a canonical lowercase team name with surrounding whitespace, quotes ("/'), and colons stripped (falling back tounknownfor empty/punctuation-only input). The rule is documented to stay in lockstep withlibrary_tracking'sOwner.canonicalize_teamso both sides agree on canonical form even when versions skew.CheckVersionStatus#server_data, wrapping theownervalue in thelibraries.push({...})call. The in-memoryVersionline.owneris intentionally left untouched so downstream branching such asline.owner == :attention_neededin#get_mode_summarykeeps working.vulns.pushandnew_versions.pushdo not carry owner data, so a single edit covers the payload's owner surface.:attention_neededsymbol, empty/nil/whitespace/punctuation fallback, idempotency) and for the payload seam (:bizops→bizops,:attention_needed→attention_needed,"bizops"→bizops, plus a regression assertion thatrow.ownerretains its pre-payload value).openspec/changes/emit-canonical-owner-names/so the change is captured spec-driven.The companion change in
library_trackingdoes the server-side normalization and the one-time merge ofOwnerrows already in the database.QA Instructions
bundle _2.5.22_ exec rspecpasses — 124 examples, 0 failures locally. (The Gemfile.lock pins bundler 2.5.22; a defaultbundlemay resolve to a newer bundler thatlibyear-bundlerrejects.)openspec validate emit-canonical-owner-names --strictreports the change is valid.lib/library_version_analysis/check_version_status.rbat thelibraries.pushsite and confirm only the outboundownervalue is wrapped —row.owneris not reassigned.[upload]log enabled and confirm owner names in the log come out canonical (lowercase, no:prefix, no quotes).References
library_tracking: coming on the same branch name.Warning
This repository does not have a PR template (
.github/PULL_REQUEST_TEMPLATE.md). Consider adding one to give future PRs a consistent structure.Co-Authored-By: Amplify 1.6.0 amplify@getjobber.com