Skip to content

[refactor] 訂閱查詢與 MCP 欄位組裝去重,壓 update_subscription 的認知複雜度 - #67

Merged
YJack0000 merged 1 commit into
mainfrom
fix/subscription-contract-sonar
Sep 7, 2026
Merged

YJack0000 merged 1 commit into
mainfrom
fix/subscription-contract-sonar

Conversation

@YJack0000

Copy link
Copy Markdown
Contributor

#64 的後續修正。#64 已經 merge 進 main,但它的 Sonar quality gate 沒過,兩個條件都是 #64 自己新增的程式碼造成的,所以這裡單獨修掉。

1. typescript:S3776 CRITICAL — update_subscription 認知複雜度 17(上限 15)

src/lib/mcp/tools-client.tsupdate_subscription.execute 裡疊了十幾個 if (... !== undefined) patch.x = ...,加上 contractId 的 unlink 分支之後超過門檻。

  • 抽出純函式 subscriptionPatch(args):判斷「有沒有帶這個欄位」的邏輯收在內部的 put() 裡只寫一次,欄位變成一行一個宣告式呼叫。execute 本體只剩兩個 if
  • contractId 的行為完全不變:明確傳 null 仍然是解除綁定,完全沒帶這個 key 仍然是不動它(optNumber 會把兩者都收斂成 undefined,所以 null 要先自己攔一次)。

2. new_duplicated_lines_density 3.81%(上限 3%)

src/db/queries.tslistSubscriptions / getSubscription / listBillingBoard 這三條訂閱讀取路徑,在 #64 補上 contractId / contractTitleleftJoin(contracts) 之後,變成三份幾乎一模一樣的 select + join(jscpd 抓到 11 行與 7 行兩組 clone)。收成一個 subscriptionRows(db) builder,三邊共用,往後加欄位只要改一個地方。

順帶把看板那支原本獨有的 customerTaxId 併進共用欄位,三條路徑形狀一致。getSubscriptionSchedule 是自己組回傳物件的,多出來的欄位不會外流到任何 MCP output schema。

src/lib/mcp/tools-client.tscreate_/update_ × subscription/contract 四支各抄一份「有帶才驗歸屬」的外鍵檢查。抽成 assertClientRefs(db, orgId, {customerPartyId, projectId, contractId}),跨租戶檢查也因此只剩一個攔截點。另外把 SUBSCRIPTION_ROW_PROPScontractId 移出與 CONTRACT_ROW_PROPS 重疊的前綴(純欄位順序調整,required 仍由 Object.keys 推導,schema 語意不變)。

jscpd 在同一組檔案上從 22 clones / 193 duplicated lines 降到 17 clones / 153 lines,消掉的 5 組正好都在 #64 的新程式碼裡。

驗證

@YJack0000
YJack0000 requested a review from yui0303 September 7, 2026 16:30
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

✅ SonarQube Quality Gate passed — pathorsAI_internal

0 open issues on this PR.

@YJack0000
YJack0000 enabled auto-merge (squash) September 7, 2026 16:35

@yui0303 yui0303 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved per request.

@YJack0000
YJack0000 merged commit ad212cf into main Sep 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants