ui: Show some basic information about found packages in the results main page #1125

Etsija · 2024-09-30T11:58:11Z

Please see the individual commits for details.

mmurto · 2024-09-30T12:21:12Z

...ganizations/$orgId/products/$productId/repositories/$repoId/_layout/runs/$runIndex/index.tsx

+  // Find deduplicated package types in packages data.
+  const detectedPackageManagers = [
+    ...new Set(
+      packages.data.map((item) => item.identifier?.type).filter(Boolean)
+    ),
+  ];


This likely doesn't work as the query is paginated, so packages only contains the first page with the default size.

The code looks to me like it wouldn't work for Gradle projects, which would instead list "Maven" as the package manager, which would be wrong. Please verify for a Gradle project that "Gradle" would be listed.

Should this rather be changed to describe that the list is of the package sources, or something else that would describe it better? I believe to type to just be ORT Identifier's type, so data to do what you're suggesting is not available here. The same is probably true for NPM/Yarn/pnpm all using NPM packages.

I believe to type to just be ORT Identifier's type, so data to do what you're suggesting is not available here.

The information about whether an Identifier belongs to a package or project is not available in the Identifier itself. What the code should do is to collect the types of the identifiers of all root projects in an ORT result instead of iterating over packages.

I believe to type to just be ORT Identifier's type, so data to do what you're suggesting is not available here.

The information about whether an Identifier belongs to a package or project is not available in the Identifier itself. What the code should do is to collect the types of the identifiers of all root projects in an ORT result instead of iterating over packages.

IMO there can be value for both, showing what package managers are used to manage the project and what type of dependencies the project uses.

Oh, and the list of package types should be sorted alphabetically to be deterministic.

Sorted by package count per type could be good and also deterministic.

Agreed, but I'd also be fine with not having per-type counts in this first iteration. At least not in the card; that's probably too detailed information that could go to a header on the "Packages" tab.

"100 packages of 2 types (Maven, NPM) found"

How about "100 packages from 2 ecosystems (Maven, NPM)" instead?

I was at first thinking about almost exactly the same as I was using "ecosystems" myself above, but then I didn't want to coin yet another new term. On the other hand, we should have the liberty to use "new" terms in the UI if we believe the (non-ORT) user is familiar with them.

TL;DR "100 packages from 2 ecosystems (Maven, NPM)" would be ok with me.

This likely doesn't work as the query is paginated, so packages only contains the first page with the default size.

I think this needs to be either queried twice, or then once with a very large "page size".

mmurto · 2024-09-30T12:21:19Z

...ganizations/$orgId/products/$productId/repositories/$repoId/_layout/runs/$runIndex/index.tsx

+    ),
+  ];
+
+  console.log(detectedPackageManagers);


Should be removed.

mmurto · 2024-09-30T12:22:15Z

...ganizations/$orgId/products/$productId/repositories/$repoId/_layout/runs/$runIndex/index.tsx

-              value={ortRun.jobs.analyzer ? '-' : 'N/A'}
-              className='hover:bg-muted/50'
+              value={ortRun.jobs.analyzer ? packagesTotal : 'N/A'}
+              description={`from ${detectedPackageManagers.length} package managers (${detectedPackageManagers.join(`,`)})`}


IMO this would look better with a badge for each package manager instead of a sentence.

I think a sentence is fine for now. For monorepos with several package managers badges would probably take up too much screen space.

sschuberth · 2024-09-30T12:58:49Z

Nit from the screenshot: "1 package managers" should omit the "s", or simply use "1 package manager(s)" to avoid logic that prints the "s" or not depending on the count.

mmurto · 2024-10-01T06:35:24Z

...ganizations/$orgId/products/$productId/repositories/$repoId/_layout/runs/$runIndex/index.tsx

+  const ecoSystems = [
+    ...new Set(
+      packages.data.map((item) => item.identifier?.type).filter(Boolean)
+    ),
+  ].sort();


This should maybe be memoized as the array can be quite large.

mmurto · 2024-10-01T11:30:54Z

...oductId/repositories/$repoId/_layout/runs/$runIndex/-components/packages-statistics-card.tsx

+  if (packagesIsPending) {
+    return <LoadingIndicator />;
+  }


This would look better if the card including the title and icon etc was there while the data is pending. I would maybe try skeleton at first.

Or just a loading replacing the package count, skeleton doesn't look too good on first try.

I replaced all "real" data with a loading indicator, because there's no point in showing "(a spinner) from ??? ecosystems (???)". So during loading and processing, only the title and icon are shown.

mmurto · 2024-10-02T06:44:24Z

ui/src/components/statistics-card.tsx

+          <div className='text-2xl font-bold'>
+            {value ? value === 'loading' ? <LoadingIndicator /> : value : '-'}
+          </div>


This should not be based on matching a magic string like "loading". The simplest solution could be to type value as React.ReactNode.

mmurto · 2024-10-02T06:52:08Z

