Skip to content

feat(subscription): let admins set expiry, renew or replace on grant, and grant in bulk - #6326

Open
YiKongk wants to merge 2 commits into
QuantumNous:mainfrom
YiKongk:feat/subscription-admin-grant-enhance
Open

feat(subscription): let admins set expiry, renew or replace on grant, and grant in bulk#6326
YiKongk wants to merge 2 commits into
QuantumNous:mainfrom
YiKongk:feat/subscription-admin-grant-enhance

Conversation

@YiKongk

@YiKongk YiKongk commented Jul 19, 2026

Copy link
Copy Markdown

⚠️ 提交说明 / PR Notice

Important

  • 请提供人工撰写的简洁摘要,避免直接粘贴未经整理的 AI 输出。

📝 变更描述 / Description

管理员为用户开通订阅目前仅支持「按套餐默认周期新建一条记录」,无法满足以下常见运营场景:

  1. 自定义到期时间:赠送指定时长或补偿至某一具体日期时,无法指定到期时间;
  2. 续期已有订阅:用户订阅临近到期需要延长时,只能新插一条记录,导致两条 active 记录并存;
  3. 批量发放:活动补偿需面向多个用户时,只能逐个操作。

具体改动

授予模式(modecreate / renew / replace,默认 create;不传该参数时行为与现状完全一致。

  • renew:定位该用户同套餐生效中的订阅,直接 UPDATE 其 end_time 而非新增记录。由于不新增记录,续期不占用 MaxPurchasePerUser 名额;无生效记录时自动回退为 create
  • replace:先将该用户同套餐生效中的记录置为 cancelled,再新建一条。两个必要的配套处理:
    • 限购计数不含状态过滤(既有语义,本 PR 未改动),刚被取消的记录仍会计入。因此 replace 仅在确实取消了记录的前提下跳过本次限购检查——一取消一新建对生效订阅数为净零,不构成新购买;未取消任何记录时(如用户订阅早已过期)限购照常执行,与 create 一致。
    • 新记录创建时用户仍处于套餐的 upgrade_groupPrevUserGroup 快照将取到空值;若不处理,替换产生的订阅到期后用户将永久保留升级分组(downgrade_group 为空、依赖"回退到购买前分组"配置的套餐均受影响)。因此 replace 将被取消记录的 PrevUserGroup 带入新记录,保证到期降组链路完整。

自定义到期时间(end_time:Unix 秒;传 0 或不传时按套餐周期计算。传入正数时直接将到期时间设为该绝对时间戳(可能早于原到期时间,即缩短);传 0 时在 max(现有到期时间, 当前时间) 基础上叠加一个套餐周期,提前续期不损失剩余时长。UI 对两种语义均有明确提示。

关于配额重置计划:calcNextResetTime 在下次重置时间晚于到期时间时返回 0(即关闭重置计划),因此 renew 延长 end_time 后需考虑重置计划的恢复;同时 custom 周期的重置基于基准时间,若在周期中途无条件重算,会造成已排定的重置被跳过、后一次又提前触发。最终实现为:已排定且仍然有效的重置时间保持不变,仅在其已被清零或被新的到期时间置为无效时重算。两种情形均有测试覆盖。

批量授予:新增 POST /api/subscription/admin/bind/batch。每个用户使用独立事务,单个用户失败(限购已满、账号不存在等)不回滚其他用户;返回 success_count / failed_count 及逐用户失败原因,并对入参去重。failed 字段显式初始化为空切片,全部成功时序列化为 [] 而非 null,前端可直接遍历。

日志:成功的授予按 #6325 引入的模式为每个用户写入 manage 日志(subscription.granted op 描述符,前端按查看者语言渲染);操作者审计新增 subscription.admin_grant_batch,单用户授予的审计参数补充 mode

附带修复

GetDBTimestamp() 通过全局 DB 句柄执行查询,在 DB.Transaction 内调用时会额外占用一条连接池连接直至事务结束。订阅相关事务路径改为通过所在事务读取(getDBTimestampFrom(tx))。这同时解决了订阅事务路径无法编写单元测试的问题:测试使用连接数限制为 1 的内存 SQLite,原实现会在等待第二条连接时死锁。

前端

  • web/default:单用户弹窗新增模式选择与 DateTimePicker(复用兑换码抽屉的「快捷时长按钮 + 时间选择器」组合);用户表格批量操作栏新增「批量分配订阅」入口,复用现有 row selection;新增批量结果弹窗,展示成功数与失败明细。到期时间提示文案抽取为共享的 getEndTimeHint,renew 模式留空时明确提示「在当前到期时间基础上顺延一个套餐周期」。
  • web/classic:单用户弹窗新增 RadioGroup 模式选择与 DatePicker。批量入口未实现——classic 的 CardTable 尚无 rowSelection 支持,实现成本高于收益。

两套前端的 i18n 均已补齐全部语言。

说明:本 PR 代码实现为 AI 辅助完成(AI-assisted),提交者已逐项审阅并在本地验证。

🚀 变更类型 / Type of change

  • 🐛 Bug 修复 (Bug fix) - 请关联对应 Issue,避免将设计取舍、理解偏差或预期不一致直接归类为 bug
  • ✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
  • ⚡ 性能优化 / 重构 (Refactor)
  • 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

✅ 提交前检查项 / Checklist

  • 人工确认: 我已亲自整理并撰写此描述,没有直接粘贴未经处理的 AI 输出。
  • 非重复提交: 我已搜索现有的 IssuesPRs,确认不是重复提交。
  • Bug fix 说明: 若此 PR 标记为 Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
  • 变更理解: 我已理解这些更改的工作原理及可能影响。
  • 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
  • 本地验证: 已在本地运行并通过测试或手动验证,维护者可以据此复核结果。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

📸 运行证明 / Proof of Work

后端:

go build ./model/ ./controller/ ./router/   → 通过
go vet  ./model/ ./controller/ ./router/    → 通过
go test ./model/ ./controller/ -count=1     → 全部通过

新增测试:
  TestAdminBindSubscriptionRenewExtendsWithoutConsumingLimit
  TestAdminBindSubscriptionRenewFallsBackToCreateWhenNoActive
  TestAdminBindSubscriptionRenewKeepsCustomResetSchedule
  TestAdminBindSubscriptionReplaceCancelsExisting
  TestAdminBindSubscriptionReplaceSucceedsAtPurchaseLimit
  TestAdminBindSubscriptionReplacePreservesPrevUserGroup(含到期后经 ExpireDueSubscriptions 正确降组)
  TestAdminBindSubscriptionHonoursCustomEndTime
  TestAdminBindSubscriptionBatchReportsPerUserFailures
  TestAdminBindSubscriptionBatchAllSuccessKeepsFailedNonNil

前端:

web/default: bun run typecheck → 通过;bun run build → 通过
web/classic: bun run build     → 通过

(三种模式的操作界面与批量分配结果面板截图见下方评论区补充。)

Summary by CodeRabbit

  • New Features

    • Added subscription grant modes (create/renew/replace) with optional custom expiration times.
    • Added admin batch subscription assignment with per-user success/failure reporting.
    • Updated subscription grant/expiry audit messaging with new localized templates.
    • Added admin UI for grant mode selection and custom expiration (including preset durations).
  • Bug Fixes

    • Improved renewal/replacement behavior for active subscriptions, including correct end-time handling and purchase-limit enforcement.
    • Prevented orphan subscription records for invalid users and validated custom end times.
  • Tests

    • Added database-backed coverage for locking, renew/replace semantics, batch results, and edge cases.

Admin subscription grants left no trace: neither a manage log for the
target user nor an operator audit entry was written, even though the
grant can also upgrade the user's group. The reset handlers in the same
file already do both.

Both new logs store a language-neutral op descriptor (action + params)
with an English fallback content, like the other audit logs, so the
frontend localizes them per viewer instead of freezing one language
into the database.

The MaxPurchasePerUser count in CreateUserSubscriptionFromPlanTx also ran
without a row lock, so two concurrent grants could both read count-1 and
both insert. Locking the user row serializes them; the limit check itself
is unchanged. This covers the order, balance and admin paths at once.

Locking the row also rejects grants for a non-existent user up front
instead of creating an orphan subscription.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 94f4fa9f-f3ce-4a5f-906b-df6e3a0393c1

📥 Commits

Reviewing files that changed from the base of the PR and between 995a5de and fd072dc.

📒 Files selected for processing (28)
  • controller/audit.go
  • controller/subscription.go
  • model/db_time.go
  • model/subscription.go
  • model/subscription_admin_grant_test.go
  • router/api-router.go
  • web/classic/src/components/table/users/modals/UserSubscriptionsModal.jsx
  • web/classic/src/i18n/locales/en.json
  • web/classic/src/i18n/locales/fr.json
  • web/classic/src/i18n/locales/ja.json
  • web/classic/src/i18n/locales/ru.json
  • web/classic/src/i18n/locales/vi.json
  • web/classic/src/i18n/locales/zh-CN.json
  • web/classic/src/i18n/locales/zh-TW.json
  • web/default/src/features/subscriptions/api.ts
  • web/default/src/features/subscriptions/components/dialogs/batch-assign-subscription-dialog.tsx
  • web/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx
  • web/default/src/features/subscriptions/constants.ts
  • web/default/src/features/subscriptions/types.ts
  • web/default/src/features/usage-logs/lib/format.ts
  • web/default/src/features/users/components/data-table-bulk-actions.tsx
  • web/default/src/i18n/locales/en.json
  • web/default/src/i18n/locales/fr.json
  • web/default/src/i18n/locales/ja.json
  • web/default/src/i18n/locales/ru.json
  • web/default/src/i18n/locales/vi.json
  • web/default/src/i18n/locales/zh-TW.json
  • web/default/src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (25)
  • model/db_time.go
  • web/default/src/features/subscriptions/api.ts
  • web/default/src/features/usage-logs/lib/format.ts
  • router/api-router.go
  • web/default/src/features/users/components/data-table-bulk-actions.tsx
  • web/default/src/features/subscriptions/constants.ts
  • controller/audit.go
  • web/classic/src/i18n/locales/zh-TW.json
  • web/classic/src/i18n/locales/ru.json
  • web/classic/src/i18n/locales/fr.json
  • web/classic/src/i18n/locales/ja.json
  • web/classic/src/i18n/locales/vi.json
  • web/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx
  • web/default/src/i18n/locales/fr.json
  • web/default/src/i18n/locales/ja.json
  • web/default/src/features/subscriptions/types.ts
  • web/default/src/i18n/locales/zh.json
  • web/classic/src/i18n/locales/en.json
  • controller/subscription.go
  • web/default/src/features/subscriptions/components/dialogs/batch-assign-subscription-dialog.tsx
  • web/default/src/i18n/locales/en.json
  • model/subscription_admin_grant_test.go
  • web/default/src/i18n/locales/ru.json
  • web/default/src/i18n/locales/vi.json
  • web/default/src/i18n/locales/zh-TW.json

Walkthrough

Subscription administration now supports create, renew, and replace modes, custom expiration times, batch assignment with per-user results, expanded audit logging, transactional user locking, and corresponding controls in both web interfaces with localized text.

Changes

Subscription grant lifecycle

Layer / File(s) Summary
Grant model and transactional behavior
model/subscription.go, model/db_time.go, model/subscription_admin_grant_test.go
Subscription creation and administrative grants support custom end times, create/renew/replace behavior, row locking, batch results, and database-backed validation tests.
Administrative grant API and audit flow
controller/subscription.go, controller/audit.go, router/api-router.go
Controllers accept grant options, expose batch binding, record individual and aggregate audit entries, and register the batch endpoint.
Default web grant controls and batch workflow
web/default/src/features/subscriptions/..., web/default/src/features/users/..., web/default/src/features/usage-logs/...
Typed requests, grant-mode helpers, individual and batch dialogs, bulk actions, API calls, and audit rendering are added.
Classic UI and localized grant text
web/classic/src/..., web/default/src/i18n/locales/*, web/classic/src/i18n/locales/*
The classic modal gains mode and expiration controls, and localized subscription grant and batch-operation strings are added.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Admin
  participant WebDialog
  participant API
  participant GrantModel
  participant Audit
  Admin->>WebDialog: select users, plan, mode, expiration
  WebDialog->>API: submit grant request
  API->>GrantModel: execute grants
  GrantModel-->>API: return successes and failures
  API->>Audit: record grant activity
  API-->>WebDialog: return result summary
Loading

Possibly related PRs

Poem

I’m a rabbit with grants in my hat,
Renew, replace, create—fancy that!
Batch bunnies hop in a row,
With audit trails wherever they go.
Custom end times sparkle bright,
And every user gets logged just right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main subscription-grant enhancements: custom expiry, renew/replace modes, and bulk granting.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Tools execution failed with the following error:

Failed to run tools: Stream initialization permanently failed: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@YiKongk
YiKongk marked this pull request as ready for review July 19, 2026 23:20

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
model/subscription.go (1)

801-863: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Audit log can misreport the effective grant mode for the renew-fallback case.

When mode == SubscriptionGrantRenew but no active subscription exists (Lines 823-828), the function silently performs a plain create instead — but AdminBindSubscription only returns (string, error), with no indication of which branch actually ran. Downstream, recordSubscriptionGrantLogs in controller/subscription.go (Lines 433-453) logs model.NormalizeGrantMode(opts.Mode) — i.e. the requested mode ("renew") — even though the applied behavior was a fresh create that consumed a purchase-limit slot. Given the PR explicitly calls out "audit logs, including grant modes" as a feature, this is a real accuracy gap for auditors reviewing grant history.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model/subscription.go` around lines 801 - 863, The renew fallback in
AdminBindSubscription must expose the effective grant mode so audit logging does
not record renew when a fresh subscription was created. Update
AdminBindSubscription and its caller recordSubscriptionGrantLogs to propagate an
effective create mode for the no-active-subscription path, while preserving
renew for successful renewals and existing behavior for other modes.
🧹 Nitpick comments (6)
model/subscription.go (2)

754-763: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

NormalizeGrantMode silently coerces unrecognized modes to "create".

Any non-empty, unrecognized Mode string (e.g. a client typo like "reneww") is silently treated as create rather than rejected. Since create mode always inserts a new row and consumes a MaxPurchasePerUser slot, this can produce an unintended extra subscription instead of surfacing a clear input error.

🛡️ Proposed fix: reject unknown non-empty modes
-func NormalizeGrantMode(mode string) string {
-	switch strings.TrimSpace(mode) {
-	case SubscriptionGrantRenew:
-		return SubscriptionGrantRenew
-	case SubscriptionGrantReplace:
-		return SubscriptionGrantReplace
-	default:
-		return SubscriptionGrantCreate
-	}
-}
+func NormalizeGrantMode(mode string) (string, error) {
+	switch strings.TrimSpace(mode) {
+	case "", SubscriptionGrantCreate:
+		return SubscriptionGrantCreate, nil
+	case SubscriptionGrantRenew:
+		return SubscriptionGrantRenew, nil
+	case SubscriptionGrantReplace:
+		return SubscriptionGrantReplace, nil
+	default:
+		return "", fmt.Errorf("invalid grant mode: %s", mode)
+	}
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model/subscription.go` around lines 754 - 763, Update NormalizeGrantMode to
reject unknown non-empty, trimmed mode values instead of mapping them to
SubscriptionGrantCreate. Preserve the existing renew and replace results, and
retain create as the default only when the input is empty; propagate the
invalid-mode error through the caller’s existing validation path.

515-522: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Reuse lockedUser.Group here. getUserGroupByIdTx(tx, userId) reads the same users.group value again, so this adds a redundant query while the row lock is held. Using lockedUser.Group removes the extra round-trip and shortens lock time.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model/subscription.go` around lines 515 - 522, Replace the subsequent
getUserGroupByIdTx(tx, userId) lookup with lockedUser.Group in the
purchase-limit flow after the lockForUpdate query. Reuse the already locked User
record while preserving the existing group-dependent behavior and error
handling.
model/subscription_admin_grant_test.go (1)

309-338: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing test for batch input deduplication.

The PR explicitly calls out deduplicated batch input as a feature, but no test exercises AdminBindSubscriptionBatch with a userIds slice containing repeats (e.g. []int{101, 101, 102}) to confirm it's only granted/counted once.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model/subscription_admin_grant_test.go` around lines 309 - 338, Add coverage
to TestAdminBindSubscriptionBatchReportsPerUserFailures or a focused batch test
that calls AdminBindSubscriptionBatch with duplicate user IDs, such as 101, 101,
and 102. Assert each unique user is processed and counted only once, including
success/failure totals and user ID lists, confirming duplicate input does not
create repeated grants or results.
controller/subscription.go (1)

358-380: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate grant-and-respond flow across two handlers.

AdminBindSubscription (Lines 358-380) and AdminCreateUserSubscription (Lines 492-519) repeat the same validate → build AdminGrantOptionsmodel.AdminBindSubscriptionrecordSubscriptionGrantLogs → respond sequence, differing only in how userId/planId are sourced. Since there are two call sites for this exact sequence, extracting it into a shared helper aligns with the guideline to use separate functions for reusable behavior.

♻️ Proposed extraction
+func grantSubscriptionAndRespond(c *gin.Context, userId int, planId int, mode string, endTime int64) {
+	opts := model.AdminGrantOptions{Mode: mode, EndTime: endTime}
+	msg, err := model.AdminBindSubscription(userId, planId, opts)
+	if err != nil {
+		common.ApiError(c, err)
+		return
+	}
+	recordSubscriptionGrantLogs(c, userId, planId, opts)
+	if msg != "" {
+		common.ApiSuccess(c, gin.H{"message": msg})
+		return
+	}
+	common.ApiSuccess(c, nil)
+}
As per coding guidelines, "Use separate functions for reusable behavior, required interface/framework callbacks, exported APIs, test fixtures, or complex business logic deserving direct tests."

Also applies to: 492-519

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controller/subscription.go` around lines 358 - 380, Extract the shared
grant-and-response sequence from AdminBindSubscription and
AdminCreateUserSubscription into a helper that accepts userId, planId, and
AdminGrantOptions, invokes model.AdminBindSubscription, records grant logs, and
writes the appropriate success or error response. Update both handlers to retain
only their distinct request validation and ID sourcing while delegating this
reusable flow to the helper.

Source: Coding guidelines

web/default/src/features/subscriptions/constants.ts (1)

65-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Tighten mode parameter typing to SubscriptionGrantMode.

getGrantModeOptions, getGrantModeDescription, and getEndTimeHint type mode as string. Since SubscriptionGrantMode already exists in types.ts and all call sites pass a SubscriptionGrantMode value, using the union type here would give compile-time safety against typos/invalid modes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/default/src/features/subscriptions/constants.ts` around lines 65 - 86,
Update getGrantModeDescription and getEndTimeHint to type their mode parameters
as SubscriptionGrantMode instead of string, importing the existing type from
types.ts. Leave getGrantModeOptions unchanged because it does not accept a mode
parameter.
web/default/src/features/subscriptions/components/dialogs/batch-assign-subscription-dialog.tsx (1)

209-255: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Custom-expiration-time UI block is duplicated with user-subscriptions-dialog.tsx.

The DateTimePicker + preset-button block (mode select, description, expiration controls, hint) is duplicated near-verbatim between this file and user-subscriptions-dialog.tsx. Extracting a shared <GrantExpirationFields mode setMode endTime setEndTime /> component/hook would reduce duplication and, notably, would have prevented the "Plan default" bug above from existing in two places independently.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@web/default/src/features/subscriptions/components/dialogs/batch-assign-subscription-dialog.tsx`
around lines 209 - 255, Extract the duplicated custom-expiration UI and related
state/handlers from the batch assignment dialog and user subscriptions dialog
into a shared GrantExpirationFields component or hook. Reuse it in both callers
with mode, setMode, endTime, and setEndTime inputs, preserving the existing
DateTimePicker, preset options, descriptions, and hint behavior while ensuring
the Plan default fix is implemented in one shared location.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@web/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx`:
- Around line 330-376: Update the “Plan default” button in the user
subscriptions dialog to clear endTime rather than assigning the truthy Date
returned by addTimeToDate(0, 0, 0). Preserve the existing handleCreate behavior
so a cleared endTime sends end_time as 0 and uses the plan’s default duration.

In `@web/default/src/features/usage-logs/lib/format.ts`:
- Around line 387-392: Update the subscription.admin_grant_batch template in the
audit text definitions to include the provided {{mode}} placeholder, matching
the single-grant subscription.admin_grant message while preserving the existing
plan and count details.

In `@web/default/src/i18n/locales/ja.json`:
- Line 238: Update the Japanese translation for “Admin granted you subscription
plan {{plan_title}} (ID: {{plan_id}})” to explicitly include `あなたに` after
`管理者が`, preserving the existing placeholders and remaining wording.
- Line 3346: Update the “Plan default” translation in the Japanese locale to a
natural label, using either 「プランの既定値」 or 「プランのデフォルト」 instead of 「プラン既定」.

---

Outside diff comments:
In `@model/subscription.go`:
- Around line 801-863: The renew fallback in AdminBindSubscription must expose
the effective grant mode so audit logging does not record renew when a fresh
subscription was created. Update AdminBindSubscription and its caller
recordSubscriptionGrantLogs to propagate an effective create mode for the
no-active-subscription path, while preserving renew for successful renewals and
existing behavior for other modes.

---

Nitpick comments:
In `@controller/subscription.go`:
- Around line 358-380: Extract the shared grant-and-response sequence from
AdminBindSubscription and AdminCreateUserSubscription into a helper that accepts
userId, planId, and AdminGrantOptions, invokes model.AdminBindSubscription,
records grant logs, and writes the appropriate success or error response. Update
both handlers to retain only their distinct request validation and ID sourcing
while delegating this reusable flow to the helper.

In `@model/subscription_admin_grant_test.go`:
- Around line 309-338: Add coverage to
TestAdminBindSubscriptionBatchReportsPerUserFailures or a focused batch test
that calls AdminBindSubscriptionBatch with duplicate user IDs, such as 101, 101,
and 102. Assert each unique user is processed and counted only once, including
success/failure totals and user ID lists, confirming duplicate input does not
create repeated grants or results.

In `@model/subscription.go`:
- Around line 754-763: Update NormalizeGrantMode to reject unknown non-empty,
trimmed mode values instead of mapping them to SubscriptionGrantCreate. Preserve
the existing renew and replace results, and retain create as the default only
when the input is empty; propagate the invalid-mode error through the caller’s
existing validation path.
- Around line 515-522: Replace the subsequent getUserGroupByIdTx(tx, userId)
lookup with lockedUser.Group in the purchase-limit flow after the lockForUpdate
query. Reuse the already locked User record while preserving the existing
group-dependent behavior and error handling.

In
`@web/default/src/features/subscriptions/components/dialogs/batch-assign-subscription-dialog.tsx`:
- Around line 209-255: Extract the duplicated custom-expiration UI and related
state/handlers from the batch assignment dialog and user subscriptions dialog
into a shared GrantExpirationFields component or hook. Reuse it in both callers
with mode, setMode, endTime, and setEndTime inputs, preserving the existing
DateTimePicker, preset options, descriptions, and hint behavior while ensuring
the Plan default fix is implemented in one shared location.

In `@web/default/src/features/subscriptions/constants.ts`:
- Around line 65-86: Update getGrantModeDescription and getEndTimeHint to type
their mode parameters as SubscriptionGrantMode instead of string, importing the
existing type from types.ts. Leave getGrantModeOptions unchanged because it does
not accept a mode parameter.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: afd511a8-4d2e-4ee9-9e8f-7f9e29b8078c

📥 Commits

Reviewing files that changed from the base of the PR and between 5a6c53d and cbdaedb.

📒 Files selected for processing (28)
  • controller/audit.go
  • controller/subscription.go
  • model/db_time.go
  • model/subscription.go
  • model/subscription_admin_grant_test.go
  • router/api-router.go
  • web/classic/src/components/table/users/modals/UserSubscriptionsModal.jsx
  • web/classic/src/i18n/locales/en.json
  • web/classic/src/i18n/locales/fr.json
  • web/classic/src/i18n/locales/ja.json
  • web/classic/src/i18n/locales/ru.json
  • web/classic/src/i18n/locales/vi.json
  • web/classic/src/i18n/locales/zh-CN.json
  • web/classic/src/i18n/locales/zh-TW.json
  • web/default/src/features/subscriptions/api.ts
  • web/default/src/features/subscriptions/components/dialogs/batch-assign-subscription-dialog.tsx
  • web/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx
  • web/default/src/features/subscriptions/constants.ts
  • web/default/src/features/subscriptions/types.ts
  • web/default/src/features/usage-logs/lib/format.ts
  • web/default/src/features/users/components/data-table-bulk-actions.tsx
  • web/default/src/i18n/locales/en.json
  • web/default/src/i18n/locales/fr.json
  • web/default/src/i18n/locales/ja.json
  • web/default/src/i18n/locales/ru.json
  • web/default/src/i18n/locales/vi.json
  • web/default/src/i18n/locales/zh-TW.json
  • web/default/src/i18n/locales/zh.json

Comment thread web/default/src/features/usage-logs/lib/format.ts
Comment thread web/default/src/i18n/locales/ja.json Outdated
Comment thread web/default/src/i18n/locales/ja.json Outdated
@YiKongk
YiKongk force-pushed the feat/subscription-admin-grant-enhance branch from cbdaedb to 94ef36b Compare July 19, 2026 23:33

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/classic/src/i18n/locales/ja.json`:
- Line 1940: Update the Japanese subscription mode labels in the locale entries
for 「替换现有」 and 「延长现有」 to 「既存のサブスクリプションを置き換える」 and 「既存のサブスクリプションを延長する」
respectively, including the corresponding entry referenced elsewhere in the
file.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 308320bf-72c4-47fb-9455-61434e4216ba

📥 Commits

Reviewing files that changed from the base of the PR and between cbdaedb and 94ef36b.

📒 Files selected for processing (28)
  • controller/audit.go
  • controller/subscription.go
  • model/db_time.go
  • model/subscription.go
  • model/subscription_admin_grant_test.go
  • router/api-router.go
  • web/classic/src/components/table/users/modals/UserSubscriptionsModal.jsx
  • web/classic/src/i18n/locales/en.json
  • web/classic/src/i18n/locales/fr.json
  • web/classic/src/i18n/locales/ja.json
  • web/classic/src/i18n/locales/ru.json
  • web/classic/src/i18n/locales/vi.json
  • web/classic/src/i18n/locales/zh-CN.json
  • web/classic/src/i18n/locales/zh-TW.json
  • web/default/src/features/subscriptions/api.ts
  • web/default/src/features/subscriptions/components/dialogs/batch-assign-subscription-dialog.tsx
  • web/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx
  • web/default/src/features/subscriptions/constants.ts
  • web/default/src/features/subscriptions/types.ts
  • web/default/src/features/usage-logs/lib/format.ts
  • web/default/src/features/users/components/data-table-bulk-actions.tsx
  • web/default/src/i18n/locales/en.json
  • web/default/src/i18n/locales/fr.json
  • web/default/src/i18n/locales/ja.json
  • web/default/src/i18n/locales/ru.json
  • web/default/src/i18n/locales/vi.json
  • web/default/src/i18n/locales/zh-TW.json
  • web/default/src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (26)
  • router/api-router.go
  • web/default/src/features/subscriptions/api.ts
  • web/classic/src/i18n/locales/fr.json
  • web/default/src/features/usage-logs/lib/format.ts
  • web/default/src/features/subscriptions/constants.ts
  • controller/audit.go
  • web/default/src/features/users/components/data-table-bulk-actions.tsx
  • web/classic/src/i18n/locales/zh-TW.json
  • web/classic/src/i18n/locales/zh-CN.json
  • web/classic/src/i18n/locales/vi.json
  • model/db_time.go
  • web/classic/src/components/table/users/modals/UserSubscriptionsModal.jsx
  • web/classic/src/i18n/locales/en.json
  • model/subscription_admin_grant_test.go
  • web/classic/src/i18n/locales/ru.json
  • web/default/src/features/subscriptions/components/dialogs/batch-assign-subscription-dialog.tsx
  • web/default/src/features/subscriptions/types.ts
  • web/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx
  • model/subscription.go
  • web/default/src/i18n/locales/vi.json
  • web/default/src/i18n/locales/ja.json
  • web/default/src/i18n/locales/zh.json
  • web/default/src/i18n/locales/zh-TW.json
  • web/default/src/i18n/locales/fr.json
  • web/default/src/i18n/locales/ru.json
  • web/default/src/i18n/locales/en.json

Comment thread web/classic/src/i18n/locales/ja.json Outdated
@YiKongk
YiKongk force-pushed the feat/subscription-admin-grant-enhance branch from 94ef36b to 995a5de Compare July 19, 2026 23:46
… and grant in bulk

Admin subscription grants could only create a new record following the
plan's own duration. Three gaps this closes:

- custom expiry: pass an absolute end time instead of the plan duration
- renew/replace: renew extends the existing active subscription in place
  (an UPDATE, so it does not consume another MaxPurchasePerUser slot) and
  falls back to creating one when there is none; replace cancels the
  existing active rows first, then inserts the new one with the purchase
  limit skipped (the swap is net-zero on active subscriptions) and the
  cancelled rows' PrevUserGroup carried over, so expiry can still revert
  the user to their origin group. Default stays "create", so existing
  behaviour is unchanged.
- batch: POST /subscription/admin/bind/batch grants one plan to several
  users, each in its own transaction, reporting per-user failures instead
  of rolling the whole set back. The failed list serializes as [] rather
  than null so the result dialog can index into it unconditionally.

Renew keeps an already-scheduled future quota reset so mid-cycle renewals
do not shift base-relative (custom-period) schedules; it only recomputes
the schedule when it was switched off or the new end time invalidates it
(calcNextResetTime zeroes the schedule when the next reset would fall past
the end time).

GetDBTimestamp read through the global DB handle, which checks out a
second pooled connection when called inside a transaction. The subscription
paths now read it through their tx, which also makes them testable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@YiKongk
YiKongk force-pushed the feat/subscription-admin-grant-enhance branch from 995a5de to fd072dc Compare July 20, 2026 00:05
@YiKongk

YiKongk commented Jul 20, 2026

Copy link
Copy Markdown
Author

@coderabbitai The remaining findings from the first review pass have been addressed in fd072dc:

  • Effective grant mode in audit logs (outside-diff, model/subscription.go): AdminBindSubscription now returns the mode that actually ran; the renew-with-no-active fallback reports create, and recordSubscriptionGrantLogs records the effective mode. Covered by an assertion in TestAdminBindSubscriptionRenewFallsBackToCreateWhenNoActive.
  • NormalizeGrantMode strictness: unknown non-empty modes are now rejected with an error instead of silently coercing to create; empty input keeps the historical default. Batch validates once up front. Covered by a new negative test.
  • Redundant group query: createUserSubscriptionFromPlanTx reuses lockedUser.Group instead of re-querying via getUserGroupByIdTx.
  • Batch dedup test: TestAdminBindSubscriptionBatchReportsPerUserFailures now feeds duplicated ids ([101, 101, 102, 102, 999]) and asserts unique processing plus a single row for the succeeded user.
  • mode typing in constants.ts: getGrantModeDescription / getEndTimeHint now take SubscriptionGrantMode.

Deferred to a follow-up PR to keep this one focused:

  • Extracting the shared grant-and-respond controller helper.
  • Extracting a shared GrantExpirationFields component for the two dialogs.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@YiKongk Thanks—those updates address the remaining first-pass findings, including the effective-mode audit trail and strict grant-mode validation. The focused scope is reasonable; the shared controller and UI extractions can follow separately if desired.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

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.

1 participant