Enqueue benefit.update when a license key is rotated or updated - #13977
Enqueue benefit.update when a license key is rotated or updated#13977pieterbeulque wants to merge 1 commit into
Conversation
Rotating or updating a license key now enqueues the benefit.update job for the associated benefit grant, so the grant properties are refreshed and a benefit_grant.updated webhook is emitted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LXM55Y42X6hbbPPTKYEJ5g
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
OpenAPI ChangesNo changes detected in the OpenAPI schema. |
![cubic-dev-ai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Confidence score: 3/5
- In
server/polar/license_key/service.py, updating a benefit can reset per-key limits or expiry to the benefit defaults, causing concrete entitlement regressions; preserve key-level overrides or avoid re-syncing the grant strategy. - In
server/polar/license_key/service.py, status-changing updates load the same grant twice, adding an unnecessary database query; return and reuse the grant from_enqueue_grant_lifecycle.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/polar/license_key/service.py">
<violation number="1" location="server/polar/license_key/service.py:187">
P3: Status-changing updates now load the same grant twice: `_enqueue_grant_lifecycle` already calls `_get_grant`, then this block repeats it. Return and reuse the loaded grant to remove the extra database query.</violation>
<violation number="2" location="server/polar/license_key/service.py:189">
P1: When a key has per-key limits or expiry, this job resets them to the benefit defaults because `benefit.update` re-runs the license-key grant strategy. Preserve key-level overrides during this update or avoid re-syncing those fields for key-level changes.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if update_dict: | ||
| grant = await self._get_grant(session, license_key) | ||
| if grant is not None: | ||
| enqueue_job("benefit.update", benefit_grant_id=grant.id) |
There was a problem hiding this comment.
P1: When a key has per-key limits or expiry, this job resets them to the benefit defaults because benefit.update re-runs the license-key grant strategy. Preserve key-level overrides during this update or avoid re-syncing those fields for key-level changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/polar/license_key/service.py, line 189:
<comment>When a key has per-key limits or expiry, this job resets them to the benefit defaults because `benefit.update` re-runs the license-key grant strategy. Preserve key-level overrides during this update or avoid re-syncing those fields for key-level changes.</comment>
<file context>
@@ -181,6 +182,12 @@ async def update(
+ if update_dict:
+ grant = await self._get_grant(session, license_key)
+ if grant is not None:
+ enqueue_job("benefit.update", benefit_grant_id=grant.id)
+
return license_key
</file context>
There was a problem hiding this comment.
that's a good point! For the rest it looks good
| await session.flush() | ||
|
|
||
| if update_dict: | ||
| grant = await self._get_grant(session, license_key) |
There was a problem hiding this comment.
P3: Status-changing updates now load the same grant twice: _enqueue_grant_lifecycle already calls _get_grant, then this block repeats it. Return and reuse the loaded grant to remove the extra database query.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/polar/license_key/service.py, line 187:
<comment>Status-changing updates now load the same grant twice: `_enqueue_grant_lifecycle` already calls `_get_grant`, then this block repeats it. Return and reuse the loaded grant to remove the extra database query.</comment>
<file context>
@@ -181,6 +182,12 @@ async def update(
await session.flush()
+
+ if update_dict:
+ grant = await self._get_grant(session, license_key)
+ if grant is not None:
+ enqueue_job("benefit.update", benefit_grant_id=grant.id)
</file context>
|
Hi, I'm the merchant who reported this issue. Just to confirm, update() not firing a hook has been the case too, but that one doesn't bother me much since it's only triggered from the merchant/admin side and I control my own backend, so I can just avoid calling update() if needed. Rotate is the actual problem because customers can trigger it themselves. It got a lot worse after the checkout rotate button merged last week (#13909). Now a customer can rotate their license right on the final checkout step, seconds after buying, without ever touching the customer portal separately. So a brand new license can get invalidated on our end within seconds of the purchase, and we have no way to know it happened. That's what turned this from a minor gap into something actually breaking real purchases. Thanks for picking it up, looking forward to the fix going out. |
|
Hmm, I don't think we should allow license key rotation directly post-checkout, that's not the best customer experience. That's probably an unintended side effect of using the same component there. cc @petru |
|
That is right, I haven't noticed that component was used in the post checkout flow. Will fix that today 👀 |
4 participants
I'm actually not sure if we want to do this or not. It was surfaced by a merchant report that the webhook didn't trigger on a key rotation, but it looks like it never triggered on a license key update at all.
Also, I think this is not the correct implementation, so consider it more of a bug report 😁
Summary
Rotating or updating a license key now enqueues
benefit.updatefor the associated benefit grant, so the grant is refreshed and abenefit_grant.updatedwebhook /benefit.updatedsystem event is emitted.What
LicenseKeyService.rotateenqueuesenqueue_job("benefit.update", benefit_grant_id=grant.id)after the new key is flushed, when a grant exists for the key.LicenseKeyService.updatedoes the same after applying updates (in addition to the existinglicense_key.sync_benefit_grantenqueue on status transitions)._get_granthelper, now shared byupdate,rotate, and_enqueue_grant_lifecycle.Why
Previously, rotating a key updated the grant's
display_keyproperty directly but never notified anyone: nobenefit_grant.updatedwebhook fired, so merchant integrations kept working off the old key. Likewise, updating a key's limits/expiry didn't propagate to the grant. Enqueuingbenefit.updaterunsupdate_benefit_grant, which re-syncs the grant properties and emits the webhook and system event.How
Both flows already (or now, via
_get_grant) resolve the benefit grant by thelicense_key_idgrant property; when one exists, the job is enqueued aftersession.flush().One behavior to be aware of for review: the
benefit.updatetask re-runs the license-keys strategygrant(update=True), which goes throughcustomer_update_grantand resetsexpires_at,limit_activations, andlimit_usageon the key from the benefit's configured properties. That means a per-key override made viaPATCH /v1/license-keys/{id}(e.g. a customlimit_usage) will be reverted back to the benefit defaults by the enqueued job. If per-key overrides should survive,customer_update_grantwould need to distinguish benefit-driven updates from key-level ones — happy to follow up if you want that changed.Checklist
uv run task lint && uv run task lint_types)🤖 Generated with Claude Code
https://claude.ai/code/session_01LXM55Y42X6hbbPPTKYEJ5g
Generated by Claude Code