Skip to content

Conversation

@filfreire
Copy link
Contributor

@filfreire filfreire commented Jul 2, 2024

This PR adds a new setting that allows disabling adding the default insomnia app version user-agent to new requests.

image

related to INS-4107

@filfreire filfreire requested a review from a team July 2, 2024 10:32
Copy link
Contributor

@gatzjames gatzjames left a comment

Choose a reason for hiding this comment

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

https://github.com/Kong/insomnia/pull/7640/files#diff-0d876a97684cc6c868835d083785c52894f879ce84216e92bdb0a9876d4abcafR577
I think the setting should be passed like the other objects here instead of using models.settings.getOrCreate() inside this function

@filfreire filfreire force-pushed the feat/INS-4107-setting-insomnia-default-UA branch from 1d7eb64 to 3f83003 Compare July 2, 2024 17:19
@filfreire filfreire requested a review from gatzjames July 3, 2024 07:35
@filfreire
Copy link
Contributor Author

filfreire commented Jul 3, 2024

@gatzjames, I submitted a second pass, please review again

@filfreire filfreire requested a review from a team July 3, 2024 07:36
@filfreire filfreire force-pushed the feat/INS-4107-setting-insomnia-default-UA branch 3 times, most recently from 95f91f4 to ac97db7 Compare July 3, 2024 09:36
gatzjames
gatzjames previously approved these changes Jul 3, 2024
Copy link
Contributor

@gatzjames gatzjames left a comment

Choose a reason for hiding this comment

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

Nice one!
Tested with many variations of enabling/disabling this and works great!

// cleanup orphan app/version user-agent headers
const existingAppVersionUserAgent: RequestHeader | null = headers.find((h: any) => h.name.toLowerCase() === 'user-agent' && h.disabled === false && h.value.startsWith('insomnia')) || null;
if (existingAppVersionUserAgent) {
headers.splice(headers.indexOf(existingAppVersionUserAgent), 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

splice is a mutating function, and tough to reason about.
Its better to use a pure function and name the output to make your code easier to read.

@jackkav jackkav requested a review from gatzjames July 3, 2024 12:07
@subnetmarco
Copy link
Member

Let's update this in such a way that it does not add the default Insomnia User-Agent to new requests that I create (but, if a different user that I am collaborating with is adding the User-Agent, then this checkbox will not do anything since it was not created by me).

The checkbox should be renamed with "Do not add Insomnia User-Agent for new requests".

@filfreire filfreire force-pushed the feat/INS-4107-setting-insomnia-default-UA branch from ac97db7 to 35d5f43 Compare July 3, 2024 13:10
jackkav
jackkav previously approved these changes Jul 3, 2024
@filfreire filfreire force-pushed the feat/INS-4107-setting-insomnia-default-UA branch from 0db33fe to 4ce4d3c Compare July 3, 2024 13:30
@filfreire filfreire requested a review from jackkav July 3, 2024 13:48
@filfreire filfreire enabled auto-merge (squash) July 3, 2024 14:05
@filfreire filfreire merged commit 9fea3a1 into Kong:develop Jul 3, 2024
@filfreire filfreire deleted the feat/INS-4107-setting-insomnia-default-UA branch July 3, 2024 14:08
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.

4 participants