Repository navigation
feat(tickets)!: remove the deprecated top-level totalMessageCount - #302
Merged
Merged
Conversation
The ticket detail response carried a second message count, deprecated since v2.3.0 in favor of ticket.totalMessageCount. It cost a COUNT query per request and disagreed with ticket.totalMessageCount on threads with hidden messages. GetTicketFirstMessages now returns only the messages. BREAKING CHANGE: GetTicketResponse drops the top-level totalMessageCount. Closes #301
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 8 minutes. View limit details
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. CodeRabbit Verify
|
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.
Closes #301
Summary
The ticket detail response carried a second message count next to
ticket.totalMessageCount. The spec deprecated it with "Removal target: v2.3.0". It goes, and so does theCOUNTquery that only fed it.GetTicketResponse ticket totalMessageCount # replyCount + 1, unchanged firstMessages - totalMessageCount # COUNT of visible message rows writeTicketResponse - msgs, total = GetTicketFirstMessages(...) # COUNT query + page query + msgs = GetTicketFirstMessages(...) # page query onlyThe two counts disagreed whenever a thread had hidden messages. On fixture ticket 1,
ticket.totalMessageCountis 3 and the top-level value was 2, because one reply is a hidden internal note. A caller that needs the visible-only number now counts the messages fromGET /api/v1/tickets/{ticketId}/messages.Evidence
developagainst the seeded harness database:get_by_id_happyandget_by_ref_happygoldens change, and each just loses the top-level field.go vet ./...,go test ./...without the harness,go test -v ./testdb ./datastoreswith the MariaDB harness (0 skips), andgo test -race ./...with the harness all pass.Merge Danger
Door: two-way
Releases are a manual step, so merging changes nothing in production. A revert brings the field and its query back.
Blast Radius: tickets clients
This breaks the wire shape and belongs in the same 4.0.0 release as #296. The tickets MCP is the only client, and it never reads the top-level field.
Reviewed with sol-loop: 1 round with gpt-6.1-sol. Sol verdict: approved
Created with Claude Opus 5.5 in T3 Code, reviewed by gpt-6.1-sol.
Note
🤖 Opus 5.5 responding on behalf of SyniRon