Fix secret loading (#3620)

README.md

@@ -7,8 +7,8 @@
 </p>
 <br/>
 <p align="center">
-  <a href="https://ci.woodpecker-ci.org/repos/3780" title="Build Status">
-    <img src="https://ci.woodpecker-ci.org/api/badges/3780/status.svg" alt="Build Status">
+  <a href="https://ci.woodpecker-ci.org/repos/3780" title="Pipeline Status">
+    <img src="https://ci.woodpecker-ci.org/api/badges/3780/status.svg" alt="Pipeline Status">
   </a>
   <a href="https://codecov.io/gh/woodpecker-ci/woodpecker">
     <img src="https://codecov.io/gh/woodpecker-ci/woodpecker/branch/main/graph/badge.svg" alt="Code coverage">

server/router/api.go

@@ -169,12 +169,17 @@ func apiRoutes(e *gin.RouterGroup) {
 			queue.GET("/norunningpipelines", api.BlockTilQueueHasRunningItem)
 		}
 
+		// global secrets can be read without actual values by any user
+		readGlobalSecrets := apiBase.Group("/secrets")
+		{
+			readGlobalSecrets.Use(session.MustUser())
+			readGlobalSecrets.GET("", api.GetGlobalSecretList)
+			readGlobalSecrets.GET("/:secret", api.GetGlobalSecret)
+		}
 		secrets := apiBase.Group("/secrets")
 		{
 			secrets.Use(session.MustAdmin())
-			secrets.GET("", api.GetGlobalSecretList)
 			secrets.POST("", api.PostGlobalSecret)
-			secrets.GET("/:secret", api.GetGlobalSecret)
 			secrets.PATCH("/:secret", api.PatchGlobalSecret)
 			secrets.DELETE("/:secret", api.DeleteGlobalSecret)
 		}

web/.eslintignore

@@ -5,4 +5,5 @@ package.json
 tsconfig.eslint.json
 tsconfig.json
 src/assets/locales/
+src/assets/dayjsLocales/
 components.d.ts

web/package.json

@@ -14,7 +14,7 @@
     "format": "prettier --write .",
     "format:check": "prettier -c .",
     "typecheck": "vue-tsc --noEmit",
-    "test": "echo 'No tests configured' && exit 0"
+    "test": "vitest"
   },
   "dependencies": {
     "@intlify/unplugin-vue-i18n": "^4.0.0",
@@ -44,6 +44,7 @@
     "@typescript-eslint/parser": "^7.0.0",
     "@vitejs/plugin-vue": "^5.0.3",
     "@vue/compiler-sfc": "^3.4.15",
+    "@vue/test-utils": "^2.4.5",
     "eslint": "^8.56.0",
     "eslint-config-airbnb-base": "^15.0.0",
     "eslint-config-airbnb-typescript": "^18.0.0",
@@ -54,6 +55,7 @@
     "eslint-plugin-simple-import-sort": "^12.0.0",
     "eslint-plugin-vue": "^9.20.1",
     "eslint-plugin-vue-scoped-css": "^2.7.2",
+    "jsdom": "^24.0.0",
     "prettier": "^3.2.4",
     "replace-in-file": "^7.1.0",
     "tinycolor2": "^1.6.0",
@@ -64,6 +66,7 @@
     "vite-plugin-prismjs": "^0.0.11",
     "vite-plugin-windicss": "^1.9.3",
     "vite-svg-loader": "^5.1.0",
+    "vitest": "^1.5.0",
     "vue-eslint-parser": "^9.4.0",
     "vue-tsc": "^2.0.0",
     "windicss": "^3.5.6"

web/src/components/repo/settings/SecretsTab.vue

@@ -83,6 +83,7 @@ async function loadSecrets(page: number, level: 'repo' | 'org' | 'global'): Prom
 
 const { resetPage, data: _secrets } = usePagination(loadSecrets, () => !selectedSecret.value, {
   each: ['repo', 'org', 'global'],
+  pageSize: 50,
 });
 const secrets = computed(() => {
   const secretsList: Record<string, Secret & { edit?: boolean; level: 'repo' | 'org' | 'global' }> = {};

web/src/compositions/usePaginate.test.ts

@@ -0,0 +1,153 @@
+import { shallowMount } from '@vue/test-utils';
+import { describe, expect, it } from 'vitest';
+import { type Ref, watch } from 'vue';
+
+import { usePagination } from './usePaginate';
+
+async function waitForState<T>(ref: Ref<T>, expected: T): Promise<void> {
+  await new Promise<void>((resolve) => {
+    watch(
+      ref,
+      (value) => {
+        if (value === expected) {
+          resolve();
+        }
+      },
+      { immediate: true },
+    );
+  });
+}
+
+// eslint-disable-next-line promise/prefer-await-to-callbacks
+export const mountComposition = (cb: () => void) => {
+  const wrapper = shallowMount({
+    setup() {
+      // eslint-disable-next-line promise/prefer-await-to-callbacks
+      cb();
+      return {};
+    },
+    template: '<div />',
+  });
+
+  return wrapper;
+};
+
+describe('usePaginate', () => {
+  const repoSecrets = [
+    [{ name: 'repo1' }, { name: 'repo2' }, { name: 'repo3' }],
+    [{ name: 'repo4' }, { name: 'repo5' }, { name: 'repo6' }],
+  ];
+  const orgSecrets = [
+    [{ name: 'org1' }, { name: 'org2' }, { name: 'org3' }],
+    [{ name: 'org4' }, { name: 'org5' }, { name: 'org6' }],
+  ];
+
+  it('should get first page', async () => {
+    let usePaginationComposition = null as unknown as ReturnType<typeof usePagination>;
+    mountComposition(() => {
+      usePaginationComposition = usePagination<{ name: string }>(
+        async (page) => repoSecrets[page - 1],
+        () => true,
+      );
+    });
+    await waitForState(usePaginationComposition.loading, true);
+    await waitForState(usePaginationComposition.loading, false);
+
+    expect(usePaginationComposition.data.value.length).toBe(3);
+    expect(usePaginationComposition.data.value[0]).toStrictEqual(repoSecrets[0][0]);
+  });
+
+  it('should get first & second page', async () => {
+    let usePaginationComposition = null as unknown as ReturnType<typeof usePagination>;
+    mountComposition(() => {
+      usePaginationComposition = usePagination<{ name: string }>(
+        async (page) => repoSecrets[page - 1],
+        () => true,
+      );
+    });
+    await waitForState(usePaginationComposition.loading, true);
+    await waitForState(usePaginationComposition.loading, false);
+
+    usePaginationComposition.nextPage();
+    await waitForState(usePaginationComposition.loading, false);
+
+    expect(usePaginationComposition.data.value.length).toBe(6);
+    expect(usePaginationComposition.data.value.at(-1)).toStrictEqual(repoSecrets[1][2]);
+  });
+
+  it('should get first page for each category', async () => {
+    let usePaginationComposition = null as unknown as ReturnType<typeof usePagination>;
+    mountComposition(() => {
+      usePaginationComposition = usePagination<{ name: string }>(
+        async (page, level) => {
+          if (level === 'repo') {
+            return repoSecrets[page - 1];
+          }
+          return orgSecrets[page - 1];
+        },
+        () => true,
+        { each: ['repo', 'org'] },
+      );
+    });
+    await waitForState(usePaginationComposition.loading, true);
+    await waitForState(usePaginationComposition.loading, false);
+
+    usePaginationComposition.nextPage();
+    await waitForState(usePaginationComposition.loading, false);
+
+    usePaginationComposition.nextPage();
+    await waitForState(usePaginationComposition.loading, false);
+
+    usePaginationComposition.nextPage();
+    await waitForState(usePaginationComposition.loading, false);
+
+    expect(usePaginationComposition.data.value.length).toBe(9);
+    expect(usePaginationComposition.data.value.at(-1)).toStrictEqual(orgSecrets[0][2]);
+  });
+
+  it('should reset page and get first page again', async () => {
+    let usePaginationComposition = null as unknown as ReturnType<typeof usePagination>;
+    mountComposition(() => {
+      usePaginationComposition = usePagination<{ name: string }>(
+        async (page) => repoSecrets[page - 1],
+        () => true,
+      );
+    });
+    await waitForState(usePaginationComposition.loading, true);
+    await waitForState(usePaginationComposition.loading, false);
+
+    usePaginationComposition.nextPage();
+    await waitForState(usePaginationComposition.loading, false);
+
+    usePaginationComposition.resetPage();
+    await waitForState(usePaginationComposition.loading, false);
+
+    expect(usePaginationComposition.data.value.length).toBe(3);
+    expect(usePaginationComposition.data.value[0]).toStrictEqual(repoSecrets[0][0]);
+  });
+
+  it('should not hasMore when no data is left', async () => {
+    let usePaginationComposition = null as unknown as ReturnType<typeof usePagination>;
+    mountComposition(() => {
+      usePaginationComposition = usePagination<{ name: string }>(
+        async (page) => repoSecrets[page - 1],
+        () => true,
+      );
+    });
+    await waitForState(usePaginationComposition.loading, true);
+    await waitForState(usePaginationComposition.loading, false);
+
+    expect(usePaginationComposition.hasMore.value).toBe(true);
+    expect(usePaginationComposition.data.value.length).toBe(3);
+
+    usePaginationComposition.nextPage();
+    await waitForState(usePaginationComposition.loading, false);
+    expect(usePaginationComposition.hasMore.value).toBe(true);
+    expect(usePaginationComposition.data.value.length).toBe(6);
+
+    usePaginationComposition.nextPage();
+    await waitForState(usePaginationComposition.loading, false);
+    expect(usePaginationComposition.hasMore.value).toBe(false);
+    expect(usePaginationComposition.data.value.length).toBe(6);
+  });
+});

web/src/compositions/usePaginate.ts

@@ -18,15 +18,19 @@ export async function usePaginate<T>(getSingle: (page: number) => Promise<T[]>):
 export function usePagination<T, S = unknown>(
   _loadData: (page: number, arg: S) => Promise<T[] | null>,
   isActive: () => boolean = () => true,
-  { scrollElement: _scrollElement, each: _each }: { scrollElement?: Ref<HTMLElement | null>; each?: S[] } = {},
+  {
+    scrollElement: _scrollElement,
+    each: _each,
+    pageSize: _pageSize,
+  }: { scrollElement?: Ref<HTMLElement | null> | null; each?: S[]; pageSize?: number } = {},
 ) {
-  const scrollElement = _scrollElement ?? ref(document.getElementById('scroll-component'));
+  const scrollElement = _scrollElement === null ? null : ref(document.getElementById('scroll-component'));
   const page = ref(1);
-  const pageSize = ref(0);
+  const pageSize = ref(_pageSize ?? 0);
   const hasMore = ref(true);
   const data = ref<T[]>([]) as Ref<T[]>;
   const loading = ref(false);
-  const each = ref(_each ?? []);
+  const each = ref([...(_each ?? [])]);
 
   async function loadData() {
     if (loading.value === true || hasMore.value === false) {
@@ -45,7 +49,7 @@ export function usePagination<T, S = unknown>(
       // use next each element
       each.value.shift();
       page.value = 1;
-      pageSize.value = 0;
+      pageSize.value = _pageSize ?? 0;
       hasMore.value = each.value.length > 0;
       if (hasMore.value) {
         loading.value = false;
@@ -65,15 +69,19 @@ export function usePagination<T, S = unknown>(
     }
   }
 
-  useInfiniteScroll(scrollElement, nextPage, { distance: 10 });
+  if (scrollElement !== null) {
+    useInfiniteScroll(scrollElement, nextPage, { distance: 10 });
+  }
 
   async function resetPage() {
     const _page = page.value;
 
+    page.value = 1;
+    pageSize.value = _pageSize ?? 0;
     hasMore.value = true;
     data.value = [];
-    each.value = (_each ?? []) as UnwrapRef<S[]>;
-    page.value = 1;
+    loading.value = false;
+    each.value = [...(_each ?? [])] as UnwrapRef<S[]>;
 
     if (_page === 1) {
       // we need to reload manually as the page is already 1, so changing won't trigger watcher

web/vite.config.ts

@@ -7,10 +7,10 @@ import replace from 'replace-in-file';
 import IconsResolver from 'unplugin-icons/resolver';
 import Icons from 'unplugin-icons/vite';
 import Components from 'unplugin-vue-components/vite';
-import { defineConfig } from 'vite';
 import prismjs from 'vite-plugin-prismjs';
 import WindiCSS from 'vite-plugin-windicss';
 import svgLoader from 'vite-svg-loader';
+import { defineConfig } from 'vitest/config';
 
 function woodpeckerInfoPlugin() {
   return {
@@ -133,4 +133,8 @@ export default defineConfig({
     host: process.env.VITE_DEV_SERVER_HOST || '127.0.0.1',
     port: 8010,
   },
+  test: {
+    globals: true,
+    environment: 'jsdom',
+  },
 });