Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): authentication from postman collection is not parsed into Restfox #236

Merged
merged 8 commits into from
Sep 9, 2024

Conversation

kobenguyent
Copy link
Collaborator

fix an issue where importing postman collection with authentication results in no auth header.

after the fix
Screenshot 2024-09-06 at 18 06 16

Copy link

netlify bot commented Sep 6, 2024

Deploy Preview for chimerical-kitsune-a0bfa0 ready!

Name Link
🔨 Latest commit 1193f48
🔍 Latest deploy log https://app.netlify.com/sites/chimerical-kitsune-a0bfa0/deploys/66deb1266a3eb200089620b9
😎 Deploy Preview https://deploy-preview-236--chimerical-kitsune-a0bfa0.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.

@flawiddsouza
Copy link
Owner

@kobenguyent
image
This file is just kept for reference. The actual file you need to update is: packages/ui/test-data/postman-import-v2/Restfox_2024-09-06.json

@flawiddsouza
Copy link
Owner

By the way, regarding this change:
image
Regex is always slower than direct string matching, so always prefer direct string matching if you know the string beforehand. See this:
image
The new code is 81% slower.

@kobenguyent
Copy link
Collaborator Author

By the way, regarding this change:

image

Regex is always slower than direct string matching, so always prefer direct string matching if you know the string beforehand. See this:

image

The new code is 81% slower.

Thanks! Not aware of this. How abt using includes?

@flawiddsouza
Copy link
Owner

image

  1. constant string checking is always the fastest, so even if it seems like code repetition, it's the best solution (the original code)
  2. 2nd is startsWith as string is checked from the beginning if the string you want to check starts from the beginning
  3. 3rd is includes as string can be anywhere within the string
  4. 4th is regex

@kobenguyent
Copy link
Collaborator Author

Thank you for the info! Updated as it is.

@flawiddsouza
Copy link
Owner

@kobenguyent I found an issue. It seems the auth logic I wrote was for Postman Schema v2.1. When I exported from postman as v2.1, I got this:
image

When I exported collection as v2.0, I got the format you're supporting:

image

We need to handle both formats. Looks like they changed the schemas for auth between 2.0 and 2.1.

@kobenguyent
Copy link
Collaborator Author

@kobenguyent I found an issue. It seems the auth logic I wrote was for Postman Schema v2.1. When I exported from postman as v2.1, I got this: image

When I exported collection as v2.0, I got the format you're supporting:

image

We need to handle both formats. Looks like they changed the schemas for auth between 2.0 and 2.1.

sure, updated and added tests!

@flawiddsouza flawiddsouza merged commit fd5e843 into main Sep 9, 2024
6 checks passed
@flawiddsouza flawiddsouza deleted the fix-postman-parser branch September 9, 2024 08:37
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