Environments order #4048

vincendep · 2021-09-23T20:09:16Z

Closes #3922
Closes #3822
Closes INS-954

dimitropoulos

Thanks! I tested using the steps in #3822 and can confirm that this PR fixes what's described in those steps.

Here's what I tested with:

vincendep · 2021-10-11T19:54:33Z

Hi @dimitropoulos. I also include a change to address issue #3922, but I was not sure if it was a good idea since there were no feedback from you under that

dimitropoulos · 2021-10-11T20:07:26Z

@vincendep is it something different than this fix? if so, it'd be great if you knew the reproduction steps (and would open a new PR for that issue so we can handle separately).

vincendep · 2021-10-11T21:20:28Z

It is quite related, since I had to update my previous changes.

develohpanda

On testing this seems to work well! A couple of notes however, would love your thoughts!

develohpanda · 2021-10-12T05:53:27Z

packages/insomnia-app/app/models/environment.ts

@@ -76,6 +76,7 @@ export async function getOrCreateForParentId(parentId: string) {
      // Deterministic base env ID. It helps reduce sync complexity since we won't have to
      // de-duplicate environments.
      _id: `${prefix}_${crypto.createHash('sha1').update(parentId).digest('hex')}`,
+      metaSortKey: 0,


I'm just curious on what purpose this serves specifically? It's worth writing a comment here to explain why the metaSortKey for the base environment should be hard-coded to 0.

develohpanda · 2021-10-12T06:01:10Z

packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.tsx

      environment,
-      { metaSortKey: 1 },
+      { metaSortKey: idx + 1 },


I think setting this to just idx should do the job. Looking at where this regression was introduced, that was the existing behavior: https://github.com/Kong/insomnia/pull/2891/files?authenticity_token=NotWiXgjksFGynH9ySdJfzuPFfvny1ipT861Ia0PCdxxQciOP08il4GzvolDtfnjCbgSLI7MzW3fm3gge3EmMQ%3D%3D&file-filters%5B%5D=.tsx#diff-83bf0bed6b835cb203957ba0b8b615b662856919617cab840b6ced94a9d02708L337-R375. During the refactor, metaSortKey: i was mistakenly changed to metaSortKey: 1.

insomnia/packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.tsx

Lines 337 to 346 in ce9637e

for (let i = 0; i < newSubEnvironments.length; i++) {

const environment = newSubEnvironments[i];

await this._updateEnvironment(

environment,

{

metaSortKey: i,

},

false,

);

}

Is the + 1 strictly necessary?

vincendep · 2021-10-12T06:08:56Z

Hi @develohpanda, I introduced that changes after looking at issue #3922. It is to keep the base environment metaSortKey always lower than the sub environment ones.

develohpanda · 2021-10-13T06:38:03Z

Hi @develohpanda, I introduced that changes after looking at issue #3922. It is to keep the base environment metaSortKey always lower than the sub environment ones.

Gotcha, thanks! I think we should remove the hard-coded value there and stick with what was there before, for the base environment. The metaSortKey for the base environment holds no significance, so while it's likely harmless to change it it doesn't feel like there's a compelling reason to change it.

The extra note in #3922 mentions the re-ordering issue, which is fixed by this PR, but that's not directly related to the base environment metaSortKey, only the sub environment ones.

How does that sound?

vincendep · 2021-10-13T06:50:26Z

It sounds good to me, indeed I was not sure of the utility to have the metaSotKey of the base environment lower than others. So I'll remove the hard coded one and set back the others to index instead of index plus 1

This reverts commit 110982c. # Conflicts: # packages/insomnia-app/app/models/environment.ts # packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.tsx

gatzjames

Good one! LGTM!

Vincenzo De Petris added 2 commits September 23, 2021 21:45

Use environment index as metaSortKey value

03269a3

Create Base Environment with metaSortKey set to 0

110982c

vercel bot temporarily deployed to Preview September 23, 2021 20:09 Inactive

vincendep changed the title ~~Use environment index as metaSortKey value~~ Environments order Sep 23, 2021

Lint fix

e5c9b0f

vercel bot temporarily deployed to Preview September 24, 2021 16:34 Inactive

develohpanda self-requested a review September 27, 2021 10:34

dimitropoulos approved these changes Oct 11, 2021

View reviewed changes

develohpanda reviewed Oct 12, 2021

View reviewed changes

Revert "Create Base Environment with metaSortKey set to 0"

1fc44c6

This reverts commit 110982c. # Conflicts: # packages/insomnia-app/app/models/environment.ts # packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.tsx

vercel bot temporarily deployed to Preview October 13, 2021 11:32 Inactive

Merge branch 'develop' into fix/env-order

702ca83

vercel bot temporarily deployed to Preview October 13, 2021 11:32 Inactive

develohpanda approved these changes Oct 13, 2021

View reviewed changes

gatzjames approved these changes Oct 13, 2021

View reviewed changes

Merge branch 'develop' into fix/env-order

bdef16b

vercel bot temporarily deployed to Preview October 13, 2021 18:44 Inactive

develohpanda enabled auto-merge (squash) October 13, 2021 18:45

develohpanda merged commit cbc1cfc into Kong:develop Oct 13, 2021

vincendep deleted the fix/env-order branch October 17, 2021 21:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Environments order #4048

Environments order #4048

vincendep commented Sep 23, 2021 •

edited by develohpanda

Loading

dimitropoulos left a comment

vincendep commented Oct 11, 2021

dimitropoulos commented Oct 11, 2021

vincendep commented Oct 11, 2021

develohpanda left a comment

develohpanda Oct 12, 2021

develohpanda Oct 12, 2021

vincendep commented Oct 12, 2021

develohpanda commented Oct 13, 2021 •

edited

Loading

vincendep commented Oct 13, 2021 •

edited

Loading

gatzjames left a comment

	for (let i = 0; i < newSubEnvironments.length; i++) {
	const environment = newSubEnvironments[i];
	await this._updateEnvironment(
	environment,
	{
	metaSortKey: i,
	},
	false,
	);
	}

Environments order #4048

Environments order #4048

Conversation

vincendep commented Sep 23, 2021 • edited by develohpanda Loading

dimitropoulos left a comment

Choose a reason for hiding this comment

vincendep commented Oct 11, 2021

dimitropoulos commented Oct 11, 2021

vincendep commented Oct 11, 2021

develohpanda left a comment

Choose a reason for hiding this comment

develohpanda Oct 12, 2021

Choose a reason for hiding this comment

develohpanda Oct 12, 2021

Choose a reason for hiding this comment

vincendep commented Oct 12, 2021

develohpanda commented Oct 13, 2021 • edited Loading

vincendep commented Oct 13, 2021 • edited Loading

gatzjames left a comment

Choose a reason for hiding this comment

vincendep commented Sep 23, 2021 •

edited by develohpanda

Loading

develohpanda commented Oct 13, 2021 •

edited

Loading

vincendep commented Oct 13, 2021 •

edited

Loading