Skip to content
This repository has been archived by the owner on Sep 19, 2024. It is now read-only.

Parse permit url #586

Closed
wants to merge 3 commits into from
Closed

Parse permit url #586

wants to merge 3 commits into from

Conversation

Sadaf-A
Copy link
Contributor

@Sadaf-A Sadaf-A commented Aug 7, 2023

Resolves #526

@netlify
Copy link

netlify bot commented Aug 7, 2023

Deploy Preview for ubiquibot-staging ready!

Name Link
🔨 Latest commit 66b1df2
🔍 Latest deploy log https://app.netlify.com/sites/ubiquibot-staging/deploys/64d3f187730acc00087d2c43
😎 Deploy Preview https://deploy-preview-586--ubiquibot-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@whilefoo whilefoo self-requested a review August 8, 2023 05:41
Copy link
Collaborator

@whilefoo whilefoo left a comment

Choose a reason for hiding this comment

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

Please fix your comment to have the issue number in the same line like this:
Resolves #526

Also include a link to the QA issue. Here are the instructions.

@@ -150,7 +150,17 @@ export const handleIssueClosed = async () => {
const shortenRecipient = shortenEthAddress(recipient, `[ CLAIM ${priceInEth} ${tokenSymbol.toUpperCase()} ]`.length);
logger.info(`Posting a payout url to the issue, url: ${payoutUrl}`);
const comment = `### [ **[ CLAIM ${priceInEth} ${tokenSymbol.toUpperCase()} ]** ](${payoutUrl})\n` + "```" + shortenRecipient + "```";
const permitComments = comments.filter((content) => content.body.includes("https://pay.ubq.fi?claim=") && content.user.type == UserType.Bot);
const permitComments = comments.filter((content) => {
if (content.body.includes("https://pay.ubq.fi?claim=") && content.user.type == UserType.Bot) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should only check for the base URL without query parameters

const permitComments = comments.filter((content) => content.body.includes("https://pay.ubq.fi?claim=") && content.user.type == UserType.Bot);
const permitComments = comments.filter((content) => {
if (content.body.includes("https://pay.ubq.fi?claim=") && content.user.type == UserType.Bot) {
const url = new URL(content.body);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to extract the URL from the comment with RegExp because the comment will be in Markdown syntax and includes other text. The example of the comment is in line 152.

const permitComments = comments.filter((content) => {
if (content.body.includes("https://pay.ubq.fi?claim=") && content.user.type == UserType.Bot) {
const url = new URL(content.body);
const searchParams = new URLSearchParams(url.search);
Copy link
Collaborator

Choose a reason for hiding this comment

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

URLSearchParams are already included in the URL object so no need to parse again.

@0x4007
Copy link
Member

0x4007 commented Aug 9, 2023

Thanks for the quick review @whilefoo

@0x4007 0x4007 added ping and removed ping labels Aug 9, 2023
@whilefoo
Copy link
Collaborator

@Sadaf-A will you finish this?

@Sadaf-A
Copy link
Contributor Author

Sadaf-A commented Aug 22, 2023

@Sadaf-A will you finish this?

After my other issue is resolved I'll get onto this one.

@0x4007 0x4007 added ping and removed ping labels Aug 29, 2023
@Draeieg
Copy link
Contributor

Draeieg commented Aug 30, 2023

@Sadaf-A will you finish this?

After my other issue is resolved I'll get onto this one.

@Sadaf-A seems the other issue is resolved you can go into this one

@rndquu
Copy link
Member

rndquu commented Sep 6, 2023

Closing this because of inactivity

@rndquu rndquu closed this Sep 6, 2023
@0x4007
Copy link
Member

0x4007 commented Sep 14, 2023

Closing this because of inactivity

Shouldn't the bot handle this automatically?

@rndquu
Copy link
Member

rndquu commented Sep 14, 2023

Closing this because of inactivity

Shouldn't the bot handle this automatically?

Yes, it should, related issue

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parse permit URL
5 participants