Forráskód Böngészése

Fix autodetect buildpacks and override from app dashboard (#3690)

Co-authored-by: jose-fully-ported <141160579+jose-fully-ported@users.noreply.github.com>
Feroze Mohideen 2 éve
szülő
commit
8c43874955

+ 1 - 1
dashboard/src/lib/porter-apps/index.ts

@@ -265,7 +265,7 @@ const clientBuildFromProto = (proto?: Build): BuildOptions | undefined => {
         method: b.method,
         context: b.context,
         buildpacks: b.buildpacks.map((b) => ({
-          name: BUILDPACK_TO_NAME[b],
+          name: BUILDPACK_TO_NAME[b] ?? b,
           buildpack: b,
         })),
         builder: b.builder,

+ 1 - 4
dashboard/src/main/home/app-dashboard/app-view/AppDataContainer.tsx

@@ -13,10 +13,7 @@ import TabSelector from "components/TabSelector";
 import { useHistory } from "react-router";
 import { match } from "ts-pattern";
 import Overview from "./tabs/Overview";
-import {
-  AppValidationResult,
-  useAppValidation,
-} from "lib/hooks/useAppValidation";
+import { useAppValidation } from "lib/hooks/useAppValidation";
 import api from "shared/api";
 import { useQueryClient } from "@tanstack/react-query";
 import Settings from "./tabs/Settings";

+ 2 - 0
dashboard/src/main/home/app-dashboard/app-view/tabs/BuildSettings.tsx

@@ -23,6 +23,8 @@ const BuildSettings: React.FC = () => {
     }
 
     if (Object.keys(errors).length > 0) {
+      // TODO: remove console.log once rollout is stable
+      console.log(errors);
       return <Error message="Unable to update app" />;
     }
 

+ 4 - 4
dashboard/src/main/home/app-dashboard/create-app/RepoSettings.tsx

@@ -127,7 +127,7 @@ const RepoSettings: React.FC<Props> = ({
             label="GitHub repository:"
             width="100%"
             value={source.git_repo_name}
-            setValue={() => {}}
+            setValue={() => { }}
             placeholder=""
           />
           {!appExists && (
@@ -176,7 +176,7 @@ const RepoSettings: React.FC<Props> = ({
                 type="text"
                 width="100%"
                 value={source.git_branch}
-                setValue={() => {}}
+                setValue={() => { }}
                 placeholder=""
               />
               <BackButton
@@ -266,7 +266,7 @@ const RepoSettings: React.FC<Props> = ({
                         projectId={projectId}
                         build={b}
                         source={source}
-                        autoDetectionDisabled={appExists}
+                        populateBuildValuesOnceAfterDetection={!appExists}
                       />
                     ))
                     .exhaustive()}
@@ -345,7 +345,7 @@ const StyledAdvancedBuildSettings = styled.div`
     cursor: pointer;
     border-radius: 20px;
     transform: ${(props: { showSettings: boolean; isCurrent: boolean }) =>
-      props.showSettings ? "" : "rotate(-90deg)"};
+    props.showSettings ? "" : "rotate(-90deg)"};
   }
 `;
 

+ 1 - 25
dashboard/src/main/home/app-dashboard/validate-apply/build-settings/buildpacks/BuildpackConfigurationModal.tsx

@@ -35,7 +35,6 @@ const BuildpackConfigurationModal: React.FC<Props> = ({
   setAvailableBuildpacks,
   isDetectingBuildpacks,
   detectBuildpacksError,
-  detectAndSetBuildPacks,
 }) => {
   const { control } = useFormContext<PorterAppFormData>();
   const { append } = useFieldArray({
@@ -49,15 +48,6 @@ const BuildpackConfigurationModal: React.FC<Props> = ({
       <Spacer y={1} />
       <Scrollable>
         <Text>Builder:</Text>
-        {!build.builder && (
-          <>
-            <Spacer y={0.5} />
-            <Text color="helper">
-              No builder detected. Click 'Detect buildpacks' below to scan your
-              repository for available builders and buildpacks.
-            </Text>
-          </>
-        )}
         {!!build.builder && (
           <Controller
             control={control}
@@ -99,21 +89,7 @@ const BuildpackConfigurationModal: React.FC<Props> = ({
             append(bp);
           }}
         />
-        <Spacer y={2} />
       </Scrollable>
-      <Footer>
-        <Shade />
-        <FooterButtons>
-          <Button onClick={() => detectAndSetBuildPacks()}>
-            <Icon src={stars} height="15px" />
-            <Spacer inline x={0.5} />
-            Detect buildpacks
-          </Button>
-          <Button onClick={closeModal} width={"75px"}>
-            Close
-          </Button>
-        </FooterButtons>
-      </Footer>
     </Modal>
   );
 };
@@ -129,7 +105,7 @@ const Scrollable = styled.div`
 
 const FooterButtons = styled.div`
   display: flex;
-  justify-content: space-between;
+  justify-content: flex-end;
 `;
 
 const Footer = styled.div`

+ 7 - 1
dashboard/src/main/home/app-dashboard/validate-apply/build-settings/buildpacks/BuildpackList.tsx

@@ -9,6 +9,7 @@ import { Buildpack } from "main/home/app-dashboard/types/buildpack";
 import { useFieldArray, useFormContext } from "react-hook-form";
 import { PorterAppFormData } from "lib/porter-apps";
 import { BuildOptions } from "lib/porter-apps/build";
+import Container from "components/porter/Container";
 
 interface Props {
   build: BuildOptions & {
@@ -77,7 +78,12 @@ const BuildpackList: React.FC<Props> = ({
 
   const renderAvailableBuildpacks = () => {
     if (isDetectingBuildpacks) {
-      return <Loading />;
+      return (
+        <Container row>
+          <Text color="helper">Detecting buildpacks in your repo from path {build.context} </Text>
+          <Loading width="100px" />
+        </Container>
+      );
     }
 
     if (detectBuildpacksError) {

+ 61 - 81
dashboard/src/main/home/app-dashboard/validate-apply/build-settings/buildpacks/BuildpackSettings.tsx

@@ -27,21 +27,19 @@ type Props = {
     method: "pack";
   };
   source: SourceOptions & { type: "github" };
-  autoDetectionDisabled?: boolean;
+  populateBuildValuesOnceAfterDetection?: boolean;
 };
 
 const BuildpackSettings: React.FC<Props> = ({
   projectId,
   build,
   source,
-  autoDetectionDisabled,
+  populateBuildValuesOnceAfterDetection,
 }) => {
-  const [enableAutoDetection, setEnableAutoDetection] = useState(
-    !autoDetectionDisabled
-  );
-  const [stackOptions, setStackOptions] = useState<
+  const [builderOptions, setBuilderOptions] = useState<
     { label: string; value: string }[]
-  >([]);
+  >(build.builder ? [{ label: build.builder, value: build.builder }] : []);
+  const [populateBuild, setPopulateBuild] = useState<boolean>(populateBuildValuesOnceAfterDetection ?? false);
   const [isModalOpen, setIsModalOpen] = useState(false);
   const [availableBuildpacks, setAvailableBuildpacks] = useState<Buildpack[]>(
     []
@@ -59,6 +57,7 @@ const BuildpackSettings: React.FC<Props> = ({
       source.git_repo_name,
       source.git_branch,
       build.context,
+      isModalOpen,
     ],
     async () => {
       const detectBuildPackRes = await api.detectBuildpack<DetectedBuildpack[]>(
@@ -83,7 +82,9 @@ const BuildpackSettings: React.FC<Props> = ({
       return detectedBuildpacks;
     },
     {
-      enabled: enableAutoDetection,
+      enabled: populateBuild || isModalOpen,
+      retry: 0,
+      refetchOnWindowFocus: false,
     }
   );
 
@@ -96,84 +97,65 @@ const BuildpackSettings: React.FC<Props> = ({
   );
 
   useEffect(() => {
-    if (!enableAutoDetection) {
-      // in this case, we are not detecting buildpacks, so we just populate based on the DB
-      if (build.builder) {
-        setValue("app.build.builder", build.builder);
-        setStackOptions([{ label: build.builder, value: build.builder }]);
-      }
-      if (build.buildpacks.length) {
-        const bps = build.buildpacks.map((bp) => ({
-          name: BUILDPACK_TO_NAME[bp.buildpack] ?? bp.buildpack,
-          buildpack: bp.buildpack,
-        }));
-        replace(bps);
-      }
-    } else {
-      if (!data) {
-        return;
-      }
-
-      if (data.length === 0) {
-        return;
-      }
-      setStackOptions(
-        data
-          .flatMap((builder) => {
-            return builder.builders.map((stack) => ({
-              label: `${builder.name} - ${stack}`,
-              value: stack.toLowerCase(),
-            }));
-          })
-          .sort((a, b) => {
-            if (a.label < b.label) {
-              return -1;
-            }
-            if (a.label > b.label) {
-              return 1;
-            }
-            return 0;
-          })
-      );
-
-      const defaultBuilder =
-        data.find(
-          (builder) => builder.name.toLowerCase() === DEFAULT_BUILDER_NAME
-        ) ?? data[0];
-
-      const allBuildpacks = defaultBuilder.others.concat(
-        defaultBuilder.detected
-      );
+    if (!data || data.length === 0) {
+      return;
+    }
 
+    setBuilderOptions(
+      data
+        .flatMap((builder) => {
+          return builder.builders.map((stack) => ({
+            label: `${builder.name} - ${stack}`,
+            value: stack.toLowerCase(),
+          }));
+        })
+        .sort((a, b) => {
+          if (a.label < b.label) {
+            return -1;
+          }
+          if (a.label > b.label) {
+            return 1;
+          }
+          return 0;
+        })
+    );
+
+    const defaultBuilder =
+      data.find(
+        (builder) => builder.name.toLowerCase() === DEFAULT_BUILDER_NAME
+      ) ?? data[0];
+
+    const allBuildpacks = defaultBuilder.others.concat(
+      defaultBuilder.detected
+    );
+
+    setAvailableBuildpacks(
+      allBuildpacks.filter(
+        (bp) => !build.buildpacks.some((b) => b.buildpack === bp.buildpack)
+      )
+    );
+
+    if (populateBuild) {
       let detectedBuilder: string;
       if (
         defaultBuilder.builders.length &&
         defaultBuilder.builders.includes(DEFAULT_HEROKU_STACK)
       ) {
-        setValue("app.build.builder", DEFAULT_HEROKU_STACK);
         detectedBuilder = DEFAULT_HEROKU_STACK;
       } else {
-        setValue("app.build.builder", defaultBuilder.builders[0]);
         detectedBuilder = defaultBuilder.builders[0];
       }
 
-      if (enableAutoDetection) {
-        setValue("app.build.builder", detectedBuilder);
-        replace(
-          defaultBuilder.detected.map((bp) => ({
-            name: bp.name,
-            buildpack: bp.buildpack,
-          }))
-        );
-        setAvailableBuildpacks(defaultBuilder.others);
-      } else {
-        setValue("app.build.builder", detectedBuilder);
-        setAvailableBuildpacks(
-          allBuildpacks.filter(
-            (bp) => !build.buildpacks.some((b) => b.buildpack === bp.buildpack)
-          )
-        );
-      }
+      setValue("app.build.builder", detectedBuilder);
+      // set buildpacks as well
+      replace(
+        defaultBuilder.detected.map((bp) => ({
+          name: bp.name,
+          buildpack: bp.buildpack,
+        }))
+      );
+      // we only want to change the form values once
+      setPopulateBuild(false);
     }
   }, [data]);
 
@@ -196,31 +178,29 @@ const BuildpackSettings: React.FC<Props> = ({
           />
         </>
       )}
-      {enableAutoDetection && status === "error" && (
+      {errorMessage && (
         <>
           <Spacer y={1} />
           <Error
-            message={`Unable to detect buildpacks at path: ${build.context}. Please make sure your repo, branch, and application root path are all set correctly and attempt to detect again.`}
+            message={errorMessage}
           />
         </>
       )}
       <Spacer y={1} />
       <Button
         onClick={() => {
-          setEnableAutoDetection(true);
           setIsModalOpen(true);
         }}
       >
-        <I className="material-icons">add</I> Add / detect buildpacks
+        <I className="material-icons">add</I> Add buildpacks
       </Button>
       {isModalOpen && (
         <BuildpackConfigurationModal
           build={build}
           closeModal={() => {
-            setEnableAutoDetection(false);
             setIsModalOpen(false);
           }}
-          sortedStackOptions={stackOptions}
+          sortedStackOptions={builderOptions}
           availableBuildpacks={availableBuildpacks}
           setAvailableBuildpacks={setAvailableBuildpacks}
           isDetectingBuildpacks={status === "loading"}