Add ticket code support for cards with auto-incrementing numbers#175
Add ticket code support for cards with auto-incrementing numbers#175mucan54 wants to merge 2 commits intomattermost:mainfrom
Conversation
Each board can now have a card prefix (e.g. "PROJ", "BUG") and cards
get auto-incrementing numbers, producing ticket codes like PROJ-1,
PROJ-2, etc. This enables git branch naming, commit references, and
quick card identification.
Backend changes:
- DB migration 000041: adds card_prefix, card_count to boards table
- Board model: new CardPrefix and CardCount fields with patch support
- Card model: new CardNumber field stored in block fields, computed
TicketCode field, and ParseTicketCode helper
- Store: atomic IncrementBoardCardCount and GetCardByTicketCode methods
- App: auto-assigns ticket numbers on card creation, populates
ticket codes on card retrieval
- API: new GET /teams/{teamID}/cards/by-ticket-code/{ticketCode}
endpoint to look up cards by ticket code
Frontend changes:
- Board/Card TypeScript types updated with new fields
- Kanban cards show ticket code badge above title
- Table rows show ticket code inline before title
- Card detail shows clickable ticket code (copies to clipboard)
- Board view title has new "Ticket prefix" setting with live preview
- Mutator method for changing board card prefix with undo support
https://claude.ai/code/session_017e9AHfd7FTjFDHSTaEZYvW
- Client-side: searchFilterCards now matches card ticket codes (e.g. typing "PROJ-42" in view header search finds the card) - Server-side: board search (both searchBoardsForUser and searchBoardsForUserInTeam) now includes card_prefix in OR conditions, so searching "PROJ" finds boards with that prefix - New BoardSearchFieldCardPrefix for dedicated prefix search - Updated search placeholder to hint at ticket code search https://claude.ai/code/session_017e9AHfd7FTjFDHSTaEZYvW
📝 WalkthroughWalkthroughThis pull request introduces ticket code support for cards, including auto-incrementing card numbers per board, generating prefixed ticket codes (e.g., "PROJ-123"), persisting metadata via database migrations, exposing a new API endpoint to retrieve cards by ticket code, and adding UI components for display and prefix management across views. Changes
Sequence DiagramsequenceDiagram
participant Client
participant API as API Handler
participant Auth as Auth/Permissions
participant App as App Logic
participant Store as Store
participant DB as Database
Client->>API: GET /teams/{teamID}/cards/by-ticket-code/{ticketCode}
API->>Auth: Validate session
Auth-->>API: Session valid
API->>App: GetCardByTicketCode(ticketCode, teamID)
App->>App: Parse ticket code (prefix, cardNumber)
App->>Store: GetCardByTicketCode(prefix, cardNumber, teamID)
Store->>DB: Query block by board_prefix & card_number
DB-->>Store: Return block record
Store-->>App: Block found
App->>Auth: Check view permission on card's board
Auth-->>App: Permission granted
App-->>API: Card object with TicketCode populated
API->>API: Audit log access
API->>API: Marshal card to JSON
API-->>Client: HTTP 200 with card JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.4.0)webapp/src/components/cardDetail/cardDetail.scssConfigurationError: Could not find "stylelint-config-sass-guidelines". Do you need to install the package or use the "configBasedir" option? webapp/src/components/table/tableRow.scssConfigurationError: Could not find "stylelint-config-sass-guidelines". Do you need to install the package or use the "configBasedir" option? webapp/src/components/kanban/kanbanCard.scssConfigurationError: Could not find "stylelint-config-sass-guidelines". Do you need to install the package or use the "configBasedir" option?
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
webapp/src/components/kanban/kanbanCard.scss (1)
59-71: Extract the ticket badge styles into a shared SCSS primitive.The same
.octo-ticket-coderule now exists here and inwebapp/src/components/table/tableRow.scss(Lines 72-79). Pulling this into a shared mixin/partial avoids the two views drifting the next time badge styling changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/src/components/kanban/kanbanCard.scss` around lines 59 - 71, Extract the duplicated .octo-ticket-code styles into a shared SCSS primitive (e.g., a partial _ticket-badge.scss exporting a mixin or %placeholder), then replace the inline rules in both kanbanCard.scss and tableRow.scss with an `@import` and application of that mixin/%placeholder; ensure the shared primitive contains all properties (font-size, font-weight, font-family, color, background, border-radius, padding, margin-bottom, cursor, user-select) so both .octo-ticket-code usages simply include the shared primitive to keep styling consistent.webapp/src/components/table/tableRow.tsx (1)
172-176: Prefer a shared ticket-code formatter instead of composing the string inline.The same
${prefix}-${number}formatting now exists in several UI paths. Routing this through a helper will keep rendering, search, and copy behavior aligned if the format changes again.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/src/components/table/tableRow.tsx` around lines 172 - 176, Create a shared formatter (e.g., formatTicketCode(prefix: string | undefined, number: string | undefined): string | null) and replace the inline string composition in tableRow.tsx (where props.board.cardPrefix and card.fields.cardNumber are used) with a call to that helper; ensure the helper returns null when prefix or number is missing and is exported from a common utils module so other UI paths (rendering, search, copy) can import and use the same formatting logic.webapp/src/components/cardDetail/cardDetail.tsx (1)
224-235: Consider adding keyboard accessibility for the clickable ticket code.The ticket code element is clickable but lacks keyboard accessibility. Users navigating with keyboards won't be able to interact with this element.
♿ Proposed fix for keyboard accessibility
{props.board.cardPrefix && card.fields.cardNumber ? ( <div className='ticket-code' title={intl.formatMessage({id: 'CardDetail.ticket-code-copy', defaultMessage: 'Click to copy ticket code'})} + role='button' + tabIndex={0} onClick={() => { const code = `${props.board.cardPrefix}-${card.fields.cardNumber}` Utils.copyTextToClipboard(code) }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + const code = `${props.board.cardPrefix}-${card.fields.cardNumber}` + Utils.copyTextToClipboard(code) + } + }} > {`${props.board.cardPrefix}-${card.fields.cardNumber}`} </div> ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/src/components/cardDetail/cardDetail.tsx` around lines 224 - 235, The ticket code currently rendered as a clickable div (class 'ticket-code') is not keyboard accessible; update the rendering in cardDetail.tsx so this element can be activated via keyboard by either replacing the div with a semantic interactive element (e.g., a button) or adding role="button", tabIndex={0} and an onKeyDown handler that calls Utils.copyTextToClipboard(code) when Enter or Space is pressed, keep the existing onClick handler, preserve the title/intl.formatMessage text and ensure the aria-label (or title) clearly describes the action for screen reader users; reference props.board.cardPrefix, card.fields.cardNumber, and Utils.copyTextToClipboard when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/api/cards.go`:
- Around line 430-438: The handler currently calls
a.app.GetCardByTicketCode(ticketCode, teamID) then checks
a.permissions.HasPermissionToBoard(userID, card.BoardID,
model.PermissionViewBoard), which leaks existence; either (A) fold permission
into the lookup by adding/using a.app.GetCardByTicketCodeWithAccess(ticketCode,
teamID, userID) (or extend GetCardByTicketCode to accept userID) so the DB query
enforces board view permission and returns not-found when unauthorized, or (B)
if changing the app layer is undesirable, change the post-lookup branch to mask
forbidden access by calling a.errorResponse(w, r, model.NewErrNotFound("card not
found")) (or the handler’s 404-equivalent) instead of returning a permission
error when a.permissions.HasPermissionToBoard(...) is false; update calls to
a.errorResponse and tests accordingly.
In `@server/model/board.go`:
- Around line 313-315: When assigning p.CardPrefix to board.CardPrefix (and the
other occurrence at the same pattern later), trim and uppercase the incoming
string and validate it with the same uppercase-alphanumeric ≤10 rule used by
Board.IsValid() (or extract that prefix check into a shared helper and call it
here). Specifically: if p.CardPrefix != nil, let v =
strings.TrimSpace(*p.CardPrefix); if v != "" set v = strings.ToUpper(v) and run
the Board.IsValid()/shared IsValidCardPrefix helper to ensure only uppercase
A-Z0-9 and max length 10; if validation fails return the appropriate error
instead of persisting the raw value, otherwise set board.CardPrefix = v. Also
apply the identical change at the other assignment block referenced (lines
~397-399).
In `@server/services/store/sqlstore/board.go`:
- Around line 335-336: undeleteBoard() currently inserts into boards and
boards_history without the new ticket-code fields, so restored boards lose
card_prefix and card_count; update the INSERT statements in undeleteBoard() to
include "card_prefix" and "card_count" columns and bind board.CardPrefix and
board.CardCount as their values, and make the same change for the other
insert/update that writes to boards/boards_history later in the file (the second
occurrence mentioned) so both restore paths persist the ticket-code fields.
- Around line 1210-1230: The current incrementBoardCardCount does an UPDATE then
a separate SELECT, which allows race conditions; change it to perform a single
atomic operation that both increments and returns the new value (or alternately
lock the row for the duration). Specifically, in incrementBoardCardCount replace
the two-step Update+Select with one statement that uses the database RETURNING
clause (e.g., UPDATE ... SET card_count = COALESCE(card_count,0) + 1 RETURNING
COALESCE(card_count,0)) and scan the returned value from that single
Exec/QueryRow call (or, if RETURNING is unavailable, perform a SELECT ... FOR
UPDATE inside a transaction locking the board row and then update+read before
committing). Ensure you update the error handling to reflect the single
statement and keep references to incrementBoardCardCount and the board id
parameter.
- Around line 1235-1272: The lookup in getCardByTicketCode assumes (teamID,
card_prefix, cardNumber) is unique but card_prefix isn't enforced unique per
team, so change the implementation to either require a board identity or fail on
ambiguity: update the getCardByTicketCode signature to accept an additional
boardID (or change callers to pass one), then add a Where(sq.Eq{"bo.id":
boardID}) clause to the query (symbols: getCardByTicketCode, boardPrefix,
cardNumber, teamID, s.getQueryBuilder, s.tablePrefix + "boards AS bo"), or if
you cannot change callers instead detect when blocksFromRows returns more than
one matching block and return a clear conflict error instead of returning the
first match (use a distinct error rather than model.NewErrNotFound) so ambiguous
prefixes are surfaced.
In `@server/services/store/store.go`:
- Around line 86-89: The IncrementBoardCardCount method currently performs
UPDATE then SELECT without a transaction; annotate it with `@withTransaction` and
move the UPDATE and subsequent SELECT into the same transactional callback so
the read sees the incremented value atomically; follow the existing pattern used
by InsertBoardWithAdmin and PatchBoard (use the same withTransaction
wrapper/signature, accept/return the same tx context type, execute the UPDATE
and then SELECT inside that tx, and return the new count and error from within
the transaction).
In `@webapp/src/blocks/card.ts`:
- Around line 15-18: The Card type declares ticketCode but the
card-normalization helper doesn't copy payload.ticketCode into the normalized
object, so responses with ticketCode lose it; update the normalization helper
(the function that maps incoming card payloads to the Card shape) to set
ticketCode: payload.ticketCode (or equivalent) on the returned Card when
present, and apply the same fix for the second normalization occurrence
referenced around lines 34-45 so both mapping sites preserve ticketCode
alongside fields/cardNumber.
In `@webapp/src/components/kanban/kanbanCard.scss`:
- Around line 59-71: The .octo-ticket-code badge is currently allowed to wrap
inside the KanbanCard’s aggressive wrapping; update the .octo-ticket-code rule
to keep ticket codes atomic by adding white-space: nowrap; (and optionally
text-overflow: ellipsis and overflow: hidden if you want clipped UI) so values
like PREFIX-123 never break across lines; locate .octo-ticket-code in
kanbanCard.scss and add the properties to that selector (reference:
.octo-ticket-code and the KanbanCard component).
In `@webapp/src/components/viewTitle.tsx`:
- Around line 131-137: The Editable's onChange currently only uppercases input
(setCardPrefix) so invalid characters and length issues remain until
onCardPrefixSave runs; update the onChange handler in the Editable component to
perform the same normalization/validation used in onCardPrefixSave (e.g.,
strip/replace invalid chars, enforce max length) before calling setCardPrefix or
alternatively trigger and show immediate validation state/error tied to
cardPrefix so users see invalid input in real time; locate the Editable usage
and modify its onChange to call the same normalization function (or validation
function) used by onCardPrefixSave.
---
Nitpick comments:
In `@webapp/src/components/cardDetail/cardDetail.tsx`:
- Around line 224-235: The ticket code currently rendered as a clickable div
(class 'ticket-code') is not keyboard accessible; update the rendering in
cardDetail.tsx so this element can be activated via keyboard by either replacing
the div with a semantic interactive element (e.g., a button) or adding
role="button", tabIndex={0} and an onKeyDown handler that calls
Utils.copyTextToClipboard(code) when Enter or Space is pressed, keep the
existing onClick handler, preserve the title/intl.formatMessage text and ensure
the aria-label (or title) clearly describes the action for screen reader users;
reference props.board.cardPrefix, card.fields.cardNumber, and
Utils.copyTextToClipboard when making the change.
In `@webapp/src/components/kanban/kanbanCard.scss`:
- Around line 59-71: Extract the duplicated .octo-ticket-code styles into a
shared SCSS primitive (e.g., a partial _ticket-badge.scss exporting a mixin or
%placeholder), then replace the inline rules in both kanbanCard.scss and
tableRow.scss with an `@import` and application of that mixin/%placeholder; ensure
the shared primitive contains all properties (font-size, font-weight,
font-family, color, background, border-radius, padding, margin-bottom, cursor,
user-select) so both .octo-ticket-code usages simply include the shared
primitive to keep styling consistent.
In `@webapp/src/components/table/tableRow.tsx`:
- Around line 172-176: Create a shared formatter (e.g., formatTicketCode(prefix:
string | undefined, number: string | undefined): string | null) and replace the
inline string composition in tableRow.tsx (where props.board.cardPrefix and
card.fields.cardNumber are used) with a call to that helper; ensure the helper
returns null when prefix or number is missing and is exported from a common
utils module so other UI paths (rendering, search, copy) can import and use the
same formatting logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9a16d939-c10d-402a-9101-398a3431f5da
📒 Files selected for processing (23)
server/api/cards.goserver/app/cards.goserver/model/board.goserver/model/card.goserver/services/store/mockstore/mockstore.goserver/services/store/sqlstore/board.goserver/services/store/sqlstore/migrations/000041_add_ticket_codes.down.sqlserver/services/store/sqlstore/migrations/000041_add_ticket_codes.up.sqlserver/services/store/sqlstore/public_methods.goserver/services/store/store.gowebapp/src/blocks/board.tswebapp/src/blocks/card.tswebapp/src/components/cardDetail/cardDetail.scsswebapp/src/components/cardDetail/cardDetail.tsxwebapp/src/components/kanban/kanbanCard.scsswebapp/src/components/kanban/kanbanCard.tsxwebapp/src/components/table/tableRow.scsswebapp/src/components/table/tableRow.tsxwebapp/src/components/viewHeader/viewHeaderSearch.tsxwebapp/src/components/viewTitle.scsswebapp/src/components/viewTitle.tsxwebapp/src/mutator.tswebapp/src/store/cards.ts
| card, err := a.app.GetCardByTicketCode(ticketCode, teamID) | ||
| if err != nil { | ||
| a.errorResponse(w, r, err) | ||
| return | ||
| } | ||
|
|
||
| if !a.permissions.HasPermissionToBoard(userID, card.BoardID, model.PermissionViewBoard) { | ||
| a.errorResponse(w, r, model.NewErrPermission("access denied to fetch card")) | ||
| return |
There was a problem hiding this comment.
Mask unauthorized ticket-code hits as not found.
This lookup happens before the board permission check, and the handler returns a different result for “exists but forbidden” vs “doesn’t exist”. Because ticket codes are predictable, that lets callers enumerate cards on private boards. Prefer folding access control into the query or returning a 404-equivalent when PermissionViewBoard fails.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/api/cards.go` around lines 430 - 438, The handler currently calls
a.app.GetCardByTicketCode(ticketCode, teamID) then checks
a.permissions.HasPermissionToBoard(userID, card.BoardID,
model.PermissionViewBoard), which leaks existence; either (A) fold permission
into the lookup by adding/using a.app.GetCardByTicketCodeWithAccess(ticketCode,
teamID, userID) (or extend GetCardByTicketCode to accept userID) so the DB query
enforces board view permission and returns not-found when unauthorized, or (B)
if changing the app layer is undesirable, change the post-lookup branch to mask
forbidden access by calling a.errorResponse(w, r, model.NewErrNotFound("card not
found")) (or the handler’s 404-equivalent) instead of returning a permission
error when a.permissions.HasPermissionToBoard(...) is false; update calls to
a.errorResponse and tests accordingly.
| if p.CardPrefix != nil { | ||
| board.CardPrefix = *p.CardPrefix | ||
| } |
There was a problem hiding this comment.
Harden cardPrefix validation on the server.
This only checks length and then persists the raw value unchanged. API callers can still save lowercase, whitespace, or hyphenated prefixes, which makes the new lookup path unreliable because ParseTicketCode() uppercases the prefix and splits on -. Please enforce the same uppercase-alphanumeric max-10 contract here when the value is non-empty, and reuse that check from Board.IsValid() so create/import paths are covered too.
Also applies to: 397-399
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/model/board.go` around lines 313 - 315, When assigning p.CardPrefix to
board.CardPrefix (and the other occurrence at the same pattern later), trim and
uppercase the incoming string and validate it with the same
uppercase-alphanumeric ≤10 rule used by Board.IsValid() (or extract that prefix
check into a shared helper and call it here). Specifically: if p.CardPrefix !=
nil, let v = strings.TrimSpace(*p.CardPrefix); if v != "" set v =
strings.ToUpper(v) and run the Board.IsValid()/shared IsValidCardPrefix helper
to ensure only uppercase A-Z0-9 and max length 10; if validation fails return
the appropriate error instead of persisting the raw value, otherwise set
board.CardPrefix = v. Also apply the identical change at the other assignment
block referenced (lines ~397-399).
| "card_prefix": board.CardPrefix, | ||
| "card_count": board.CardCount, |
There was a problem hiding this comment.
Restore flow still drops the new ticket-code fields.
These fields are now persisted on normal writes and delete history, but undeleteBoard() in this same file still inserts boards/boards_history without card_prefix and card_count. Restoring a deleted board will therefore lose its prefix and reset the counter, which can reissue duplicate ticket numbers on the next card creation.
Also applies to: 435-436
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/services/store/sqlstore/board.go` around lines 335 - 336,
undeleteBoard() currently inserts into boards and boards_history without the new
ticket-code fields, so restored boards lose card_prefix and card_count; update
the INSERT statements in undeleteBoard() to include "card_prefix" and
"card_count" columns and bind board.CardPrefix and board.CardCount as their
values, and make the same change for the other insert/update that writes to
boards/boards_history later in the file (the second occurrence mentioned) so
both restore paths persist the ticket-code fields.
| func (s *SQLStore) incrementBoardCardCount(db sq.BaseRunner, boardID string) (int64, error) { | ||
| query := s.getQueryBuilder(db). | ||
| Update(s.tablePrefix+"boards"). | ||
| Set("card_count", sq.Expr("COALESCE(card_count, 0) + 1")). | ||
| Where(sq.Eq{"id": boardID}) | ||
|
|
||
| if _, err := query.Exec(); err != nil { | ||
| return 0, fmt.Errorf("incrementBoardCardCount error for board %s: %w", boardID, err) | ||
| } | ||
|
|
||
| // Read back the new count | ||
| row := s.getQueryBuilder(db). | ||
| Select("COALESCE(card_count, 0)"). | ||
| From(s.tablePrefix + "boards"). | ||
| Where(sq.Eq{"id": boardID}). | ||
| QueryRow() | ||
|
|
||
| var count int64 | ||
| if err := row.Scan(&count); err != nil { | ||
| return 0, fmt.Errorf("incrementBoardCardCount read error for board %s: %w", boardID, err) | ||
| } |
There was a problem hiding this comment.
Make the increment-and-read a single atomic operation.
This does UPDATE and then a separate SELECT. Under concurrent card creation, two callers can both observe the later value and assign the same cardNumber. Use one statement that both increments and returns the new count, or lock the board row for the duration of the read.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/services/store/sqlstore/board.go` around lines 1210 - 1230, The
current incrementBoardCardCount does an UPDATE then a separate SELECT, which
allows race conditions; change it to perform a single atomic operation that both
increments and returns the new value (or alternately lock the row for the
duration). Specifically, in incrementBoardCardCount replace the two-step
Update+Select with one statement that uses the database RETURNING clause (e.g.,
UPDATE ... SET card_count = COALESCE(card_count,0) + 1 RETURNING
COALESCE(card_count,0)) and scan the returned value from that single
Exec/QueryRow call (or, if RETURNING is unavailable, perform a SELECT ... FOR
UPDATE inside a transaction locking the board row and then update+read before
committing). Ensure you update the error handling to reflect the single
statement and keep references to incrementBoardCardCount and the board id
parameter.
| func (s *SQLStore) getCardByTicketCode(db sq.BaseRunner, boardPrefix string, cardNumber int64, teamID string) (*model.Block, error) { | ||
| query := s.getQueryBuilder(db). | ||
| Select(s.blockFields("b")...). | ||
| From(s.tablePrefix + "blocks AS b"). | ||
| Join(s.tablePrefix + "boards AS bo ON b.board_id = bo.id"). | ||
| Where(sq.Eq{"bo.card_prefix": boardPrefix}). | ||
| Where(sq.Eq{"bo.team_id": teamID}). | ||
| Where(sq.Eq{"b.type": model.TypeCard}). | ||
| Where(sq.Eq{"b.delete_at": 0}) | ||
|
|
||
| rows, err := query.Query() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("getCardByTicketCode error: %w", err) | ||
| } | ||
| defer s.CloseRows(rows) | ||
|
|
||
| blocks, err := s.blocksFromRows(rows) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // Find the block whose fields.cardNumber matches | ||
| for _, block := range blocks { | ||
| if cn, ok := block.Fields["cardNumber"]; ok { | ||
| var num int64 | ||
| switch v := cn.(type) { | ||
| case float64: | ||
| num = int64(v) | ||
| case int64: | ||
| num = v | ||
| } | ||
| if num == cardNumber { | ||
| return block, nil | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil, model.NewErrNotFound("card with ticket code") |
There was a problem hiding this comment.
Team-scoped ticket lookup is ambiguous without prefix uniqueness.
This query assumes (teamID, card_prefix, cardNumber) identifies exactly one card, but nothing in the write path enforces card_prefix uniqueness per team. If two boards in the same team both use PROJ, PROJ-1 can match multiple cards here and this method will return whichever row happens to come back first.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/services/store/sqlstore/board.go` around lines 1235 - 1272, The lookup
in getCardByTicketCode assumes (teamID, card_prefix, cardNumber) is unique but
card_prefix isn't enforced unique per team, so change the implementation to
either require a board identity or fail on ambiguity: update the
getCardByTicketCode signature to accept an additional boardID (or change callers
to pass one), then add a Where(sq.Eq{"bo.id": boardID}) clause to the query
(symbols: getCardByTicketCode, boardPrefix, cardNumber, teamID,
s.getQueryBuilder, s.tablePrefix + "boards AS bo"), or if you cannot change
callers instead detect when blocksFromRows returns more than one matching block
and return a clear conflict error instead of returning the first match (use a
distinct error rather than model.NewErrNotFound) so ambiguous prefixes are
surfaced.
| // IncrementBoardCardCount atomically increments the card_count for a board and returns the new count. | ||
| IncrementBoardCardCount(boardID string) (int64, error) | ||
| // GetCardByTicketCode returns a card block matching the given board prefix and card number. | ||
| GetCardByTicketCode(boardPrefix string, cardNumber int64, teamID string) (*model.Block, error) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if IncrementBoardCardCount implementation uses a transaction
ast-grep --pattern $'func ($_ *SQLStore) incrementBoardCardCount($_, $_) ($_, $_) {
$$$
}'Repository: mattermost/mattermost-plugin-boards
Length of output: 1894
🏁 Script executed:
#!/bin/bash
# Search for calls to incrementBoardCardCount to understand the context
rg "incrementBoardCardCount" --type go -B2 -A2Repository: mattermost/mattermost-plugin-boards
Length of output: 1513
🏁 Script executed:
#!/bin/bash
# Check for transaction wrapping patterns in board.go
rg "BeginTx|Tx\(" server/services/store/sqlstore/board.go --type go | head -20Repository: mattermost/mattermost-plugin-boards
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Look for `@withTransaction` annotations in store.go to understand the pattern
rg "@withTransaction|withTx" server/services/store/ --type go | head -20Repository: mattermost/mattermost-plugin-boards
Length of output: 1181
🏁 Script executed:
#!/bin/bash
# Check the store.go interface around IncrementBoardCardCount with more context
sed -n '80,95p' server/services/store/store.goRepository: mattermost/mattermost-plugin-boards
Length of output: 1046
🏁 Script executed:
#!/bin/bash
# Check a few other Store interface methods to understand the `@withTransaction` pattern
rg "IncrementBoardCardCount|InsertBoardWithAdmin|PatchBoard" server/services/store/store.go -B1 -A1Repository: mattermost/mattermost-plugin-boards
Length of output: 762
Add @withTransaction annotation to ensure IncrementBoardCardCount is atomic.
The method claims to "atomically increment" in its doc comment, but the implementation (server/services/store/sqlstore/board.go, lines 1210–1233) performs an UPDATE followed by a separate SELECT without a transaction. Concurrent card creations can cause the SELECT to read a count that was modified between the UPDATE and the read, violating atomicity.
The codebase establishes a @withTransaction pattern for transaction-requiring operations—InsertBoardWithAdmin and PatchBoard use it; add it here as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/services/store/store.go` around lines 86 - 89, The
IncrementBoardCardCount method currently performs UPDATE then SELECT without a
transaction; annotate it with `@withTransaction` and move the UPDATE and
subsequent SELECT into the same transactional callback so the read sees the
incremented value atomically; follow the existing pattern used by
InsertBoardWithAdmin and PatchBoard (use the same withTransaction
wrapper/signature, accept/return the same tx context type, execute the UPDATE
and then SELECT inside that tx, and return the new count and error from within
the transaction).
| type Card = Block & { | ||
| fields: CardFields | ||
| cardNumber?: number | ||
| ticketCode?: string |
There was a problem hiding this comment.
Preserve ticketCode when normalizing cards.
This helper adds ticketCode to the Card type but never copies it from the incoming payload, so server responses that already include ticketCode get normalized without it.
Also applies to: 34-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@webapp/src/blocks/card.ts` around lines 15 - 18, The Card type declares
ticketCode but the card-normalization helper doesn't copy payload.ticketCode
into the normalized object, so responses with ticketCode lose it; update the
normalization helper (the function that maps incoming card payloads to the Card
shape) to set ticketCode: payload.ticketCode (or equivalent) on the returned
Card when present, and apply the same fix for the second normalization
occurrence referenced around lines 34-45 so both mapping sites preserve
ticketCode alongside fields/cardNumber.
| .octo-ticket-code { | ||
| display: inline-block; | ||
| font-size: 11px; | ||
| font-weight: 600; | ||
| font-family: monospace; | ||
| color: rgba(var(--center-channel-color-rgb), 0.56); | ||
| background: rgba(var(--center-channel-color-rgb), 0.08); | ||
| border-radius: 3px; | ||
| padding: 1px 6px; | ||
| margin-bottom: 4px; | ||
| cursor: pointer; | ||
| user-select: all; | ||
| } |
There was a problem hiding this comment.
Keep ticket codes on one line.
KanbanCard enables aggressive wrapping at Line 7, so this badge can split PREFIX-123 across lines in narrow columns. Ticket codes are easier to scan/copy if they stay atomic.
💡 Suggested fix
.octo-ticket-code {
display: inline-block;
font-size: 11px;
font-weight: 600;
font-family: monospace;
color: rgba(var(--center-channel-color-rgb), 0.56);
background: rgba(var(--center-channel-color-rgb), 0.08);
border-radius: 3px;
padding: 1px 6px;
margin-bottom: 4px;
cursor: pointer;
user-select: all;
+ white-space: nowrap;
+ overflow-wrap: normal;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .octo-ticket-code { | |
| display: inline-block; | |
| font-size: 11px; | |
| font-weight: 600; | |
| font-family: monospace; | |
| color: rgba(var(--center-channel-color-rgb), 0.56); | |
| background: rgba(var(--center-channel-color-rgb), 0.08); | |
| border-radius: 3px; | |
| padding: 1px 6px; | |
| margin-bottom: 4px; | |
| cursor: pointer; | |
| user-select: all; | |
| } | |
| .octo-ticket-code { | |
| display: inline-block; | |
| font-size: 11px; | |
| font-weight: 600; | |
| font-family: monospace; | |
| color: rgba(var(--center-channel-color-rgb), 0.56); | |
| background: rgba(var(--center-channel-color-rgb), 0.08); | |
| border-radius: 3px; | |
| padding: 1px 6px; | |
| margin-bottom: 4px; | |
| cursor: pointer; | |
| user-select: all; | |
| white-space: nowrap; | |
| overflow-wrap: normal; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@webapp/src/components/kanban/kanbanCard.scss` around lines 59 - 71, The
.octo-ticket-code badge is currently allowed to wrap inside the KanbanCard’s
aggressive wrapping; update the .octo-ticket-code rule to keep ticket codes
atomic by adding white-space: nowrap; (and optionally text-overflow: ellipsis
and overflow: hidden if you want clipped UI) so values like PREFIX-123 never
break across lines; locate .octo-ticket-code in kanbanCard.scss and add the
properties to that selector (reference: .octo-ticket-code and the KanbanCard
component).
| <Editable | ||
| className='card-prefix-input' | ||
| value={cardPrefix} | ||
| placeholderText={intl.formatMessage({id: 'ViewTitle.ticket-prefix-placeholder', defaultMessage: 'e.g. PROJ'})} | ||
| onChange={(newPrefix) => setCardPrefix(newPrefix.toUpperCase())} | ||
| saveOnEsc={true} | ||
| onSave={onCardPrefixSave} |
There was a problem hiding this comment.
This is not real-time validation yet.
onChange only uppercases the value, so invalid characters and overlong prefixes remain visible until save rewrites them. If the intent is real-time validation, apply the same normalization here or surface a validation error immediately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@webapp/src/components/viewTitle.tsx` around lines 131 - 137, The Editable's
onChange currently only uppercases input (setCardPrefix) so invalid characters
and length issues remain until onCardPrefixSave runs; update the onChange
handler in the Editable component to perform the same normalization/validation
used in onCardPrefixSave (e.g., strip/replace invalid chars, enforce max length)
before calling setCardPrefix or alternatively trigger and show immediate
validation state/error tied to cardPrefix so users see invalid input in real
time; locate the Editable usage and modify its onChange to call the same
normalization function (or validation function) used by onCardPrefixSave.
2 participants
Summary
This PR adds ticket code functionality to cards, allowing boards to assign auto-incrementing ticket numbers with a customizable prefix (e.g., "PROJ-1", "PROJ-2"). Users can now search for cards by their ticket codes and view them throughout the UI.
Key Changes
Backend:
card_prefixandcard_countcolumns toboardsandboards_historytables via migrationIncrementBoardCardCount()to atomically increment board card counters when creating cardsGetCardByTicketCode()to retrieve cards by their ticket code (prefix + number)GET /teams/{teamID}/cards/by-ticket-code/{ticketCode}to fetch cards by ticket codecard_prefixin addition to title, allowing users to find boards by ticket prefixParseTicketCode()utility to parse ticket codes in "PREFIX-NUMBER" formatCardmodel to includeCardNumberandTicketCodefieldsBoardmodel to includeCardPrefixandCardCountfields with validation (max 10 chars)CardPrefixsupport toBoardPatchfor updating board ticket prefixesFrontend:
changeBoardCardPrefix()mutator method for updating board prefixesImplementation Details
board.cardPrefixandcard.cardNumberrather than stored separatelycard_countcounterhttps://claude.ai/code/session_017e9AHfd7FTjFDHSTaEZYvW
Change Impact: 🔴 High
Reasoning: This PR introduces a critical race condition in ticket code auto-increment logic AND breaks the existing test suite. The
TestCreateCardtest uses strict gomock and explicitly mocksGetBoard,InsertBlock, andGetMembersForBoard, but the new code calls the unmockedIncrementBoardCardCountmethod. Additionally, the implementation uses an unsafe UPDATE+SELECT pattern without transaction protection, guaranteeing duplicate ticket numbers under concurrent load and corrupting board data.Regression Risk:
High and critical. The PR has two blocking issues: (1) Test suite breakage (confirmed)—
SetupTestHelpercreates a strict gomock controller (gomock.NewController(t)) which enforces explicit expectations for all method calls. TheTestCreateCardtest setsEXPECT()forGetBoard,InsertBlock, andGetMembersForBoard, but does NOT set an expectation forIncrementBoardCardCount. WhenCreateCardis executed (line 26 in cards.go), it callsa.store.IncrementBoardCardCount(boardID), which is an unmocked method on a strict mock. This causes the test to fail with a gomock "unexpected call" error. The PR was not validated against the existing test suite. (2) Race condition in increment logic (confirmed)—incrementBoardCardCount(lines 1210-1233 in board.go) uses non-atomic UPDATE+SELECT: the UPDATE (line 1216) and SELECT (line 1225) are separate unprotected statements. Concurrent threads can interleave between them, both receiving the same counter value and assigning identical ticket numbers to different cards. Under production load, duplicate ticket numbers are virtually guaranteed, corrupting board data.QA Recommendation:
Critical fixes required before merge. (1) Test fix (blocking)—add
th.Store.EXPECT().IncrementBoardCardCount(gomock.Any()).Return(1, nil)to theTestCreateCardsuccess scenario; add test cases for error handling when increment fails; rungo test ./app -run TestCreateCard -vto verify test passes. (2) Full test suite run (must-do)—executego test ./app -vto ensure all existing tests pass with the new code. (3) Concurrency test (mandatory)—write an integration test spawning 50+ concurrentCreateCardrequests to the same board; verify allCardNumbervalues are unique, strictly sequential with no gaps or duplicates; run multiple times to confirm. (4) Code fix (blocking)—replace unsafe UPDATE+SELECT inincrementBoardCardCountwith atomic operations: PostgreSQLRETURNING, MySQLLAST_INSERT_ID(), or explicitSELECT ... FOR UPDATEwithin a transaction. Do not merge or deploy—PR breaks existing tests and introduces data corruption under normal production load.