Skip to content

feat: enable editing of IDP sync configuration for groups and roles in the UI #16098

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

Merged
merged 40 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
96b0c09
chore: remove stack component
jaaydenh Jan 10, 2025
a88b72f
chore: styling cleanup
jaaydenh Jan 10, 2025
cb00c73
feat: add form and mutation queries
jaaydenh Jan 10, 2025
4b15a2a
feat: add form fields
jaaydenh Jan 12, 2025
0befeca
chore: add delete button
jaaydenh Jan 14, 2025
f8349de
fix: update input component to 40px height
jaaydenh Jan 14, 2025
7124c11
fix: fix styles for MultiSelectCombobox
jaaydenh Jan 14, 2025
30c8a9c
chore: update export policy button design
jaaydenh Jan 14, 2025
a7c954b
fix: update copy and spacing
jaaydenh Jan 14, 2025
c09a3f3
chore: update styles
jaaydenh Jan 14, 2025
8714f5e
fix: fix storybook tests
jaaydenh Jan 14, 2025
b558f35
fix: fix legacy group mappings styles
jaaydenh Jan 14, 2025
aa0e53c
fix: format
jaaydenh Jan 14, 2025
7635856
feat: create link component
jaaydenh Jan 14, 2025
91ca380
fix: use new Link component
jaaydenh Jan 15, 2025
328ce2f
fix: use correct field value
jaaydenh Jan 15, 2025
52d563c
fix: update error handling
jaaydenh Jan 15, 2025
b1d7649
feat: add e2e tests for group and role sync
jaaydenh Jan 15, 2025
b3812f1
chore: update link components
jaaydenh Jan 15, 2025
d19a0dc
fix: remove unnecessary arbitrary values
jaaydenh Jan 16, 2025
43159ff
chore: extract Form components
jaaydenh Jan 16, 2025
528f2d8
fix: adding loading spinners
jaaydenh Jan 16, 2025
d283fa1
fix: update icon button sizing to match design
jaaydenh Jan 17, 2025
ad2fc45
chore: improve e2e tests
jaaydenh Jan 20, 2025
c99e009
fix: change to min width fit
jaaydenh Jan 20, 2025
31cd599
fix: updates for PR review comments
jaaydenh Jan 20, 2025
2354883
fix: remove async from onClick
jaaydenh Jan 22, 2025
3c63c8c
fix: fix tabs styling
jaaydenh Jan 22, 2025
623a50e
fix: format
jaaydenh Jan 22, 2025
5779c8d
fix: updates for PR comments
jaaydenh Jan 22, 2025
395fdc8
chore: switch to useId for form ids
jaaydenh Jan 22, 2025
79ce400
fix: styling
jaaydenh Jan 22, 2025
854684a
fix: add resolves
jaaydenh Jan 22, 2025
50c12db
fix: revert button styles
jaaydenh Jan 22, 2025
16665e2
chore: use Link component
jaaydenh Jan 22, 2025
3dbcee3
fix: show form errors and update mapping schema validation
jaaydenh Jan 27, 2025
9b709ae
fix: remove unnecessary async
jaaydenh Jan 27, 2025
bee5783
fix: remove useEffect
jaaydenh Jan 27, 2025
d3b64b2
fix: error display
jaaydenh Jan 27, 2025
0362c16
fix: format
jaaydenh Jan 27, 2025
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
38 changes: 37 additions & 1 deletion site/e2e/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,49 @@ export const createOrganizationSyncSettings = async () => {
"fbd2116a-8961-4954-87ae-e4575bd29ce0",
"13de3eb4-9b4f-49e7-b0f8-0c3728a0d2e2",
],
"idp-org-2": ["fbd2116a-8961-4954-87ae-e4575bd29ce0"],
"idp-org-2": ["6b39f0f1-6ad8-4981-b2fc-d52aef53ff1b"],
},
organization_assign_default: true,
});
return settings;
};

export const createGroupSyncSettings = async (orgId: string) => {
const settings = await API.patchGroupIdpSyncSettings(
{
field: "group-field-test",
mapping: {
"idp-group-1": [
"fbd2116a-8961-4954-87ae-e4575bd29ce0",
"13de3eb4-9b4f-49e7-b0f8-0c3728a0d2e2",
],
"idp-group-2": ["6b39f0f1-6ad8-4981-b2fc-d52aef53ff1b"],
},
regex_filter: "@[a-zA-Z0-9]+",
auto_create_missing_groups: true,
},
orgId,
);
return settings;
};

export const createRoleSyncSettings = async (orgId: string) => {
const settings = await API.patchRoleIdpSyncSettings(
{
field: "role-field-test",
mapping: {
"idp-role-1": [
"fbd2116a-8961-4954-87ae-e4575bd29ce0",
"13de3eb4-9b4f-49e7-b0f8-0c3728a0d2e2",
],
"idp-role-2": ["6b39f0f1-6ad8-4981-b2fc-d52aef53ff1b"],
},
},
orgId,
);
return settings;
};

