From 25e2c8055c2f339a2ec495721a9b2232356e6a0f Mon Sep 17 00:00:00 2001
From: Anbraten <anton@ju60.de>
Date: Sat, 18 Mar 2023 21:49:12 +0100
Subject: [PATCH] UI improvements (#1640)

---
 .../layout/popups/DeployPipelinePopup.vue     |   2 +-
 web/src/components/layout/scaffold/Header.vue |  24 +-
 .../repo/pipeline/PipelineStepList.vue        |   5 +
 web/src/lib/api/index.ts                      |   2 +-
 web/src/store/pipelines.ts                    |   5 +-
 web/src/views/repo/RepoBranch.vue             |   4 +-
 web/src/views/repo/pipeline/Pipeline.vue      | 176 ++++++-------
 .../views/repo/pipeline/PipelineWrapper.vue   | 233 ++++++++----------
 8 files changed, 199 insertions(+), 252 deletions(-)

diff --git a/web/src/components/layout/popups/DeployPipelinePopup.vue b/web/src/components/layout/popups/DeployPipelinePopup.vue
index 04198596d..8a52dea9c 100644
--- a/web/src/components/layout/popups/DeployPipelinePopup.vue
+++ b/web/src/components/layout/popups/DeployPipelinePopup.vue
@@ -58,7 +58,7 @@ import { inject } from '~/compositions/useInjectProvide';
 
 const props = defineProps<{
   open: boolean;
-  pipelineNumber: number;
+  pipelineNumber: string;
 }>();
 
 const emit = defineEmits<{
diff --git a/web/src/components/layout/scaffold/Header.vue b/web/src/components/layout/scaffold/Header.vue
index 9c256fb95..ce99e15ea 100644
--- a/web/src/components/layout/scaffold/Header.vue
+++ b/web/src/components/layout/scaffold/Header.vue
@@ -1,15 +1,21 @@
 <template>
   <header class="bg-white dark:bg-dark-gray-900 border-b dark:border-gray-700 text-color">
     <FluidContainer class="!py-0">
-      <div class="flex flex-wrap items-center justify-between py-4 <md:flex-row <md:gap-y-4">
+      <div class="flex w-full items-center justify-between py-4 <md:flex-row <md:gap-y-4">
         <div
-          class="flex flex-wrap items-center justify-start <md:w-full <md:justify-center"
+          class="flex items-center min-w-0 justify-start <md:justify-center"
           :class="{
             'md:flex-1': searchBoxPresent,
           }"
         >
-          <IconButton v-if="goBack" icon="back" :title="$t('back')" class="mr-2 <md:hidden" @click="goBack" />
-          <h1 class="flex flex-wrap text-xl text-color items-center gap-x-2">
+          <IconButton
+            v-if="goBack"
+            icon="back"
+            :title="$t('back')"
+            class="flex-shrink-0 mr-2 <md:hidden"
+            @click="goBack"
+          />
+          <h1 class="flex text-xl min-w-0 text-color items-center gap-x-2">
             <slot name="title" />
           </h1>
         </div>
@@ -22,7 +28,7 @@
         />
         <div
           v-if="$slots.titleActions"
-          class="flex flex-wrap items-center justify-end gap-x-2 <md:w-full <md:justify-center"
+          class="flex items-center justify-end gap-x-2 <md:w-full <md:justify-center"
           :class="{
             'md:flex-1': searchBoxPresent,
           }"
@@ -31,7 +37,7 @@
         </div>
       </div>
 
-      <div v-if="enableTabs" class="flex flex-wrap justify-between">
+      <div v-if="enableTabs" class="flex justify-between">
         <Tabs class="<md:order-2" />
         <div
           v-if="$slots.titleActions"
@@ -50,13 +56,11 @@ import FluidContainer from '~/components/layout/FluidContainer.vue';
 
 import Tabs from './Tabs.vue';
 
-export interface Props {
+const props = defineProps<{
   goBack?: () => void;
   enableTabs?: boolean;
   search?: string;
-}
-
-const props = defineProps<Props>();
+}>();
 defineEmits(['update:search']);
 
 const searchBoxPresent = props.search !== undefined;
