Skip to content

Fix Phase 17 inline review issues#245

Merged
menvil merged 1 commit into
developfrom
fix/phase17-review-2
Jun 2, 2026
Merged

Fix Phase 17 inline review issues#245
menvil merged 1 commit into
developfrom
fix/phase17-review-2

Conversation

@menvil

@menvil menvil commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary

Three inline review findings — all valid, all fixed.

1 — stripe_price_id prefers payload over config

CreditPackWebhookHandler was always writing $pack->stripePriceId (from local config/env). The webhook payload metadata may carry the actual price_id that Stripe used. Fixed: extract $metadata['price_id'] and use it, falling back to $pack->stripePriceId only when null/empty.

2 — $hasInsufficientCredits reset in all exit paths

The flag was only cleared at the top of convert(). Missing locations:

  • resetTargetSelection() — now clears both convertError and hasInsufficientCredits; covers backToUploadSummary() and resetCurrentUpload() (replace/remove file)
  • backToFormatStep() — explicit reset added (doesn't call resetTargetSelection)
  • goToSettingsStep() when navigating away from convert — explicit reset added
  • convertAnother() — added both fields to the $this->reset([...]) list

3 — text-whitecolor:var(--ca-on-primary)

Design token --ca-on-primary: #ffffff exists in theme.css. No Tailwind utility class exists for it yet. Replaced text-white with color:var(--ca-on-primary) in the existing inline style on the <a> element. No docs/design/design-contract.md file exists in the repo — not created (not required by the finding).

Test plan

  • composer test passes (487 tests)
  • composer lint passes
  • npm run build passes

🤖 Generated with Claude Code


Summary by cubic

Fixes three Phase 17 inline review issues. Uses the Stripe webhook’s price_id when present, clears stale convert errors, and applies the --ca-on-primary design token for button text color.

  • Bug Fixes
    • Webhooks: read price_id from event metadata for stripe_price_id, falling back to $pack->stripePriceId only if missing.
    • Converter: reset convertError and hasInsufficientCredits in goToSettingsStep, backToFormatStep, resetTargetSelection, and convertAnother to prevent stale state.
    • UI: replace text-white with color:var(--ca-on-primary) in the buy-credits CTA to use the theme token.

Written for commit e55ae46. Summary will update on new commits.

Review in cubic

- Prefer payload price_id over config fallback in credit pack
  transaction metadata (stripe_price_id)
- Reset hasInsufficientCredits in resetTargetSelection(), backToFormatStep(),
  goToSettingsStep(), and convertAnother() so stale error state cannot
  bleed into subsequent convert attempts
- Replace text-white with color:var(--ca-on-primary) in buy-credits-cta
  component to use design token instead of hardcoded utility

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • release

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d4d967f6-887b-493b-9f69-7c932fa93708

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/phase17-review-2

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 and usage tips.

@menvil menvil merged commit 9a5021f into develop Jun 2, 2026
3 checks passed
@menvil menvil deleted the fix/phase17-review-2 branch June 2, 2026 19:12
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