Skip to content

feat: integrate backend with idp sync page #14755

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 13 commits into from
Sep 24, 2024
Merged

feat: integrate backend with idp sync page #14755

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a40ba7d
feat: idp sync initial commit
jaaydenh Sep 17, 2024
7ff9a07
fix: hookup backend data for groups and roles
jaaydenh Sep 20, 2024
6e303f4
chore: cleanup
jaaydenh Sep 20, 2024
310726b
feat: separate groups and roles into tabs
jaaydenh Sep 21, 2024
615c8d3
feat: implement export policy button
jaaydenh Sep 22, 2024
45f4d13
feat: handle missing groups
jaaydenh Sep 22, 2024
080b032
chore: add story for missing groups
jaaydenh Sep 22, 2024
8e1e021
chore: add stories for export policy button
jaaydenh Sep 22, 2024
c863619
fix: updates for PR review
jaaydenh Sep 23, 2024
41631f0
chore: update tests
jaaydenh Sep 24, 2024
3db9f48
chore: document uuid regex
jaaydenh Sep 24, 2024
431beea
chore: remove unused
jaaydenh Sep 24, 2024
f166784
fix: fix stories
jaaydenh Sep 24, 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
Prev Previous commit
Next Next commit
chore: add stories for export policy button
  • Loading branch information
@jaaydenh
jaaydenh committed Sep 22, 2024
commit 8e1e021c2ab7634d146e6c0535ae234350f3068e
69 changes: 69 additions & 0 deletions site/src/pages/ManagementSettingsPage/IdpSyncPage/ExportPolicyButton.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import type { Meta, StoryObj } from "@storybook/react";
import { expect, fn, userEvent, waitFor, within } from "@storybook/test";
import {
MockGroupSyncSettings,
MockOrganization,
MockRoleSyncSettings,
} from "testHelpers/entities";
import { ExportPolicyButton } from "./ExportPolicyButton";

const meta: Meta<typeof ExportPolicyButton> = {
title: "modules/resources/ExportPolicyButton",
component: ExportPolicyButton,
args: {
policy: JSON.stringify(MockGroupSyncSettings, null, 2),
type: "groups",
organization: MockOrganization,
},
};

export default meta;
type Story = StoryObj<typeof ExportPolicyButton>;

export const Default: Story = {};

export const ClickExportGroupPolicy: Story = {
args: {
policy: JSON.stringify(MockGroupSyncSettings, null, 2),
type: "groups",
organization: MockOrganization,
download: fn(),
},
play: async ({ canvasElement, args }) => {
const canvas = within(canvasElement);
await userEvent.click(
canvas.getByRole("button", { name: "Export Policy" }),
);
await waitFor(() =>
expect(args.download).toHaveBeenCalledWith(
expect.anything(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expect.anything(),
expect.any(Blob)

`${MockOrganization.name}_groups-policy.json`,
),
);
const blob: Blob = (args.download as jest.Mock).mock.calls[0][0];
Copy link
Member

Choose a reason for hiding this comment

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

oh this is so gross (but I don't know what to suggest to make it better)

could you add a comment above it to elaborate why this incantation is justified/necessary? I think I understand what it's doing, but clarity is good, and more junior devs might not get it!

await expect(blob.type).toEqual("application/json");
Copy link
Member

Choose a reason for hiding this comment

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

Can we also assert that the blob contents match the MockGroupSyncSettings value?

},
};

export const ClickExportRolePolicy: Story = {
args: {
policy: JSON.stringify(MockRoleSyncSettings, null, 2),
type: "roles",
organization: MockOrganization,
download: fn(),
},
play: async ({ canvasElement, args }) => {
const canvas = within(canvasElement);
await userEvent.click(
canvas.getByRole("button", { name: "Export Policy" }),
);
await waitFor(() =>
expect(args.download).toHaveBeenCalledWith(
expect.anything(),
`${MockOrganization.name}_roles-policy.json`,
),
);
const blob: Blob = (args.download as jest.Mock).mock.calls[0][0];
await expect(blob.type).toEqual("application/json");
Copy link
Member

Choose a reason for hiding this comment

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

Same comments for the group story apply here

},
};
4 changes: 3 additions & 1 deletion site/src/pages/ManagementSettingsPage/IdpSyncPage/ExportPolicyButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ interface DownloadPolicyButtonProps {
policy: string | null;
Copy link
Member

Choose a reason for hiding this comment

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

Is there any information on how policies are defined? I'm seeing that we're downloading just the policy itself when the user clicks the button

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The policy is meant to be a json representation of GroupSyncSettings and RoleSyncSettings so the goal was to make ExportPolicyButton work for both types

type: "groups" | "roles";
organization: Organization;
download?: (file: Blob, filename: string) => void;
}

export const ExportPolicyButton: FC<DownloadPolicyButtonProps> = ({
policy,
type,
organization,
download = saveAs,
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how big of a deal this is, but the current implementation means that we can't add additional functionality without completely overriding the default saveAs function. Every consumer would have to import saveAs themselves, and then wire up the functionality for component implementation

How often would we have a use case where we'd want to get rid of the saveAs functionality vs wanting to always have saveAs, and then potentially add more functionality on top?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was meant to just support the Storybook test case. Is there a better way to handle this?

}) => {
const [isDownloading, setIsDownloading] = useState(false);

Expand All @@ -29,7 +31,7 @@ export const ExportPolicyButton: FC<DownloadPolicyButtonProps> = ({
const file = new Blob([policy], {
type: "application/json",
});
saveAs(file, `${organization.name}_${type}-policy.json`);
download(file, `${organization.name}_${type}-policy.json`);
} catch (e) {
console.error(e);
displayError("Failed to export policy json");
Expand Down
Loading