Skip to content

fix: throw error when decline binding is unavailable - #239

Closed
iyop666 wants to merge 3 commits into
boundlessfi:mainfrom
iyop666:fix-decline-fake-success
Closed

iyop666 wants to merge 3 commits into
boundlessfi:mainfrom
iyop666:fix-decline-fake-success

Conversation

@iyop666

@iyop666 iyop666 commented May 28, 2026

Copy link
Copy Markdown

What

The useDeclineApplicant hook returns a fake success result when the contract client does not include declineApplicant. This leaves the optimistic cache removal in place and shows a misleading success toast.

Why

Line 207 returns { persisted: false, result: { txHash: "local-decline-applicant" } } instead of throwing. Since the mutation "succeeds", onError never fires and the optimistic update is never rolled back.

How

Replace the fake return with throw new ApplicationError(...) so the error boundary rolls back the cache and the user sees an actual error message.

Checklist

  • No new dependencies
  • Existing error handling (onError + onMutate rollback) handles this correctly

Summary by CodeRabbit

  • New Features
    • Applications can now be declined directly from the review dashboard with an optional decline reason
    • A confirmation dialog allows you to add context before declining an application
    • Declined applications are automatically moved to a separate section
    • Real-time notifications confirm successful or failed decline operations
    • The decline button is disabled during processing to prevent duplicate submissions

Review Change Stack

iyop666 and others added 2 commits May 28, 2026 06:00
Previously the mutation returned a fake success result even when
declineApplicant was not available on the contract client. This
left the optimistic removal in place and showed a misleading
success toast.

Now it throws so the error boundary rolls back the cache and
the user sees an actual error message.
@vercel

vercel Bot commented May 28, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Threadflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@iyop666, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 9 minutes and 39 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5b99441f-b8bf-4fc9-b7d3-a85570034bf3

📥 Commits

Reviewing files that changed from the base of the PR and between ec2f038 and 7f25145.

📒 Files selected for processing (1)
  • hooks/use-bounty-application.ts
📝 Walkthrough

Walkthrough

The PR adds applicant decline functionality to the bounty review dashboard. A new useDeclineApplicant hook implements the decline mutation with optimistic cache updates, the dashboard component imports necessary UI components and wires the hook, and the full decline flow is implemented via a decline button on each card that opens an alert dialog for confirming the decline with an optional reason.

Changes

Application Decline Workflow

Layer / File(s) Summary
Decline contract and hook
hooks/use-bounty-application.ts
ApplicationContractClient now includes optional declineApplicant method signature. The useDeclineApplicant hook performs the decline mutation, executes optimistic cache update moving the applicant from applications to declinedApplications with status: "declined" and normalized reason, restores cache on error, and invalidates bounty detail and list queries on successful persistence.
Dashboard component setup
components/bounty/application-review-dashboard.tsx
Imports AlertDialog components, XCircle icon, and the useDeclineApplicant hook. Adds declineReason and applicationToDecline state, and wires the hook to expose declineApplicant mutation and isDeclining loading flag.
Decline dialog and handlers
components/bounty/application-review-dashboard.tsx
Introduces openDeclineDialog/closeDeclineDialog helpers and handleDeclineApplicant function that validates selection, calls the mutation with bounty ID and applicant address, updates compare selection on success, and displays success/error toasts. Adds red-outlined Decline button to each application card (disabled while declining) that opens the dialog for that application. Renders AlertDialog that displays when an application is selected for decline, with textarea for optional reason and Cancel/Decline action buttons; Decline action calls the handler and respects the isDeclining state.

Sequence Diagram

sequenceDiagram
  participant User
  participant Dashboard as ApplicationReviewDashboard
  participant Dialog as AlertDialog
  participant Hook as useDeclineApplicant
  participant Contracts as ApplicationContracts
  participant Cache as React Query Cache
  
  User->>Dashboard: Click Decline button on card
  Dashboard->>Dialog: Open (set applicationToDecline)
  User->>Dialog: Enter reason and click Decline
  Dialog->>Dashboard: handleDeclineApplicant()
  Dashboard->>Hook: declineApplicant(bountyId, applicantAddress, reason)
  Hook->>Cache: Optimistic update (move to declinedApplications)
  Hook->>Contracts: Call contract declineApplicant()
  Contracts-->>Hook: { txHash }
  Hook->>Cache: Invalidate bounty detail and lists
  Hook-->>Dashboard: Success
  Dashboard->>Dashboard: Update compare selection, show success toast
  Dashboard->>Dialog: Close dialog
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

  • boundlessfi/bounties#200: The main ApplicationReviewDashboard component implementation that this PR extends with decline functionality via the new dialog and button UI.

