Merge pull request #44282 from nextcloud/fix/app-discover-pin-and-parse

fix(settings): Support `order` property on App Discover elements and hide future elements

Author: susnux

apps/settings/src/components/AppStoreDiscover/AppStoreDiscoverSection.vue

@@ -38,7 +38,7 @@ import NcIconSvgWrapper from '@nextcloud/vue/dist/Components/NcIconSvgWrapper.js
 import NcLoadingIcon from '@nextcloud/vue/dist/Components/NcLoadingIcon.js'
 
 import logger from '../../logger'
-import { apiTypeParser } from '../../utils/appDiscoverTypeParser.ts'
+import { parseApiResponse, filterElements } from '../../utils/appDiscoverParser.ts'
 
 const PostType = defineAsyncComponent(() => import('./PostType.vue'))
 const CarouselType = defineAsyncComponent(() => import('./CarouselType.vue'))
@@ -50,7 +50,7 @@ const elements = ref<IAppDiscoverElements[]>([])
  * Shuffle using the Fisher-Yates algorithm
  * @param array The array to shuffle (in place)
  */
-const shuffleArray = (array) => {
+const shuffleArray = <T, >(array: T[]): T[] => {
 	for (let i = array.length - 1; i > 0; i--) {
 		const j = Math.floor(Math.random() * (i + 1));
 		[array[i], array[j]] = [array[j], array[i]]
@@ -64,8 +64,14 @@ const shuffleArray = (array) => {
 onBeforeMount(async () => {
 	try {
 		const { data } = await axios.get<Record<string, unknown>[]>(generateUrl('/settings/api/apps/discover'))
-		const parsedData = data.map(apiTypeParser)
-		elements.value = shuffleArray(parsedData)
+		// Parse data to ensure dates are useable and then filter out expired or future elements
+		const parsedElements = data.map(parseApiResponse).filter(filterElements)
+		// Shuffle elements to make it looks more interesting
+		const shuffledElements = shuffleArray(parsedElements)
+		// Sort pinned elements first
+		shuffledElements.sort((a, b) => (a.order ?? Infinity) < (b.order ?? Infinity) ? -1 : 1)
+		// Set the elements to the UI
+		elements.value = shuffledElements
 	} catch (error) {
 		hasError.value = true
 		logger.error(error as Error)

apps/settings/src/constants/AppDiscoverTypes.ts

@@ -41,6 +41,11 @@ export interface IAppDiscoverElement {
 	 */
 	id: string,
 
+	/**
+	 * Order of this element to pin elements (smaller = shown on top)
+	 */
+	order?: number
+
 	/**
 	 * Optional, localized, headline for the element
 	 */
@@ -54,12 +59,12 @@ export interface IAppDiscoverElement {
 	/**
 	 * Optional date when this element will get valid (only show since then)
 	 */
-	date?: Date|number
+	date?: number
 
 	/**
 	 * Optional date when this element will be invalid (only show until then)
 	 */
-	expiryDate?: Date|number
+	expiryDate?: number
 }
 
 /** Wrapper for media source and MIME type */

apps/settings/src/utils/appDiscoverParser.spec.ts

@@ -0,0 +1,96 @@
+/**
+ * @copyright Copyright (c) 2024 Ferdinand Thiessen <opensource@fthiessen.de>
+ *
+ * @author Ferdinand Thiessen <opensource@fthiessen.de>
+ *
+ * @license AGPL-3.0-or-later
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+import type { IAppDiscoverElement } from '../constants/AppDiscoverTypes'
+
+import { describe, expect, it } from '@jest/globals'
+import { filterElements, parseApiResponse } from './appDiscoverParser'
+
+describe('App Discover API parser', () => {
+	describe('filterElements', () => {
+		it('can filter expired elements', () => {
+			const result = filterElements({ id: 'test', type: 'post', expiryDate: 100 })
+			expect(result).toBe(false)
+		})
+
+		it('can filter upcoming elements', () => {
+			const result = filterElements({ id: 'test', type: 'post', date: Date.now() + 10000 })
+			expect(result).toBe(false)
+		})
+
+		it('ignores element without dates', () => {
+			const result = filterElements({ id: 'test', type: 'post' })
+			expect(result).toBe(true)
+		})
+
+		it('allows not yet expired elements', () => {
+			const result = filterElements({ id: 'test', type: 'post', expiryDate: Date.now() + 10000 })
+			expect(result).toBe(true)
+		})
+
+		it('allows yet included elements', () => {
+			const result = filterElements({ id: 'test', type: 'post', date: 100 })
+			expect(result).toBe(true)
+		})
+
+		it('allows elements included and not expired', () => {
+			const result = filterElements({ id: 'test', type: 'post', date: 100, expiryDate: Date.now() + 10000 })
+			expect(result).toBe(true)
+		})
+
+		it('can handle null values', () => {
+			const result = filterElements({ id: 'test', type: 'post', date: null, expiryDate: null } as unknown as IAppDiscoverElement)
+			expect(result).toBe(true)
+		})
+	})
+
+	describe('parseApiResponse', () => {
+		it('can handle basic post', () => {
+			const result = parseApiResponse({ id: 'test', type: 'post' })
+			expect(result).toEqual({ id: 'test', type: 'post' })
+		})
+
+		it('can handle carousel', () => {
+			const result = parseApiResponse({ id: 'test', type: 'carousel' })
+			expect(result).toEqual({ id: 'test', type: 'carousel' })
+		})
+
+		it('can handle showcase', () => {
+			const result = parseApiResponse({ id: 'test', type: 'showcase' })
+			expect(result).toEqual({ id: 'test', type: 'showcase' })
+		})
+
+		it('throws on unknown type', () => {
+			expect(() => parseApiResponse({ id: 'test', type: 'foo-bar' })).toThrow()
+		})
+
+		it('parses the date', () => {
+			const result = parseApiResponse({ id: 'test', type: 'showcase', date: '2024-03-19T17:28:19+0000' })
+			expect(result).toEqual({ id: 'test', type: 'showcase', date: 1710869299000 })
+		})
+
+		it('parses the expiryDate', () => {
+			const result = parseApiResponse({ id: 'test', type: 'showcase', expiryDate: '2024-03-19T17:28:19Z' })
+			expect(result).toEqual({ id: 'test', type: 'showcase', expiryDate: 1710869299000 })
+		})
+	})
+})

…tings/src/utils/appDiscoverTypeParser.ts …/settings/src/utils/appDiscoverParser.tsapps/settings/src/utils/appDiscoverTypeParser.ts renamed to apps/settings/src/utils/appDiscoverParser.ts apps/settings/src/utils/appDiscoverTypeParser.ts renamed to apps/settings/src/utils/appDiscoverParser.ts

@@ -20,14 +20,14 @@
  *
  */
 
-import type { IAppDiscoverCarousel, IAppDiscoverElements, IAppDiscoverPost, IAppDiscoverShowcase } from '../constants/AppDiscoverTypes.ts'
+import type { IAppDiscoverCarousel, IAppDiscoverElement, IAppDiscoverElements, IAppDiscoverPost, IAppDiscoverShowcase } from '../constants/AppDiscoverTypes.ts'
 
 /**
  * Helper to transform the JSON API results to proper frontend objects (app discover section elements)
  *
  * @param element The JSON API element to transform
  */
-export const apiTypeParser = (element: Record<string, unknown>): IAppDiscoverElements => {
+export const parseApiResponse = (element: Record<string, unknown>): IAppDiscoverElements => {
 	const appElement = { ...element }
 	if (appElement.date) {
 		appElement.date = Date.parse(appElement.date as string)
@@ -45,3 +45,21 @@ export const apiTypeParser = (element: Record<string, unknown>): IAppDiscoverEle
 	}
 	throw new Error(`Invalid argument, app discover element with type ${element.type ?? 'unknown'} is unknown`)
 }
+
+/**
+ * Filter outdated or upcoming elements
+ * @param element Element to check
+ */
+export const filterElements = (element: IAppDiscoverElement) => {
+	const now = Date.now()
+	// Element not yet published
+	if (element.date && element.date > now) {
+		return false
+	}
+
+	// Element expired
+	if (element.expiryDate && element.expiryDate < now) {
+		return false
+	}
+	return true
+}