diff --git a/web/src/components/repo/pipeline/PipelineStepList.vue b/web/src/components/repo/pipeline/PipelineStepList.vue
index 948e9b830..8a8d424aa 100644
--- a/web/src/components/repo/pipeline/PipelineStepList.vue
+++ b/web/src/components/repo/pipeline/PipelineStepList.vue
@@ -69,6 +69,11 @@
               />
               <PipelineStatusIcon :status="workflow.state" class="!h-4 !w-4" />
               <span class="truncate">{{ workflow.name }}</span>
+              <PipelineStepDuration
+                v-if="workflow.start_time !== workflow.end_time"
+                :step="workflow"
+                class="mr-1 pr-2px"
+              />
             </button>
           </div>
           <div
diff --git a/web/src/lib/api/index.ts b/web/src/lib/api/index.ts
index 26dfeace9..4cc2f65b5 100644
--- a/web/src/lib/api/index.ts
+++ b/web/src/lib/api/index.ts
@@ -73,7 +73,7 @@ export default class WoodpeckerClient extends ApiClient {
 
   // Deploy triggers a deployment for an existing pipeline using the
   // specified target environment.
-  deployPipeline(owner: string, repo: string, pipelineNumber: number, options: DeploymentOptions): Promise<Pipeline> {
+  deployPipeline(owner: string, repo: string, pipelineNumber: string, options: DeploymentOptions): Promise<Pipeline> {
     const vars = {
       ...options.variables,
       event: 'deployment',
diff --git a/web/src/store/pipelines.ts b/web/src/store/pipelines.ts
index 75b692569..7e9137f5f 100644
--- a/web/src/store/pipelines.ts
+++ b/web/src/store/pipelines.ts
@@ -15,7 +15,10 @@ export const usePipelineStore = defineStore('pipelines', () => {
   function setPipeline(owner: string, repo: string, pipeline: Pipeline) {
     const _repoSlug = repoSlug(owner, repo);
     const repoPipelines = pipelines.get(_repoSlug) || new Map();
-    repoPipelines.set(pipeline.number, { ...(repoPipelines.get(pipeline.number) || {}), ...pipeline });
+    repoPipelines.set(pipeline.number, {
+      ...(repoPipelines.get(pipeline.number) || {}),
+      ...pipeline,
+    });
     pipelines.set(_repoSlug, repoPipelines);
   }
 
diff --git a/web/src/views/repo/RepoBranch.vue b/web/src/views/repo/RepoBranch.vue
index 6153b3e0e..46f19d965 100644
--- a/web/src/views/repo/RepoBranch.vue
+++ b/web/src/views/repo/RepoBranch.vue
@@ -32,7 +32,9 @@ export default defineComponent({
     }
 
     const allPipelines = inject<Ref<Pipeline[]>>('pipelines');
-    const pipelines = computed(() => allPipelines?.value.filter((b) => b.branch === branch.value));
+    const pipelines = computed(() =>
+      allPipelines?.value.filter((b) => b.branch === branch.value && b.event !== 'pull_request'),
+    );
 
     return { pipelines, repo };
   },
diff --git a/web/src/views/repo/pipeline/Pipeline.vue b/web/src/views/repo/pipeline/Pipeline.vue
index 8452e11e5..18c8f4a4e 100644
--- a/web/src/views/repo/pipeline/Pipeline.vue
+++ b/web/src/views/repo/pipeline/Pipeline.vue
@@ -15,8 +15,8 @@
         </div>
 
         <div v-else-if="pipeline.status === 'blocked'" class="flex flex-col flex-grow justify-center items-center p-2">
-          <Icon name="status-blocked" class="w-32 h-32 text-color" />
-          <p class="text-xl text-color">{{ $t('repo.pipeline.protected.awaits') }}</p>
+          <Icon name="status-blocked" class="w-16 h-16 text-color mb-4" />
+          <p class="text-xl text-color mb-4">{{ $t('repo.pipeline.protected.awaits') }}</p>
           <div v-if="repoPermissions.push" class="flex mt-2 space-x-4">
             <Button
               color="green"
@@ -49,8 +49,8 @@
   </FluidContainer>
 </template>
 
-<script lang="ts">
-import { computed, defineComponent, inject, PropType, Ref, toRef } from 'vue';
+<script lang="ts" setup>
+import { computed, inject, Ref, toRef } from 'vue';
 import { useI18n } from 'vue-i18n';
 import { useRoute, useRouter } from 'vue-router';
 
@@ -65,112 +65,84 @@ import useNotifications from '~/compositions/useNotifications';
 import { Pipeline, PipelineStep, Repo, RepoPermissions } from '~/lib/api/types';
 import { findStep } from '~/utils/helpers';
 
-export default defineComponent({
-  name: 'Pipeline',
+const props = defineProps<{
+  stepId?: string | null;
+}>();
 
-  components: {
-    Button,
-    PipelineStepList,
-    Icon,
-    PipelineLog,
-    FluidContainer,
-  },
+const apiClient = useApiClient();
+const router = useRouter();
+const route = useRoute();
+const notifications = useNotifications();
+const i18n = useI18n();
 
-  props: {
-    stepId: {
-      type: String as PropType<string | null>,
-      default: null,
-    },
-  },
+const pipeline = inject<Ref<Pipeline>>('pipeline');
+const repo = inject<Ref<Repo>>('repo');
+const repoPermissions = inject<Ref<RepoPermissions>>('repo-permissions');
+if (!repo || !repoPermissions || !pipeline) {
+  throw new Error('Unexpected: "repo", "repoPermissions" & "pipeline" should be provided at this place');
+}
 
-  setup(props) {
-    const apiClient = useApiClient();
-    const router = useRouter();
-    const route = useRoute();
-    const notifications = useNotifications();
-    const i18n = useI18n();
+const stepId = toRef(props, 'stepId');
 
-    const pipeline = inject<Ref<Pipeline>>('pipeline');
-    const repo = inject<Ref<Repo>>('repo');
-    const repoPermissions = inject<Ref<RepoPermissions>>('repo-permissions');
-    if (!repo || !repoPermissions || !pipeline) {
-      throw new Error('Unexpected: "repo", "repoPermissions" & "pipeline" should be provided at this place');
+const defaultStepId = computed(() => {
+  if (!pipeline.value || !pipeline.value.steps || !pipeline.value.steps[0].children) {
+    return null;
+  }
+
+  return pipeline.value.steps[0].children[0].pid;
+});
+
+const selectedStepId = computed({
+  get() {
+    if (stepId.value !== '' && stepId.value !== null && stepId.value !== undefined) {
+      const id = parseInt(stepId.value, 10);
+      const step = pipeline.value?.steps?.reduce(
+        (prev, p) => prev || p.children?.find((c) => c.pid === id),
+        undefined as PipelineStep | undefined,
+      );
+      if (step) {
+        return step.pid;
+      }
+
+      // return fallback if step-id is provided, but step can not be found
+      return defaultStepId.value;
     }
 
-    const stepId = toRef(props, 'stepId');
+    // is opened on >= md-screen
+    if (window.innerWidth > 768) {
+      return defaultStepId.value;
+    }
 
-    const defaultStepId = computed(() => {
-      if (!pipeline.value || !pipeline.value.steps || !pipeline.value.steps[0].children) {
-        return null;
-      }
+    return null;
+  },
+  set(_selectedStepId: number | null) {
+    if (!_selectedStepId) {
+      router.replace({ params: { ...route.params, stepId: '' } });
+      return;
+    }
 
-      return pipeline.value.steps[0].children[0].pid;
-    });
-
-    const selectedStepId = computed({
-      get() {
-        if (stepId.value !== '' && stepId.value !== null) {
-          const id = parseInt(stepId.value, 10);
-          const step = pipeline.value?.steps?.reduce(
-            (prev, p) => prev || p.children?.find((c) => c.pid === id),
-            undefined as PipelineStep | undefined,
-          );
-          if (step) {
-            return step.pid;
-          }
-
-          // return fallback if step-id is provided, but step can not be found
-          return defaultStepId.value;
-        }
-
-        // is opened on >= md-screen
-        if (window.innerWidth > 768) {
-          return defaultStepId.value;
-        }
-
-        return null;
-      },
-      set(_selectedStepId: number | null) {
-        if (!_selectedStepId) {
-          router.replace({ params: { ...route.params, stepId: '' } });
-          return;
-        }
-
-        router.replace({ params: { ...route.params, stepId: `${_selectedStepId}` } });
-      },
-    });
-
-    const selectedStep = computed(() => findStep(pipeline.value.steps || [], selectedStepId.value || -1));
-    const error = computed(() => pipeline.value?.error || selectedStep.value?.error);
-
-    const { doSubmit: approvePipeline, isLoading: isApprovingPipeline } = useAsyncAction(async () => {
-      if (!repo) {
-        throw new Error('Unexpected: Repo is undefined');
-      }
-
-      await apiClient.approvePipeline(repo.value.owner, repo.value.name, `${pipeline.value.number}`);
-      notifications.notify({ title: i18n.t('repo.pipeline.protected.approve_success'), type: 'success' });
-    });
-
-    const { doSubmit: declinePipeline, isLoading: isDecliningPipeline } = useAsyncAction(async () => {
-      if (!repo) {
-        throw new Error('Unexpected: Repo is undefined');
-      }
-
-      await apiClient.declinePipeline(repo.value.owner, repo.value.name, `${pipeline.value.number}`);
-      notifications.notify({ title: i18n.t('repo.pipeline.protected.decline_success'), type: 'success' });
-    });
-
-    return {
-      repoPermissions,
-      selectedStepId,
-      pipeline,
-      error,
-      isApprovingPipeline,
-      isDecliningPipeline,
-      approvePipeline,
-      declinePipeline,
-    };
+    router.replace({ params: { ...route.params, stepId: `${_selectedStepId}` } });
   },
 });
+
+const selectedStep = computed(() => findStep(pipeline.value.steps || [], selectedStepId.value || -1));
+const error = computed(() => pipeline.value?.error || selectedStep.value?.error);
+
+const { doSubmit: approvePipeline, isLoading: isApprovingPipeline } = useAsyncAction(async () => {
+  if (!repo) {
+    throw new Error('Unexpected: Repo is undefined');
+  }
+
+  await apiClient.approvePipeline(repo.value.owner, repo.value.name, `${pipeline.value.number}`);
+  notifications.notify({ title: i18n.t('repo.pipeline.protected.approve_success'), type: 'success' });
+});
+
+const { doSubmit: declinePipeline, isLoading: isDecliningPipeline } = useAsyncAction(async () => {
+  if (!repo) {
+    throw new Error('Unexpected: Repo is undefined');
+  }
+
+  await apiClient.declinePipeline(repo.value.owner, repo.value.name, `${pipeline.value.number}`);
+  notifications.notify({ title: i18n.t('repo.pipeline.protected.decline_success'), type: 'success' });
+});
 </script>
diff --git a/web/src/views/repo/pipeline/PipelineWrapper.vue b/web/src/views/repo/pipeline/PipelineWrapper.vue
index 150afe6a0..bc296f3df 100644
--- a/web/src/views/repo/pipeline/PipelineWrapper.vue
+++ b/web/src/views/repo/pipeline/PipelineWrapper.vue
@@ -8,9 +8,9 @@
       :fluid-content="activeTab !== 'tasks'"
     >
       <template #title>
-        <span class="w-full md:w-auto text-center">{{ $t('repo.pipeline.pipeline', { pipelineId }) }}</span>
+        <span class="flex-shrink-0 text-center">{{ $t('repo.pipeline.pipeline', { pipelineId }) }}</span>
         <span class="<md:hidden">-</span>
-        <span class="w-full md:w-auto text-center truncate">{{ message }}</span>
+        <span class="text-center truncate">{{ message }}</span>
       </template>
 
       <template #titleActions>
@@ -75,13 +75,14 @@
   </template>
 </template>
 
-<script lang="ts">
+<script lang="ts" setup>
 import { Tooltip } from 'floating-vue';
-import { computed, defineComponent, inject, onBeforeUnmount, onMounted, provide, Ref, ref, toRef, watch } from 'vue';
+import { computed, inject, onBeforeUnmount, onMounted, provide, Ref, ref, toRef, watch } from 'vue';
 import { useI18n } from 'vue-i18n';
 import { useRoute, useRouter } from 'vue-router';
 
 import Button from '~/components/atomic/Button.vue';
+import DeployPipelinePopup from '~/components/layout/popups/DeployPipelinePopup.vue';
 import Scaffold from '~/components/layout/scaffold/Scaffold.vue';
 import Tab from '~/components/layout/scaffold/Tab.vue';
 import PipelineStatusIcon from '~/components/repo/pipeline/PipelineStatusIcon.vue';
@@ -94,150 +95,110 @@ import { useRouteBackOrDefault } from '~/compositions/useRouteBackOrDefault';
 import { Repo, RepoPermissions } from '~/lib/api/types';
 import { usePipelineStore } from '~/store/pipelines';
 
-export default defineComponent({
-  name: 'PipelineWrapper',
+const props = defineProps<{
+  repoOwner: string;
+  repoName: string;
+  pipelineId: string;
+}>();
 
-  components: {
-    Button,
-    PipelineStatusIcon,
-    Tab,
-    Tooltip,
-    Scaffold,
-  },
+const apiClient = useApiClient();
+const route = useRoute();
+const router = useRouter();
+const notifications = useNotifications();
+const favicon = useFavicon();
+const i18n = useI18n();
 
-  props: {
-    repoOwner: {
-      type: String,
-      required: true,
-    },
+const pipelineStore = usePipelineStore();
+const pipelineId = toRef(props, 'pipelineId');
+const repoOwner = toRef(props, 'repoOwner');
+const repoName = toRef(props, 'repoName');
+const repo = inject<Ref<Repo>>('repo');
+const repoPermissions = inject<Ref<RepoPermissions>>('repo-permissions');
+if (!repo || !repoPermissions) {
+  throw new Error('Unexpected: "repo" & "repoPermissions" should be provided at this place');
+}
 
-    repoName: {
-      type: String,
-      required: true,
-    },
+const pipeline = pipelineStore.getPipeline(repoOwner, repoName, pipelineId);
+const { since, duration, created } = usePipeline(pipeline);
+provide('pipeline', pipeline);
 
-    pipelineId: {
-      type: String,
-      required: true,
-    },
-  },
+const { message } = usePipeline(pipeline);
 
-  setup(props) {
-    const apiClient = useApiClient();
-    const route = useRoute();
-    const router = useRouter();
-    const notifications = useNotifications();
-    const favicon = useFavicon();
-    const i18n = useI18n();
+const showDeployPipelinePopup = ref(false);
 
-    const pipelineStore = usePipelineStore();
-    const pipelineId = toRef(props, 'pipelineId');
-    const repoOwner = toRef(props, 'repoOwner');
-    const repoName = toRef(props, 'repoName');
-    const repo = inject<Ref<Repo>>('repo');
-    const repoPermissions = inject<Ref<RepoPermissions>>('repo-permissions');
-    if (!repo || !repoPermissions) {
-      throw new Error('Unexpected: "repo" & "repoPermissions" should be provided at this place');
+async function loadPipeline(): Promise<void> {
+  if (!repo) {
+    throw new Error('Unexpected: Repo is undefined');
+  }
+
+  await pipelineStore.loadPipeline(repo.value.owner, repo.value.name, parseInt(pipelineId.value, 10));
+
+  favicon.updateStatus(pipeline.value?.status);
+}
+
+const { doSubmit: cancelPipeline, isLoading: isCancelingPipeline } = useAsyncAction(async () => {
+  if (!repo) {
+    throw new Error('Unexpected: Repo is undefined');
+  }
+
+  if (!pipeline.value?.steps) {
+    throw new Error('Unexpected: Pipeline steps not loaded');
+  }
+
+  // TODO: is selectedStepId right?
+  // const step = findStep(pipeline.value.steps, selectedStepId.value || 2);
+
+  // if (!step) {
+  //   throw new Error('Unexpected: Step not found');
+  // }
+
+  await apiClient.cancelPipeline(repo.value.owner, repo.value.name, parseInt(pipelineId.value, 10));
+  notifications.notify({ title: i18n.t('repo.pipeline.actions.cancel_success'), type: 'success' });
+});
+
+const { doSubmit: restartPipeline, isLoading: isRestartingPipeline } = useAsyncAction(async () => {
+  if (!repo) {
+    throw new Error('Unexpected: Repo is undefined');
+  }
+
+  await apiClient.restartPipeline(repo.value.owner, repo.value.name, pipelineId.value, { fork: true });
+  notifications.notify({ title: i18n.t('repo.pipeline.actions.restart_success'), type: 'success' });
+  // TODO: directly send to newest pipeline?
+  await router.push({ name: 'repo', params: { repoName: repo.value.name, repoOwner: repo.value.owner } });
+});
+
+onMounted(loadPipeline);
+watch([repoName, repoOwner, pipelineId], loadPipeline);
+onBeforeUnmount(() => {
+  favicon.updateStatus('default');
+});
+
+const activeTab = computed({
+  get() {
+    if (route.name === 'repo-pipeline-changed-files') {
+      return 'changed-files';
     }
 
-    const pipeline = pipelineStore.getPipeline(repoOwner, repoName, pipelineId);
-    const { since, duration, created } = usePipeline(pipeline);
-    provide('pipeline', pipeline);
-
-    const { message } = usePipeline(pipeline);
-
-    const showDeployPipelinePopup = ref(false);
-
-    async function loadPipeline(): Promise<void> {
-      if (!repo) {
-        throw new Error('Unexpected: Repo is undefined');
-      }
-
-      await pipelineStore.loadPipeline(repo.value.owner, repo.value.name, parseInt(pipelineId.value, 10));
-
-      favicon.updateStatus(pipeline.value?.status);
+    if (route.name === 'repo-pipeline-config') {
+      return 'config';
     }
 
-    const { doSubmit: cancelPipeline, isLoading: isCancelingPipeline } = useAsyncAction(async () => {
-      if (!repo) {
-        throw new Error('Unexpected: Repo is undefined');
-      }
+    return 'tasks';
+  },
+  set(tab: string) {
+    if (tab === 'tasks') {
+      router.replace({ name: 'repo-pipeline' });
+    }
 
-      if (!pipeline.value?.steps) {
-        throw new Error('Unexpected: Pipeline steps not loaded');
-      }
+    if (tab === 'changed-files') {
+      router.replace({ name: 'repo-pipeline-changed-files' });
+    }
 
-      // TODO: is selectedStepId right?
-      // const step = findStep(pipeline.value.steps, selectedStepId.value || 2);
-
-      // if (!step) {
-      //   throw new Error('Unexpected: Step not found');
-      // }
-
-      await apiClient.cancelPipeline(repo.value.owner, repo.value.name, parseInt(pipelineId.value, 10));
-      notifications.notify({ title: i18n.t('repo.pipeline.actions.cancel_success'), type: 'success' });
-    });
-
-    const { doSubmit: restartPipeline, isLoading: isRestartingPipeline } = useAsyncAction(async () => {
-      if (!repo) {
-        throw new Error('Unexpected: Repo is undefined');
-      }
-
-      await apiClient.restartPipeline(repo.value.owner, repo.value.name, pipelineId.value, { fork: true });
-      notifications.notify({ title: i18n.t('repo.pipeline.actions.restart_success'), type: 'success' });
-      // TODO: directly send to newest pipeline?
-      await router.push({ name: 'repo', params: { repoName: repo.value.name, repoOwner: repo.value.owner } });
-    });
-
-    onMounted(loadPipeline);
-    watch([repo, pipelineId], loadPipeline);
-    onBeforeUnmount(() => {
-      favicon.updateStatus('default');
-    });
-
-    const activeTab = computed({
-      get() {
-        if (route.name === 'repo-pipeline-changed-files') {
-          return 'changed-files';
-        }
-
-        if (route.name === 'repo-pipeline-config') {
-          return 'config';
-        }
-
-        return 'tasks';
-      },
-      set(tab: string) {
-        if (tab === 'tasks') {
-          router.replace({ name: 'repo-pipeline' });
-        }
-
-        if (tab === 'changed-files') {
-          router.replace({ name: 'repo-pipeline-changed-files' });
-        }
-
-        if (tab === 'config') {
-          router.replace({ name: 'repo-pipeline-config' });
-        }
-      },
-    });
-
-    return {
-      repoPermissions,
-      pipeline,
-      repo,
-      message,
-      isCancelingPipeline,
-      isRestartingPipeline,
-      showDeployPipelinePopup,
-      activeTab,
-      since,
-      duration,
-      cancelPipeline,
-      restartPipeline,
-      goBack: useRouteBackOrDefault({ name: 'repo' }),
-      created,
-    };
+    if (tab === 'config') {
+      router.replace({ name: 'repo-pipeline-config' });
+    }
   },
 });
+
+const goBack = useRouteBackOrDefault({ name: 'repo' });
 </script>