-
Notifications
You must be signed in to change notification settings - Fork 35
app-catalog: Display current and latest app versions in list #387
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.
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
base: main
Are you sure you want to change the base?
Conversation
a5e6d28
to
4efceed
Compare
Signed-off-by: Evangelos Skopelitis <[email protected]> change change
Signed-off-by: Evangelos Skopelitis <[email protected]>
4efceed
to
5c74180
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This change enhances the app-catalog list to display both current and latest app versions, helping users identify when their installed applications need updates. The existing "App Version" column is renamed to "Current Version" and a new "Latest Version" column is added that fetches the latest available versions from ArtifactHub.
- Adds a new API function to fetch latest app versions from ArtifactHub
- Implements version formatting with consistent "v" prefix handling
- Updates the releases list to show both current and latest versions side by side
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
app-catalog/src/api/charts.tsx | Adds fetchLatestAppVersion function to query ArtifactHub API for latest app versions |
app-catalog/src/components/releases/List.tsx | Adds version formatting utility, fetches latest versions, and updates table columns to display both current and latest versions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
useEffect(() => { | ||
if (!releases?.length) { | ||
setLatestMap({}); | ||
return; | ||
} | ||
|
||
Promise.all( | ||
releases.map(async r => { | ||
const chartName = r?.chart?.metadata?.name; | ||
const v = chartName ? await fetchLatestAppVersion(chartName).catch(() => '—') : '—'; | ||
return [r.name, v] as const; | ||
}) |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation makes parallel API calls for each release without any rate limiting or batching. Consider adding a delay between requests or implementing batching to avoid overwhelming the ArtifactHub API, especially with large numbers of installed applications.
useEffect(() => { | |
if (!releases?.length) { | |
setLatestMap({}); | |
return; | |
} | |
Promise.all( | |
releases.map(async r => { | |
const chartName = r?.chart?.metadata?.name; | |
const v = chartName ? await fetchLatestAppVersion(chartName).catch(() => '—') : '—'; | |
return [r.name, v] as const; | |
}) | |
// Helper to batch promises | |
async function batchPromises<T, R>(items: T[], batchSize: number, fn: (item: T) => Promise<R>): Promise<R[]> { | |
const results: R[] = []; | |
for (let i = 0; i < items.length; i += batchSize) { | |
const batch = items.slice(i, i + batchSize); | |
const batchResults = await Promise.all(batch.map(fn)); | |
results.push(...batchResults); | |
} | |
return results; | |
} | |
useEffect(() => { | |
if (!releases?.length) { | |
setLatestMap({}); | |
return; | |
} | |
// Limit to 5 concurrent requests per batch | |
batchPromises( | |
releases, | |
5, | |
async r => { | |
const chartName = r?.chart?.metadata?.name; | |
const v = chartName ? await fetchLatestAppVersion(chartName).catch(() => '—') : '—'; | |
return [r.name, v] as const; | |
} |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems valid too?
It looks like the exact rate limits are not documented: https://artifacthub.io/docs/topics/faq/#what-are-the-api-rate-limits
I guess 100+ charts is not uncommon.
const selectedPackage = | ||
packages.find( | ||
p => p?.name?.toLowerCase() === lowerChartName || p?.normalized_name === lowerChartName | ||
) ?? packages[0]; |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback to packages[0]
could return the wrong package if no exact match is found. This may display incorrect version information for applications that don't have an exact name match in ArtifactHub.
) ?? packages[0]; | |
); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one seems a valid point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this.
Left a couple of notes, and checked the copilot review comments.
}).then(response => response.text()); | ||
} | ||
|
||
export async function fetchLatestAppVersion(chartName: string): Promise<string> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one needs docs.
const selectedPackage = | ||
packages.find( | ||
p => p?.name?.toLowerCase() === lowerChartName || p?.normalized_name === lowerChartName | ||
) ?? packages[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one seems a valid point?
useEffect(() => { | ||
if (!releases?.length) { | ||
setLatestMap({}); | ||
return; | ||
} | ||
|
||
Promise.all( | ||
releases.map(async r => { | ||
const chartName = r?.chart?.metadata?.name; | ||
const v = chartName ? await fetchLatestAppVersion(chartName).catch(() => '—') : '—'; | ||
return [r.name, v] as const; | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems valid too?
It looks like the exact rate limits are not documented: https://artifacthub.io/docs/topics/faq/#what-are-the-api-rate-limits
I guess 100+ charts is not uncommon.
return '—'; | ||
} | ||
|
||
return s.startsWith('v') ? s : `v${s}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should add a v to it if it's not there. Some version numbers don't use v in there, so adding it could be confusing to people.
This change adds the latest app version in the app-catalog list, allowing users to see if their app version is out of date and requires an update.
The "App Version" column has become "Current Version" to reflect the current app version, and the "Latest Version" column reflects the latest app version.
Fixes: #342
Testing
cd app-catalog && npm run build && npm run start
in one terminal