Skip to content

feat: add coder_parameter_order to all data.coder_parameter fields #223

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 15, 2024
Merged

feat: add coder_parameter_order to all data.coder_parameter fields #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f8d456
feat: add coder_parameter_order
michaelbrewer Apr 11, 2024
8452de1
feat(git-clone): add coder_parameter order variables
michaelbrewer Apr 11, 2024
75b4da4
Merge branch 'main' into feat/code_parameter_order
michaelbrewer Apr 14, 2024
6c94297
feat: use just a single coder_parameter_order for the group
michaelbrewer Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions aws-region/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,13 @@ describe("aws-region", async () => {
});
expect(state.outputs.value.value).toBe("us-west-2");
});

it("set custom order for coder_parameter", async () => {
const order = 99;
const state = await runTerraformApply(import.meta.dir, {
coder_parameter_order: order.toString(),
});
expect(state.resources).toHaveLength(1);
expect(state.resources[0].instances[0].attributes.order).toBe(order);
});
});
7 changes: 7 additions & 0 deletions aws-region/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ variable "exclude" {
type = list(string)
}

variable "coder_parameter_order" {
type = number
description = "The order determines the position of a template parameter in the UI/CLI presentation. The lowest order is shown first and parameters with equal order are sorted by name (ascending order)."
default = null
}

locals {
# This is a static list because the regions don't change _that_
# frequently and including the `aws_regions` data source requires
Expand Down Expand Up @@ -176,6 +182,7 @@ data "coder_parameter" "region" {
display_name = var.display_name
description = var.description
default = var.default == "" ? null : var.default
order = var.coder_parameter_order
mutable = var.mutable
dynamic "option" {
for_each = { for k, v in local.regions : k => v if !(contains(var.exclude, k)) }
Expand Down
9 changes: 9 additions & 0 deletions azure-region/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,13 @@ describe("azure-region", async () => {
});
expect(state.outputs.value.value).toBe("westus");
});

it("set custom order for coder_parameter", async () => {
const order = 99;
const state = await runTerraformApply(import.meta.dir, {
coder_parameter_order: order.toString(),
});
expect(state.resources).toHaveLength(1);
expect(state.resources[0].instances[0].attributes.order).toBe(order);
});
});
7 changes: 7 additions & 0 deletions azure-region/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ variable "exclude" {
type = list(string)
}

variable "coder_parameter_order" {
type = number
description = "The order determines the position of a template parameter in the UI/CLI presentation. The lowest order is shown first and parameters with equal order are sorted by name (ascending order)."
default = null
}

locals {
# Note: Options are limited to 64 regions, some redundant regions have been removed.
all_regions = {
Expand Down Expand Up @@ -309,6 +315,7 @@ data "coder_parameter" "region" {
display_name = var.display_name
description = var.description
default = var.default == "" ? null : var.default
order = var.coder_parameter_order
mutable = var.mutable
icon = "/icon/azure.png"
dynamic "option" {
Expand Down
10 changes: 10 additions & 0 deletions dotfiles/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,14 @@ describe("dotfiles", async () => {
});
expect(state.outputs.dotfiles_uri.value).toBe(default_dotfiles_uri);
});

it("set custom order for coder_parameter", async () => {
const order = 99;
const state = await runTerraformApply(import.meta.dir, {
agent_id: "foo",
coder_parameter_order: order.toString(),
});
expect(state.resources).toHaveLength(2);
expect(state.resources[0].instances[0].attributes.order).toBe(order);
});
});
7 changes: 7 additions & 0 deletions dotfiles/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,17 @@ variable "default_dotfiles_uri" {
default = ""
}

variable "coder_parameter_order" {
type = number
description = "The order determines the position of a template parameter in the UI/CLI presentation. The lowest order is shown first and parameters with equal order are sorted by name (ascending order)."
default = null
}

data "coder_parameter" "dotfiles_uri" {
type = "string"
name = "dotfiles_uri"
display_name = "Dotfiles URL (optional)"
order = var.coder_parameter_order
default = var.default_dotfiles_uri
description = "Enter a URL for a [dotfiles repository](https://dotfiles.github.io) to personalize your workspace"
mutable = true
Expand Down
9 changes: 9 additions & 0 deletions exoscale-instance-type/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,13 @@ describe("exoscale-instance-type", async () => {
});
}).toThrow('default value "gpu3.huge" must be defined as one of options');
});

it("set custom order for coder_parameter", async () => {
const order = 99;
const state = await runTerraformApply(import.meta.dir, {
coder_parameter_order: order.toString(),
});
expect(state.resources).toHaveLength(1);
expect(state.resources[0].instances[0].attributes.order).toBe(order);
});
});
7 changes: 7 additions & 0 deletions exoscale-instance-type/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ variable "exclude" {
type = list(string)
}

variable "coder_parameter_order" {
type = number
description = "The order determines the position of a template parameter in the UI/CLI presentation. The lowest order is shown first and parameters with equal order are sorted by name (ascending order)."
default = null
}