Suggested reviewers

  • Benjtalkshow

Poem

🐰 A bounty declines with grace so fine,
Applicants removed from the review line.
Cache optimized with each kind refusal,
The dashboard now handles dismissal.
Hop hop—another feature takes flight! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% 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 accurately describes the main change: throwing an error when the decline binding is unavailable, which is the core fix implemented across the modified files.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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 `@hooks/use-bounty-application.ts`:
- Around line 192-209: Replace the direct global client access with the shared
resolver: call resolveApplicationClient() to obtain the application client
instead of reading (globalThis).__applicationContracts, then call
client.declineApplicant with the same payload; if resolveApplicationClient()
fails or declineApplicant is unavailable/throws, throw an ApplicationError with
an appropriate code/message (preserving the original failure semantics) rather
than a generic Error so downstream handlers can rely on ApplicationError.code;
update the block containing declineApplicant in use-bounty-application.ts to use
resolveApplicationClient() and ApplicationError consistently.
🪄 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: 6e525fc4-999e-4383-ab14-ca3cbba1ed94

📥 Commits

Reviewing files that changed from the base of the PR and between ede3b20 and ec2f038.

📒 Files selected for processing (2)
  • components/bounty/application-review-dashboard.tsx
  • hooks/use-bounty-application.ts

Comment thread hooks/use-bounty-application.ts Outdated
Address CodeRabbit review: use the shared resolver instead of
direct globalThis access, and throw ApplicationError with proper
code instead of generic Error.
@iyop666

iyop666 commented May 28, 2026

Copy link
Copy Markdown
Author

@coderabbitai review — pushed a fix addressing your feedback. Now uses resolveApplicationClient() and ApplicationError with proper code.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the hooks file
echo "=== hooks/use-bounty-application.ts ==="
cat hooks/use-bounty-application.ts

echo ""
echo "=== components/bounty/application-review-dashboard.tsx ==="
cat components/bounty/application-review-dashboard.tsx

# Also look for ApplicationError and resolveApplicationClient definitions
echo ""
echo "=== ApplicationError definition ==="
rg -n "class ApplicationError|export.*ApplicationError" --type ts -A 10

echo ""
echo "=== resolveApplicationClient definition ==="
rg -n "resolveApplicationClient" --type ts -A 10

Length of output: 27503



@iyop666 The fix looks solid. The critical issue (fake success silently swallowing errors) is definitively resolved, and the overall structure is clean and consistent across all hooks. A few minor observations below:


hooks/use-bounty-application.ts

  1. persisted is now always true — since the { persisted: false, result: { txHash: "local-decline-applicant" } } path is gone, onSuccess's if (response.persisted) guard is effectively dead code. You can simplify this:

    // Before
    onSuccess: (response, v) => {
      if (response.persisted) {
        qc.invalidateQueries({ queryKey: bountyKeys.detail(v.bountyId) });
        qc.invalidateQueries({ queryKey: bountyKeys.lists() });
      }
    },
    
    // After (simplified)
    onSettled: (_r, _e, v) => {
      qc.invalidateQueries({ queryKey: bountyKeys.detail(v.bountyId) });
      qc.invalidateQueries({ queryKey: bountyKeys.lists() });
    },

    Or keep onSuccess if you want to avoid re-invalidating on error. Either way it's a minor cleanup.

  2. ApplicationError missing this.name — a JS best practice so error.name shows "ApplicationError" in logs/stack traces rather than "Error":

    export class ApplicationError extends Error {
      code: ApplicationErrorCode;
      constructor(code: ApplicationErrorCode, message: string) {
        super(message);
        this.name = "ApplicationError"; // add this
        this.code = code;
      }
    }
  3. "missing_contract_bindings" for a missing optional binding — using the same error code for "no client at all" and "client exists but declineApplicant is absent" is workable, but if you ever want to give consumers finer-grained error handling, a separate code like "feature_unavailable" would be cleaner. Not a blocker.


components/bounty/application-review-dashboard.tsx

  1. isDeclining disables all Decline buttons — when one decline is in-flight, every other application's Decline button also becomes disabled. It's a safe/conservative choice, but if multiple simultaneous declines should ever be allowed, you'd need per-application pending state. Fine for now.

  2. event.preventDefault() + manual submit — correct approach to prevent AlertDialogAction's default dismiss-on-click from closing the dialog before the async mutation resolves.


Overall the implementation is in good shape. Items 1 and 2 are the most worthwhile cleanups; the rest are informational. Happy to approve once you've had a chance to consider them.

[approve]

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