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

Added plugin cookie jar for Issue #1028 #1039

Merged
merged 3 commits into from Jul 12, 2018
Merged

Conversation

@coolhome
Copy link
Contributor

coolhome commented Jul 9, 2018

Closes #1028

@welcome

This comment has been minimized.

Copy link

welcome bot commented Jul 9, 2018

💖 Thanks for opening this pull request! 💖

To help make this a smooth process, please be sure you have first read the
contributing guidelines.

@coolhome coolhome changed the title Added plugin cookie jar for PR #1028 Added plugin cookie jar for Issue #1028 Jul 9, 2018
Copy link
Contributor

gschier left a comment

This looks awesome @coolhome! Thanks for doing this. I just had one comment about the implementation.

// Validate cookie domain
let cookiesWithMatchedDomain = cookies.filter(cookie => cookie.domain === cookieDomain);

if(cookiesWithMatchedDomain.length === 0) {

This comment has been minimized.

Copy link
@gschier

gschier Jul 9, 2018

Contributor

What do you think of changing this to use the same matching logic as the Request => Cookie plugin? This would allow the user to also match on URL path and would keep the behavior consistent.

https://github.com/getinsomnia/insomnia/blob/528a8c3b2b8c9eeb368a1087aa74e76ec1d7e931/plugins/insomnia-plugin-request/index.js#L175-L199

This comment has been minimized.

Copy link
@gschier

gschier Jul 9, 2018

Contributor

Also note, this would mean changing the "Cookie Domain" parameter to accept a more flexible "URL"

This comment has been minimized.

Copy link
@coolhome

coolhome Jul 9, 2018

Author Contributor

@gschier I did not realize that the request plugin did this. I am now using the same function.

:)

EDIT: I'm still not sure if "Url" is a good argument. I did add a little description with an example.

Copy link
Contributor

gschier left a comment

Awesome work @coolhome!

@gschier gschier merged commit 0aea04d into Kong:develop Jul 12, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@welcome

This comment has been minimized.

Copy link

welcome bot commented Jul 12, 2018

Congrats on merging your first pull request! 🎉🎉🎉 You're helping make Insomnia awesome! 🙌

@coolhome coolhome deleted the coolhome:issue-1028 branch Jul 12, 2018
luizmariz pushed a commit to luizmariz/insomnia that referenced this pull request Jan 22, 2020
* Added plugin cookie jar for PR Kong#1028

* Refactored how cookie domain works. Now consistent with request plugin

* Updated unit tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.