There may be a bug in the packages endpoint - please hold on merging this until I've verified it.

Etsija · 2024-10-02T07:50:52Z

There may be a bug in the packages endpoint - please hold on merging this until I've verified it.

I disabled auto-merge.

This optional field can be used to provide some extra at-a-glance information for the ORT run main results page. An example of such info is the detected package managers for the packages statistics card. Signed-off-by: Jyrki Keisala <[email protected]>

For the purpose of de-duplicating and showing all unique ecosystems found from the packages (specifically, from `identifier.type`), it is currently required to fetch all package data and filter it in the front-end. For large projects, this can take seconds for the first time data is fetched and processed. In order to not block the whole ORT run results main page from rendering, while the package data is being processed, move the processing and showing of the data to its own subcomponent of the page. This way, the main page loads, and a loading state (a spinning wheel) is only shown in the package statistics card, until data is fetched and processed. Signed-off-by: Jyrki Keisala <[email protected]>

Show the total number of de-duplicated packages that were analyzed, and the de-duplicated types of the package identifiers. It should be mentioned that while both packages and projects have identifiers, project identifiers (which would probably be more valuable data to show here) are currently not available. This can be improved in a later PR, when that data is available. Signed-off-by: Jyrki Keisala <[email protected]>

While package data is being fetched and processed, show the card with a title and icon, and show loading indicator in place of the actual data which is not yet ready. Signed-off-by: Jyrki Keisala <[email protected]>

Etsija requested a review from mmurto as a code owner September 30, 2024 11:58

mmurto requested changes Sep 30, 2024

View reviewed changes

Etsija force-pushed the do-893-improve-the-packages-overview-card branch 2 times, most recently from ace3360 to 0a93d06 Compare October 1, 2024 04:26

Etsija requested review from sschuberth, mmurto and mnonnenmacher October 1, 2024 04:28

Etsija enabled auto-merge October 1, 2024 04:30

mmurto requested changes Oct 1, 2024

View reviewed changes

Etsija force-pushed the do-893-improve-the-packages-overview-card branch 3 times, most recently from aef49bd to f4c6b95 Compare October 1, 2024 10:02

Etsija requested a review from mmurto October 1, 2024 10:02

mmurto requested changes Oct 1, 2024

View reviewed changes

Etsija force-pushed the do-893-improve-the-packages-overview-card branch from f4c6b95 to 9db83d0 Compare October 2, 2024 03:22

Etsija requested a review from mmurto October 2, 2024 05:32

mmurto requested changes Oct 2, 2024

View reviewed changes

Etsija force-pushed the do-893-improve-the-packages-overview-card branch from 068485e to 325a296 Compare October 2, 2024 07:50

Etsija disabled auto-merge October 2, 2024 07:50

Etsija requested a review from mmurto October 2, 2024 08:16

Etsija force-pushed the do-893-improve-the-packages-overview-card branch from 325a296 to 0cdc213 Compare October 2, 2024 09:53

mmurto mentioned this pull request Oct 2, 2024

Packages endpoint fails for certain repositories #1137

Open

Etsija added 4 commits October 3, 2024 08:02

feat(ui): Improve the loading state of package statistics card

3343717

While package data is being fetched and processed, show the card with a title and icon, and show loading indicator in place of the actual data which is not yet ready. Signed-off-by: Jyrki Keisala <[email protected]>

Etsija force-pushed the do-893-improve-the-packages-overview-card branch from 0cdc213 to 3343717 Compare October 3, 2024 05:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ui: Show some basic information about found packages in the results main page #1125

ui: Show some basic information about found packages in the results main page #1125

Etsija commented Sep 30, 2024 •

edited

Loading

mmurto Sep 30, 2024

sschuberth Sep 30, 2024

mmurto Sep 30, 2024

sschuberth Sep 30, 2024

mmurto Sep 30, 2024

mmurto Sep 30, 2024

sschuberth Sep 30, 2024

mnonnenmacher Sep 30, 2024 •

edited

Loading

sschuberth Sep 30, 2024

Etsija Sep 30, 2024

mmurto Sep 30, 2024

mmurto Sep 30, 2024

sschuberth Sep 30, 2024

sschuberth commented Sep 30, 2024

mmurto Oct 1, 2024

mmurto Oct 1, 2024

mmurto Oct 1, 2024

Etsija Oct 2, 2024 •

edited

Loading

mmurto Oct 2, 2024

mmurto commented Oct 2, 2024

Etsija commented Oct 2, 2024

ui: Show some basic information about found packages in the results main page #1125

Are you sure you want to change the base?

ui: Show some basic information about found packages in the results main page #1125

Conversation

Etsija commented Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnonnenmacher Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sschuberth commented Sep 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Etsija Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmurto commented Oct 2, 2024

Etsija commented Oct 2, 2024

Etsija commented Sep 30, 2024 •

edited

Loading

mnonnenmacher Sep 30, 2024 •

edited

Loading

Etsija Oct 2, 2024 •

edited

Loading