export const createCustomRole = async (
orgId: string,
name: string,
Expand Down
44 changes: 31 additions & 13 deletions site/e2e/tests/deployment/idpOrgSync.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,22 @@ test.beforeEach(async ({ page }) => {
});

test.describe("IdpOrgSyncPage", () => {
test("show empty table when no org mappings are present", async ({
page,
}) => {
requiresLicense();
await page.goto("/deployment/idp-org-sync", {
waitUntil: "domcontentloaded",
});

await expect(
page.getByRole("row", { name: "idp-org-1" }),
).not.toBeVisible();
await expect(
page.getByRole("heading", { name: "No organization mappings" }),
).toBeVisible();
});

test("add new IdP organization mapping with API", async ({ page }) => {
requiresLicense();

Expand All @@ -29,14 +45,14 @@ test.describe("IdpOrgSyncPage", () => {
page.getByRole("switch", { name: "Assign Default Organization" }),
).toBeChecked();

await expect(page.getByText("idp-org-1")).toBeVisible();
await expect(page.getByRole("row", { name: "idp-org-1" })).toBeVisible();
await expect(
page.getByText("fbd2116a-8961-4954-87ae-e4575bd29ce0").first(),
page.getByRole("row", { name: "fbd2116a-8961-4954-87ae-e4575bd29ce0" }),
).toBeVisible();

await expect(page.getByText("idp-org-2")).toBeVisible();
await expect(page.getByRole("row", { name: "idp-org-2" })).toBeVisible();
await expect(
page.getByText("fbd2116a-8961-4954-87ae-e4575bd29ce0").last(),
page.getByRole("row", { name: "6b39f0f1-6ad8-4981-b2fc-d52aef53ff1b" }),
).toBeVisible();
});

Expand All @@ -47,12 +63,12 @@ test.describe("IdpOrgSyncPage", () => {
waitUntil: "domcontentloaded",
});

await expect(page.getByText("idp-org-1")).toBeVisible();
await page
.getByRole("button", { name: /delete/i })
.first()
.click();
await expect(page.getByText("idp-org-1")).not.toBeVisible();
const row = page.getByTestId("idp-org-idp-org-1");
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a test ID selector here? I'm not sure why the row selectors wouldn't work here, but would still work for other test cases

await expect(row.getByRole("cell", { name: "idp-org-1" })).toBeVisible();
await row.getByRole("button", { name: /delete/i }).click();
await expect(
row.getByRole("cell", { name: "idp-org-1" }),
).not.toBeVisible();
await expect(
page.getByText("Organization sync settings updated."),
).toBeVisible();
Expand All @@ -67,7 +83,7 @@ test.describe("IdpOrgSyncPage", () => {
const syncField = page.getByRole("textbox", {
name: "Organization sync field",
});
const saveButton = page.getByRole("button", { name: /save/i }).first();
const saveButton = page.getByRole("button", { name: /save/i });

await expect(saveButton).toBeDisabled();

Expand Down Expand Up @@ -154,8 +170,10 @@ test.describe("IdpOrgSyncPage", () => {
// Verify new mapping appears in table
const newRow = page.getByTestId("idp-org-new-idp-org");
await expect(newRow).toBeVisible();
await expect(newRow.getByText("new-idp-org")).toBeVisible();
await expect(newRow.getByText(orgName)).toBeVisible();
await expect(
newRow.getByRole("cell", { name: "new-idp-org" }),
).toBeVisible();
await expect(newRow.getByRole("cell", { name: orgName })).toBeVisible();

await expect(
page.getByText("Organization sync settings updated."),
Expand Down
184 changes: 184 additions & 0 deletions site/e2e/tests/organizations/idpGroupSync.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
import { expect, test } from "@playwright/test";
import {
createGroupSyncSettings,
createOrganizationWithName,
deleteOrganization,
setupApiCalls,
} from "../../api";
import { randomName, requiresLicense } from "../../helpers";
import { login } from "../../helpers";
import { beforeCoderTest } from "../../hooks";

test.beforeEach(async ({ page }) => {
beforeCoderTest(page);
await login(page);
await setupApiCalls(page);
});

test.describe("IdpGroupSyncPage", () => {
test("show empty table when no group mappings are present", async ({
page,
}) => {
requiresLicense();
const org = await createOrganizationWithName(randomName());
await page.goto(`/organizations/${org.name}/idp-sync?tab=groups`, {
waitUntil: "domcontentloaded",
});

await expect(
page.getByRole("row", { name: "idp-group-1" }),
).not.toBeVisible();
await expect(
page.getByRole("heading", { name: "No group mappings" }),
).toBeVisible();

await deleteOrganization(org.name);
Copy link
Member

Choose a reason for hiding this comment

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

I'm asking because I haven't had much chance to touch Playwright yet, and don't know what it does for test setup: is deleteOrganization needed for test isolation?

I'm mainly wondering why the call is present for the first two tests as (what looks like) a cleanup step, but some of the later, non-deletion tests in this file don't have it

Copy link
Member

Choose a reason for hiding this comment

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

I'm also not sure if Playwright supports any "true concurrency", rather than doing everything through the event loop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally added this to fix some test failures across tests that were creating organizations. From the documentation and my testing, it does appear tests are run in parallel. The intention was to add this cleanup step anytime an org is created to avoid test flakes in the future.

});

test("add new IdP group mapping with API", async ({ page }) => {
requiresLicense();
const org = await createOrganizationWithName(randomName());
await createGroupSyncSettings(org.id);

await page.goto(`/organizations/${org.name}/idp-sync?tab=groups`, {
waitUntil: "domcontentloaded",
});

await expect(
page.getByRole("switch", { name: "Auto create missing groups" }),
).toBeChecked();

await expect(page.getByRole("row", { name: "idp-group-1" })).toBeVisible();
await expect(
page.getByRole("row", { name: "fbd2116a-8961-4954-87ae-e4575bd29ce0" }),
).toBeVisible();

await expect(page.getByRole("row", { name: "idp-group-2" })).toBeVisible();
await expect(
page.getByRole("row", { name: "6b39f0f1-6ad8-4981-b2fc-d52aef53ff1b" }),
).toBeVisible();

await deleteOrganization(org.name);
});

test("delete a IdP group to coder group mapping row", async ({ page }) => {
requiresLicense();
const org = await createOrganizationWithName(randomName());
await createGroupSyncSettings(org.id);

await page.goto(`/organizations/${org.name}/idp-sync?tab=groups`, {
waitUntil: "domcontentloaded",
});

const row = page.getByTestId("group-idp-group-1");
await expect(row.getByRole("cell", { name: "idp-group-1" })).toBeVisible();
await row.getByRole("button", { name: /delete/i }).click();
await expect(
row.getByRole("cell", { name: "idp-group-1" }),
).not.toBeVisible();
await expect(
page.getByText("IdP Group sync settings updated."),
).toBeVisible();
});

test("update sync field", async ({ page }) => {
requiresLicense();
const org = await createOrganizationWithName(randomName());
await page.goto(`/organizations/${org.name}/idp-sync?tab=groups`, {
waitUntil: "domcontentloaded",
});

const syncField = page.getByRole("textbox", {
name: "Group sync field",
});
const saveButton = page.getByRole("button", { name: /save/i });

await expect(saveButton).toBeDisabled();

await syncField.fill("test-field");
await expect(saveButton).toBeEnabled();

await page.getByRole("button", { name: /save/i }).click();

await expect(
page.getByText("IdP Group sync settings updated."),
).toBeVisible();
});

test("toggle off auto create missing groups", async ({ page }) => {
requiresLicense();
const org = await createOrganizationWithName(randomName());
await page.goto(`/organizations/${org.name}/idp-sync?tab=groups`, {
waitUntil: "domcontentloaded",
});

const toggle = page.getByRole("switch", {
name: "Auto create missing groups",
});
await toggle.click();

await expect(
page.getByText("IdP Group sync settings updated."),
).toBeVisible();

await expect(toggle).toBeChecked();
});

test("export policy button is enabled when sync settings are present", async ({
page,
}) => {
requiresLicense();
const org = await createOrganizationWithName(randomName());
await createGroupSyncSettings(org.id);
await page.goto(`/organizations/${org.name}/idp-sync?tab=groups`, {
waitUntil: "domcontentloaded",
});

const exportButton = page.getByRole("button", { name: /Export Policy/i });
await expect(exportButton).toBeEnabled();
await exportButton.click();
});

test("add new IdP group mapping with UI", async ({ page }) => {
requiresLicense();
const orgName = randomName();
await createOrganizationWithName(orgName);

await page.goto(`/organizations/${orgName}/idp-sync?tab=groups`, {
waitUntil: "domcontentloaded",
});

const idpOrgInput = page.getByLabel("IdP group name");
const orgSelector = page.getByPlaceholder("Select group");
const addButton = page.getByRole("button", {
name: /Add IdP group/i,
});

await expect(addButton).toBeDisabled();

await idpOrgInput.fill("new-idp-group");

// Select Coder organization from combobox
await orgSelector.click();
await page.getByRole("option", { name: /Everyone/i }).click();

// Add button should now be enabled
await expect(addButton).toBeEnabled();

await addButton.click();

// Verify new mapping appears in table
const newRow = page.getByTestId("group-new-idp-group");
await expect(newRow).toBeVisible();
await expect(
newRow.getByRole("cell", { name: "new-idp-group" }),
).toBeVisible();
await expect(newRow.getByRole("cell", { name: "Everyone" })).toBeVisible();

await expect(
page.getByText("IdP Group sync settings updated."),
).toBeVisible();

await deleteOrganization(orgName);
});
});
Loading
Loading