Skip to content

Conversation

@stefanbitcr
Copy link
Contributor

@stefanbitcr stefanbitcr commented Jun 30, 2025

📝 Description

Add request to pay an ebill button.

Caveats:

Notes

  • Internally the endpoint is called request_to_mint_from_ebill. Here we called it request to pay, however we can rename it to the original if its argued to be less ambiguous.

📸 Screenshots/Screen record (if applicable)

image

✅ Checklist

Please ensure the following tasks are completed before requesting a review:

  • My code adheres to the style guidelines of this project.
  • I have run npm run lint or the equivalent linting command.
  • I have added or updated tests (if applicable).
  • My changes have been tested thoroughly in different browsers/resolutions (if applicable).
  • I have updated the documentation (if applicable).
  • I have checked that there are no console errors or warnings.
  • I have verified that the application builds without errors.
  • I have tested responsiveness for mobile and desktop views (if applicable).

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a Request to Pay feature for an e-bill, allowing users to initiate a mint request via a new button in the quote page. Key changes include adding the new mutation and state handling for the mint request, updating the QuoteActions component to handle the new ebillPaid flag, and displaying additional bill details in the quotes table.

Comments suppressed due to low confidence (1)

src/pages/quotes/QuotePage.tsx:494

  • [nitpick] Ensure the button's label reflects the feature's intent consistently; if 'Request to Pay' is chosen over the internal 'request to mint' naming, consider documenting the naming decision for clarity.
                Request to Pay {requestToMintMutation.isPending && <LoaderIcon className="stroke-1 animate-spin" />}

},
throwOnError: true,
})
console.log(data)
Copy link

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove or replace the console.log statement if it is no longer needed for debugging to keep production code clean.

Suggested change
console.log(data)

Copilot uses AI. Check for mistakes.
},
onSuccess: (data) => {
toast.success("Payment request has been created.")
console.log(data)
Copy link

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider removing this console.log once debugging is complete to avoid cluttering production logs.

Suggested change
console.log(data)

Copilot uses AI. Check for mistakes.
@qodo-code-review
Copy link

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

55 - Partially compliant

Compliant requirements:

• Prevent user from clicking the request to pay button twice

Non-compliant requirements:

• Check and display the current state of payment requests

Requires further human verification:

• Verify that the button state management works correctly in the UI
• Test that duplicate clicks are properly prevented during loading states

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

State Management

The mintRequestResponse state is set after successful API call but there's no mechanism to reset it or handle multiple requests properly. The button remains enabled after a successful request, potentially allowing duplicate requests if the component re-renders.

{value.status === "Accepted" && "keyset_id" in value && !ebillPaid && !newKeyset ? (
  <ConfirmDrawer
    title="Confirm requesting to mint"
    description="Are you sure you want to request to mint from this e-bill?"
    open={requestToMintConfirmDrawerOpen}
    onOpenChange={setRequestToMintConfirmDrawerOpen}
    onSubmit={() => {
      onRequestToMint()
      setRequestToMintConfirmDrawerOpen(false)
    }}
    submitButtonText="Yes, request to mint"
    trigger={
      <Button className="flex-1" disabled={isFetching || requestToMintMutation.isPending} variant="default">
        Request to Pay {requestToMintMutation.isPending && <LoaderIcon className="stroke-1 animate-spin" />}
      </Button>
    }
  />
) : (
  <></>
)}
Missing Validation

The request to mint button condition checks for ebillPaid and newKeyset states, but doesn't check if a mint request has already been made successfully, which could lead to duplicate requests.

{value.status === "Accepted" && "keyset_id" in value && !ebillPaid && !newKeyset ? (
  <ConfirmDrawer
    title="Confirm requesting to mint"
    description="Are you sure you want to request to mint from this e-bill?"
    open={requestToMintConfirmDrawerOpen}
    onOpenChange={setRequestToMintConfirmDrawerOpen}
    onSubmit={() => {
      onRequestToMint()
      setRequestToMintConfirmDrawerOpen(false)
    }}
    submitButtonText="Yes, request to mint"
    trigger={
      <Button className="flex-1" disabled={isFetching || requestToMintMutation.isPending} variant="default">
        Request to Pay {requestToMintMutation.isPending && <LoaderIcon className="stroke-1 animate-spin" />}
      </Button>
    }
  />
) : (
  <></>
)}

@codecov
Copy link

codecov bot commented Jun 30, 2025

Codecov Report

Attention: Patch coverage is 0% with 176 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pages/quotes/QuotePage.tsx 0.00% 166 Missing ⚠️
src/generated/client/sdk.gen.ts 0.00% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

@qodo-code-review
Copy link

qodo-code-review bot commented Jun 30, 2025

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Prevent duplicate mint requests

The button should be disabled when a mint request has already been made to
prevent duplicate requests. Add mintRequestResponse to the disabled condition to
ensure the button becomes inactive after a successful request.

src/pages/quotes/QuotePage.tsx [481-500]

 {value.status === "Accepted" && "keyset_id" in value && !ebillPaid && !newKeyset ? (
   <ConfirmDrawer
     title="Confirm requesting to mint"
     description="Are you sure you want to request to mint from this e-bill?"
     open={requestToMintConfirmDrawerOpen}
     onOpenChange={setRequestToMintConfirmDrawerOpen}
     onSubmit={() => {
       onRequestToMint()
       setRequestToMintConfirmDrawerOpen(false)
     }}
     submitButtonText="Yes, request to mint"
     trigger={
-      <Button className="flex-1" disabled={isFetching || requestToMintMutation.isPending} variant="default">
+      <Button className="flex-1" disabled={isFetching || requestToMintMutation.isPending || !!mintRequestResponse} variant="default">
         Request to Pay {requestToMintMutation.isPending && <LoaderIcon className="stroke-1 animate-spin" />}
       </Button>
     }
   />
 ) : (
   <></>
 )}
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the "Request to Pay" button can be clicked multiple times, and disabling it after a successful request by checking mintRequestResponse improves UI robustness and prevents duplicate submissions.

Low
  • Update

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jun 30, 2025

Deploying wildcat-dashboard with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7a87463
Status: ✅  Deploy successful!
Preview URL: https://484fd8dc.wildcat-dashboard.pages.dev
Branch Preview URL: https://stefan-requestmint.wildcat-dashboard.pages.dev

View logs

Copy link

@zupzup zupzup left a comment

Choose a reason for hiding this comment

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

LGTM 👍

* Request Mint
*/

// pub struct RequestToMintFromEBillRequest {
Copy link

Choose a reason for hiding this comment

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

can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed

},
onError: (error) => {
toast.error("Error while requesting to pay")
console.warn(error)
Copy link

Choose a reason for hiding this comment

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

is this a leftover? Seems a bit unnecessary, since the error can be checked in dev tools

@stefanbitcr stefanbitcr merged commit 14eed8c into master Jun 30, 2025
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants