Skip to content

feat(site): show version files diff based on active version #11686

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 1 commit into from
Jan 18, 2024
Merged

feat(site): show version files diff based on active version #11686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 33 additions & 0 deletions site/src/api/queries/templates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
type QueryOptions,
} from "react-query";
import { delay } from "utils/delay";
import { getTemplateVersionFiles } from "utils/templateVersion";

export const templateByNameKey = (orgId: string, name: string) => [
orgId,
Expand Down Expand Up @@ -236,6 +237,38 @@ export const resources = (versionId: string) => {
};
};

export const templateFiles = (fileId: string) => {
return {
queryKey: ["templateFiles", fileId],
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it makes sense to update the query keys for templateFiles and previousTemplateVersion to start with something like "template"? Feels like it'd be a good way to make sure we have an easy way to invalidate any template queries down the line

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we had the template ID or name I would say yes, but since they are "isolated" I'm ok to keep the current names until we see value in changing them.

queryFn: async () => {
const tarFile = await API.getFile(fileId);
return getTemplateVersionFiles(tarFile);
},
};
};

export const previousTemplateVersion = (
organizationId: string,
templateName: string,
versionName: string,
) => {
return {
queryKey: [
"templateVersion",
organizationId,
templateName,
versionName,
"previous",
],
queryFn: () =>
API.getPreviousTemplateVersionByName(
organizationId,
templateName,
versionName,
),
};
};

const waitBuildToBeFinished = async (version: TemplateVersion) => {
let data: TemplateVersion;
let jobStatus: ProvisionerJobStatus;
Expand Down
2 changes: 1 addition & 1 deletion site/src/components/TemplateFiles/TemplateFiles.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const meta: Meta<typeof TemplateFiles> = {
component: TemplateFiles,
args: {
currentFiles: exampleFiles,
previousFiles: exampleFiles,
baseFiles: exampleFiles,
tab: { value: "0", set: action("change tab") },
},
};
Expand Down
40 changes: 32 additions & 8 deletions site/src/components/TemplateFiles/TemplateFiles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { type Interpolation, type Theme } from "@emotion/react";
import { type FC } from "react";
import { useEffect, type FC } from "react";
import { DockerIcon } from "components/Icons/DockerIcon";
import { MarkdownIcon } from "components/Icons/MarkdownIcon";
import { TerraformIcon } from "components/Icons/TerraformIcon";
import { SyntaxHighlighter } from "components/SyntaxHighlighter/SyntaxHighlighter";
import { UseTabResult } from "hooks/useTab";
import { UseTabResult, useTab } from "hooks/useTab";
import { AllowedExtension, TemplateVersionFiles } from "utils/templateVersion";
import InsertDriveFileOutlined from "@mui/icons-material/InsertDriveFileOutlined";

Expand Down Expand Up @@ -39,19 +39,22 @@ const languageByExtension: Record<AllowedExtension, string> = {

interface TemplateFilesProps {
currentFiles: TemplateVersionFiles;
previousFiles?: TemplateVersionFiles;
/**
* Files used to compare with current files
*/
baseFiles?: TemplateVersionFiles;
tab: UseTabResult;
}

export const TemplateFiles: FC<TemplateFilesProps> = ({
currentFiles,
previousFiles,
baseFiles,
tab,
}) => {
const filenames = Object.keys(currentFiles);
const selectedFilename = filenames[Number(tab.value)];
const currentFile = currentFiles[selectedFilename];
const previousFile = previousFiles && previousFiles[selectedFilename];
const previousFile = baseFiles && baseFiles[selectedFilename];
Copy link
Member

Choose a reason for hiding this comment

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

Not a call out to your code, but there's some weird behavior here (based around undefined) that I'm guessing that the compiler isn't complaining about right now, because we don't have it set strictly enough.

Here's a TypeScript snippet with the compiler tightened up. I'm going to be bringing this up at the Frontend Variety later today

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think we should set this rule as true: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-1.html#checked-indexed-accesses---nouncheckedindexedaccess


return (
<div css={styles.files}>
Expand All @@ -61,9 +64,9 @@ export const TemplateFiles: FC<TemplateFilesProps> = ({
const extension = getExtension(filename) as AllowedExtension;
const icon = iconByExtension[extension];
const hasDiff =
previousFiles &&
previousFiles[filename] &&
currentFiles[filename] !== previousFiles[filename];
baseFiles &&
baseFiles[filename] &&
currentFiles[filename] !== baseFiles[filename];

return (
<button
Expand Down Expand Up @@ -93,6 +96,27 @@ export const TemplateFiles: FC<TemplateFilesProps> = ({
</div>
);
};

export const useFileTab = (templateFiles: TemplateVersionFiles | undefined) => {
// Tabs The default tab is the tab that has main.tf but until we loads the
// files and check if main.tf exists we don't know which tab is the default
// one so we just use empty string
const tab = useTab("file", "");
const isLoaded = tab.value !== "";
useEffect(() => {
if (templateFiles && !isLoaded) {
const terraformFileIndex = Object.keys(templateFiles).indexOf("main.tf");
// If main.tf exists use the index if not just use the first tab
tab.set(terraformFileIndex !== -1 ? terraformFileIndex.toString() : "0");
}
}, [isLoaded, tab, templateFiles]);

return {
...tab,
isLoaded,
};
};

const styles = {
tabs: (theme) => ({
display: "flex",
Expand Down
68 changes: 0 additions & 68 deletions site/src/components/TemplateFiles/hooks.ts
View file
Open in desktop

This file was deleted.

30 changes: 21 additions & 9 deletions site/src/pages/TemplatePage/TemplateFilesPage/TemplateFilesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,41 @@
import { type FC } from "react";
import { Helmet } from "react-helmet-async";
import { Loader } from "components/Loader/Loader";
import { TemplateFiles } from "components/TemplateFiles/TemplateFiles";
import { useFileTab, useTemplateFiles } from "components/TemplateFiles/hooks";
import {
TemplateFiles,
useFileTab,
} from "components/TemplateFiles/TemplateFiles";
import { useTemplateLayoutContext } from "pages/TemplatePage/TemplateLayout";
import { getTemplatePageTitle } from "../utils";
import { useQuery } from "react-query";
import { previousTemplateVersion, templateFiles } from "api/queries/templates";
import { useOrganizationId } from "hooks";

const TemplateFilesPage: FC = () => {
const orgId = useOrganizationId();
const { template, activeVersion } = useTemplateLayoutContext();
const { data: templateFiles } = useTemplateFiles(
template.name,
activeVersion,
const { data: currentFiles } = useQuery(
templateFiles(activeVersion.job.file_id),
);
const tab = useFileTab(templateFiles?.currentFiles);
const { data: previousTemplate } = useQuery(
previousTemplateVersion(orgId, template.name, activeVersion.name),
);
const { data: previousFiles } = useQuery({
...templateFiles(previousTemplate?.job.file_id ?? ""),
enabled: Boolean(previousTemplate),
});
const tab = useFileTab(currentFiles);

return (
<>
<Helmet>
<title>{getTemplatePageTitle("Source Code", template)}</title>
</Helmet>

{templateFiles && tab.isLoaded ? (
{previousFiles && currentFiles && tab.isLoaded ? (
<TemplateFiles
currentFiles={templateFiles.currentFiles}
previousFiles={templateFiles.previousFiles}
currentFiles={currentFiles}
baseFiles={previousFiles}
tab={tab}
/>
) : (
Expand Down
51 changes: 38 additions & 13 deletions site/src/pages/TemplateVersionPage/TemplateVersionPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@ import { useParams } from "react-router-dom";
import { pageTitle } from "utils/page";
import TemplateVersionPageView from "./TemplateVersionPageView";
import { useQuery } from "react-query";
import { templateVersionByName } from "api/queries/templates";
import { useFileTab, useTemplateFiles } from "components/TemplateFiles/hooks";
import {
templateByName,
templateFiles,
templateVersion,
templateVersionByName,
} from "api/queries/templates";
import { useFileTab } from "components/TemplateFiles/TemplateFiles";

type Params = {
version: string;
Expand All @@ -18,16 +23,30 @@ export const TemplateVersionPage: FC = () => {
const { version: versionName, template: templateName } =
useParams() as Params;
const orgId = useOrganizationId();
const templateVersionQuery = useQuery(

/**
* Template version files
*/
const templateQuery = useQuery(templateByName(orgId, templateName));
const selectedVersionQuery = useQuery(
templateVersionByName(orgId, templateName, versionName),
);
const { data: templateFiles, error: templateFilesError } = useTemplateFiles(
templateName,
templateVersionQuery.data,
);
const tab = useFileTab(templateFiles?.currentFiles);
const selectedVersionFilesQuery = useQuery({
...templateFiles(selectedVersionQuery.data?.job.file_id ?? ""),
enabled: Boolean(selectedVersionQuery.data),
});
const activeVersionQuery = useQuery({
...templateVersion(templateQuery.data?.active_version_id ?? ""),
enabled: Boolean(templateQuery.data),
});
const activeVersionFilesQuery = useQuery({
...templateFiles(activeVersionQuery.data?.job.file_id ?? ""),
enabled: Boolean(activeVersionQuery.data),
});
const tab = useFileTab(selectedVersionFilesQuery.data);

const permissions = usePermissions();
const versionId = templateVersionQuery.data?.id;
const versionId = selectedVersionQuery.data?.id;
const createWorkspaceUrl = useMemo(() => {
const params = new URLSearchParams();
if (versionId) {
Expand All @@ -44,10 +63,16 @@ export const TemplateVersionPage: FC = () => {
</Helmet>

<TemplateVersionPageView
error={templateVersionQuery.error || templateFilesError}
currentVersion={templateVersionQuery.data}
currentFiles={templateFiles?.currentFiles}
previousFiles={templateFiles?.previousFiles}
error={
templateQuery.error ||
selectedVersionQuery.error ||
selectedVersionFilesQuery.error ||
activeVersionQuery.error ||
activeVersionFilesQuery.error
}
currentVersion={selectedVersionQuery.data}
currentFiles={selectedVersionFilesQuery.data}
baseFiles={activeVersionFilesQuery.data}
versionName={versionName}
templateName={templateName}
tab={tab}
Expand Down
2 changes: 1 addition & 1 deletion site/src/pages/TemplateVersionPage/TemplateVersionPageView.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const defaultArgs: TemplateVersionPageViewProps = {
"some.tpl": `{{.Name}}`,
"some.sh": `echo "Hello world"`,
},
previousFiles: undefined,
baseFiles: undefined,
error: undefined,
};

Expand Down
6 changes: 3 additions & 3 deletions site/src/pages/TemplateVersionPage/TemplateVersionPageView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export interface TemplateVersionPageViewProps {
error: unknown;
currentVersion: TemplateVersion | undefined;
currentFiles: TemplateVersionFiles | undefined;
previousFiles: TemplateVersionFiles | undefined;
baseFiles: TemplateVersionFiles | undefined;
}

export const TemplateVersionPageView: FC<TemplateVersionPageViewProps> = ({
Expand All @@ -38,7 +38,7 @@ export const TemplateVersionPageView: FC<TemplateVersionPageViewProps> = ({
createWorkspaceUrl,
currentVersion,
currentFiles,
previousFiles,
baseFiles,
error,
}) => {
return (
Expand Down Expand Up @@ -103,7 +103,7 @@ export const TemplateVersionPageView: FC<TemplateVersionPageViewProps> = ({
<TemplateFiles
tab={tab}
currentFiles={currentFiles}
previousFiles={previousFiles}
baseFiles={baseFiles}
/>
</>
)}
Expand Down
4 changes: 1 addition & 3 deletions site/src/utils/templateVersion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import * as API from "api/api";
import { FileTree, createFile } from "./filetree";
import { TarReader } from "./tar";

// Content by filename
export type TemplateVersionFiles = Record<string, string>;

export const getTemplateVersionFiles = async (
fileId: string,
tarFile: ArrayBuffer,
): Promise<TemplateVersionFiles> => {
const files: TemplateVersionFiles = {};
const tarFile = await API.getFile(fileId);
const tarReader = new TarReader();
await tarReader.readFile(tarFile);
for (const file of tarReader.fileInfo) {
Expand Down