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

Parse Permit Url #738

Merged
merged 6 commits into from
Sep 18, 2023
Merged

Conversation

Sadaf-A
Copy link
Contributor

@Sadaf-A Sadaf-A commented Sep 8, 2023

Resolves #526

@netlify
Copy link

netlify bot commented Sep 8, 2023

Deploy Preview for ubiquibot-staging ready!

Name Link
🔨 Latest commit b23de34
🔍 Latest deploy log https://app.netlify.com/sites/ubiquibot-staging/deploys/6507ed2914164700089f5732
😎 Deploy Preview https://deploy-preview-738--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.

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.

Also post QA

src/handlers/payout/action.ts Outdated Show resolved Hide resolved
src/handlers/payout/action.ts Outdated Show resolved Hide resolved
src/handlers/payout/action.ts Show resolved Hide resolved
0xcodercrane
0xcodercrane previously approved these changes Sep 18, 2023
- removing unused import statement
@0xcodercrane 0xcodercrane merged commit 8166989 into ubiquity:development Sep 18, 2023
9 checks passed
@whilefoo
Copy link
Collaborator

the parsing should be done with URLSearchParams to avoid dependence on param order like @Sadaf-A did. I don't understand why you changed it to regex

@Sadaf-A
Copy link
Contributor Author

Sadaf-A commented Sep 18, 2023

the parsing should be done with URLSearchParams to avoid dependence on param order like @Sadaf-A did. I don't understand why you changed it to regex

@whilefoo
@0xcodercrane made those changes so I didn't wanna make any more changes i fixed the errors that arised after it earlier i was also using URLSearchParams as requested by you

@whilefoo
Copy link
Collaborator

@whilefoo
@0xcodercrane made those changes so I didn't wanna make any more changes i fixed the errors that arised after it earlier i was also using URLSearchParams as requested by you

yes I know @0xcodercrane made those changes

@0xcodercrane
Copy link
Contributor

We don't create a complex claim URL here, so I guess no need to worry about the param order.

In upper lines, we were also using regex so wanted to make the parse consistent.

If the order problem happens, lets update it at that time.

@0x4007
Copy link
Member

0x4007 commented Sep 23, 2023

This isn't the spec

@Sadaf-A
Copy link
Contributor Author

Sadaf-A commented Sep 23, 2023

the parsing should be done with URLSearchParams to avoid dependence on param order like @Sadaf-A did. I don't understand why you changed it to regex

@whilefoo @pavlovcik should i open another PR for this?

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
4 participants