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

feat: adding support for cookies in snippets #421

Merged
merged 1 commit into from
Mar 22, 2022

Conversation

erunion
Copy link
Member

@erunion erunion commented Mar 22, 2022

🚥 Fix RM-3946

🧰 Changes

This adds support for both cookie auth and cookie parameters in api snippets. This work was previously added to api in #393 but, contrary to what that PR says, snippets have never supported it.

🧬 QA & Testing

  • See the test I added for cookie auth.
  • I uncommented and fixed the existing cookie parameter test we had and that's passing now.

@erunion erunion added enhancement New feature or request area:snippets Issues related to code snippets labels Mar 22, 2022

delete queryParams[param];
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why I was doing this hokey property deletion stuff instead of just setting directly to metadata... 🤷

@@ -32,11 +32,11 @@
],
"cookies": [
{
"name": "foo",
"name": "foo-cookie",
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed these cookie names off foo and bar because this, and the full, test have those already as parameters and api doesn't support differing parameter types that share the same name.

@erunion erunion requested review from a team, kellyjosephprice, kanadgupta, billhimmelsbach and Dashron and removed request for a team March 22, 2022 20:08
@erunion erunion added this to the v5 milestone Mar 22, 2022
Copy link

@Dashron Dashron left a comment

Choose a reason for hiding this comment

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

code and tests look fine

Copy link

@kellyjosephprice kellyjosephprice left a comment

Choose a reason for hiding this comment

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

LGTM.

packages/httpsnippet-client-api/src/index.js Outdated Show resolved Hide resolved
Base automatically changed from fix/operationid-handling-cleanup to main March 22, 2022 20:45
@erunion erunion force-pushed the feat/cookie-support-in-snippets branch from 304fcdc to 6ebb613 Compare March 22, 2022 20:57
@erunion erunion merged commit a355800 into main Mar 22, 2022
@erunion erunion deleted the feat/cookie-support-in-snippets branch March 22, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:snippets Issues related to code snippets enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants