feat(billing): Add seat activity and seats service protos - #411
feat(billing): Add seat activity and seats service protos#411volokluev wants to merge 7 commits into
Conversation
- Add seat_activity service endpoints: close_contract, get_original_seat_dates, get_seat_activity_history, get_seat_contract_state, get_seat_state, perform_seat_actions - Add seats service endpoints: assign_seat, check_assign_seats, disable_seat, get_daily_seat_usage, get_daily_seat_usage_by_project, get_seat_status, remove_seat, rollover_seats, update_seat_identifier - Define shared types: SeatStatus, SeatObject, SeatState, SeatActionRecord, ExternalProductSeatActivity, SeatActionItem, OriginalSeatDate, ExternalProductSeatStatus, SeatAssignmentOutcome, ProjectSeatUsage
|
The latest Buf updates on your PR. Results from workflow ci / buf-checks (pull_request).
|
Move SeatStatus into billing/v1/common/v1/seat_status.proto so both seat_activity and seats services share one enum. Remove the duplicate ExternalProductSeatStatus from seats/v1/seats.proto.
Add documentation comments to all seat_activity and seats service proto messages explaining what each endpoint does, its constraints, and any notable behavior (e.g. OVER_QUOTA -> DISABLED mapping, idempotent rollover, high-throughput check-in path).
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 092a501. Configure here.
| uint64 project_id = 3; | ||
| google.protobuf.Timestamp timestamp = 4; | ||
| sentry_protos.billing.v1.common.v1.SeatStatus status = 5; | ||
| uint32 seat_category = 6; |
There was a problem hiding this comment.
Seat category uses raw integer type
Medium Severity
SeatActionRecord.seat_category is a uint32 while every other seat-category field in this change uses the SeatCategory enum, including SeatObject in the same file. Callers get a raw number instead of a typed value, and changing the field later is a breaking generated-code change.
Reviewed by Cursor Bugbot for commit 092a501. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
|
||
| // Input item for PerformSeatActions: pairs a seat with its desired new state. | ||
| message SeatActionItem { | ||
| SeatObject seat_object = 1; |
There was a problem hiding this comment.
Bug: The SeatActionItem message incorrectly uses the SeatState read-model for input, creating ambiguity with a duplicated external_product_identifier and requiring clients to send meaningless server-generated fields.
Severity: MEDIUM
Suggested Fix
Refactor SeatActionItem to not embed the SeatState read-model. Instead, it should accept only the necessary input fields directly, such as the seat_object and the desired current_status. This removes the ambiguity of the duplicate identifier and eliminates the need for clients to send meaningless server-generated timestamp fields.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
proto/sentry_protos/billing/v1/services/seat_activity/v1/seat_activity.proto#L54-L57
Potential issue: The `SeatActionItem` message incorrectly uses `SeatState`, a
read-model, as an input for a write operation. This forces clients to provide
meaningless server-generated values like `read_ts`. More critically, it duplicates the
`external_product_identifier` field, which is also present in the `seat_object` field.
The schema provides no guidance on how to resolve conflicts if these two identifiers
differ, creating ambiguity for the server implementation and potential for subtle bugs.
Did we get this right? 👍 / 👎 to inform future reviews.
1 participant
Summary
Adds proto definitions for two new billing services:
seat_activity service
CloseContract- close a seat contractGetOriginalSeatDates- get original creation dates for seatsGetSeatActivityHistory- get seat activity history for a contractGetSeatContractState- check if a seat contract exists and is activeGetSeatState- get the current state of a seatPerformSeatActions- perform batch seat actionsseats service
AssignSeat- assign a seatCheckAssignSeats- check if seats can be assignedDisableSeat- disable a seatGetDailySeatUsage- get daily seat usageGetDailySeatUsageByProject- get daily seat usage by projectGetSeatStatusForExternalProduct- get seat status for an external productRemoveSeat- remove a seatRolloverSeatsToNewContract- rollover seats to a new contractUpdateSeatIdentifier- update a seat identifierShared types
SeatStatus(consolidated intobilling/v1/common/v1/seat_status.proto— shared by both services)SeatObject,SeatState,SeatActionRecord,ExternalProductSeatActivity,SeatActionItem,OriginalSeatDateSeatAssignmentOutcome,ProjectSeatUsageDependencies