locals {
# https://www.exoscale.com/pricing/

Expand Down Expand Up @@ -257,6 +263,7 @@ data "coder_parameter" "instance_type" {
display_name = var.display_name
description = var.description
default = var.default == "" ? null : var.default
order = var.coder_parameter_order
mutable = var.mutable
dynamic "option" {
for_each = [for k, v in concat(
Expand Down
9 changes: 9 additions & 0 deletions exoscale-zone/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,13 @@ describe("exoscale-zone", async () => {
});
expect(state.outputs.value.value).toBe("at-vie-1");
});

it("set custom order for coder_parameter", async () => {
const order = 99;
const state = await runTerraformApply(import.meta.dir, {
coder_parameter_order: order.toString(),
});
expect(state.resources).toHaveLength(1);
expect(state.resources[0].instances[0].attributes.order).toBe(order);
});
});
6 changes: 6 additions & 0 deletions exoscale-zone/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ variable "exclude" {
type = list(string)
}

variable "coder_parameter_order" {
type = number
description = "The order determines the position of a template parameter in the UI/CLI presentation. The lowest order is shown first and parameters with equal order are sorted by name (ascending order)."
default = null
}

locals {
# This is a static list because the zones don't change _that_
Expand Down Expand Up @@ -94,6 +99,7 @@ data "coder_parameter" "zone" {
display_name = var.display_name
description = var.description
default = var.default == "" ? null : var.default
order = var.coder_parameter_order
mutable = var.mutable
dynamic "option" {
for_each = { for k, v in local.zones : k => v if !(contains(var.exclude, k)) }
Expand Down
9 changes: 9 additions & 0 deletions gcp-region/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,13 @@ describe("gcp-region", async () => {
});
expect(state.outputs.value.value).toBe("us-west2-b");
});

it("set custom order for coder_parameter", async () => {
const order = 99;
const state = await runTerraformApply(import.meta.dir, {
coder_parameter_order: order.toString(),
});
expect(state.resources).toHaveLength(1);
expect(state.resources[0].instances[0].attributes.order).toBe(order);
});
});
7 changes: 7 additions & 0 deletions gcp-region/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ variable "single_zone_per_region" {
type = bool
}

variable "coder_parameter_order" {
type = number
description = "The order determines the position of a template parameter in the UI/CLI presentation. The lowest order is shown first and parameters with equal order are sorted by name (ascending order)."
default = null
}

locals {
zones = {
# US Central
Expand Down Expand Up @@ -715,6 +721,7 @@ data "coder_parameter" "region" {
icon = "/icon/gcp.png"
mutable = var.mutable
default = var.default != null && var.default != "" && (!var.gpu_only || try(local.zones[var.default].gpu, false)) ? var.default : null
order = var.coder_parameter_order
dynamic "option" {
for_each = {
for k, v in local.zones : k => v
Expand Down
30 changes: 30 additions & 0 deletions git-config/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,34 @@ describe("git-config", async () => {
{ type: "coder_env", name: "git_commmiter_name" },
]);
});

it("set custom order for coder_parameter for both fields", async () => {
const order = 20;
const state = await runTerraformApply(import.meta.dir, {
agent_id: "foo",
allow_username_change: "true",
allow_email_change: "true",
coder_parameter_order: order.toString(),
});
expect(state.resources).toHaveLength(5);
// user_email order is the same as the order
expect(state.resources[0].instances[0].attributes.order).toBe(order);
// username order is incremented by 1
// @ts-ignore: Object is possibly 'null'.
expect(state.resources[1].instances[0]?.attributes.order).toBe(order + 1);
});

it("set custom order for coder_parameter for just username", async () => {
const order = 30;
const state = await runTerraformApply(import.meta.dir, {
agent_id: "foo",
allow_email_change: "false",
allow_username_change: "true",
coder_parameter_order: order.toString(),
});
expect(state.resources).toHaveLength(4);
// user_email was not created
// username order is incremented by 1
expect(state.resources[0].instances[0].attributes.order).toBe(order + 1);
});
});
7 changes: 7 additions & 0 deletions git-config/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ variable "allow_email_change" {
default = false
}

variable "coder_parameter_order" {
type = number
description = "The order determines the position of a template parameter in the UI/CLI presentation. The lowest order is shown first and parameters with equal order are sorted by name (ascending order)."
default = null
}
Copy link
Member

Choose a reason for hiding this comment

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

Would it be feasible to just use one coder_parameter_order for all parameters? If someone wants to change these individually, then there may be something wrong with the module or how it's being used.

Why? This makes modules updates more risky. If we ever add a third parameter, it could end up anywhere when the user updates.

Perhaps we can still apply order if it's given, e.g. order = var.coder_parameter_order ? var.coder_parameter_order + 0 : null, + 1, etc.

An author can reserve as much ordered space for a module as they wish.

module "git-config" {
  source                = "registry.coder.com/modules/git-config/coder"
  version               = "1.0.11"
  agent_id              = coder_agent.example.id
  allow_email_change    = true
  coder_parameter_order = 10
}

module "gcp_region" {
  source                = "registry.coder.com/modules/gcp-region/coder"
  version               = "1.0.11"
  regions               = ["us", "europe"]
  coder_parameter_order = 20
}

To clarify this, maybe we could name it something like coder_parameter_group_order or coder_parameter_order_weight, I'm open to better ideas 😄.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, i can keep it all the same field name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mafredri - i have updated the code and tests


data "coder_workspace" "me" {}

Expand All @@ -34,6 +39,7 @@ data "coder_parameter" "user_email" {
name = "user_email"
type = "string"
default = ""
order = var.coder_parameter_order != null ? var.coder_parameter_order + 0 : null
description = "Git user.email to be used for commits. Leave empty to default to Coder user's email."
display_name = "Git config user.email"
mutable = true
Expand All @@ -44,6 +50,7 @@ data "coder_parameter" "username" {
name = "username"
type = "string"
default = ""
order = var.coder_parameter_order != null ? var.coder_parameter_order + 1 : null
description = "Git user.name to be used for commits. Leave empty to default to Coder user's Full Name."
display_name = "Full Name for Git config"
mutable = true
Expand Down