fix(usage): migrate usage breakdown table to universal table DEV-1013 DEV-1247 #6269

duvld · 2025-09-18T23:28:06Z

🗒️ Checklist

run linter locally
update developer docs (API, README, inline, etc.), if any
for user-facing doc changes create a Zulip thread at #Kobo support docs, if any
draft PR with a title <type>(<scope>)<!>: <title> DEV-1234
assign yourself, tag PR: at least Front end and/or Back end or workflow
fill in the template below and delete template comments
review thyself: read the diff and repro the preview as written
open PR & confirm that CI passes & request reviewers, if needed
delete this section before merging

📣 Summary

Use the new table component to display asset usage data. Fixes old pagination issues

💭 Notes

Modeled after how access logs uses the universal table. Removes uid from the AssetWithUsage type as it is not actually expected as a response from backend--updated logic accordingly

👀 Preview steps

ℹ️ have an account and many projects
submit some data
navigate to account/usage > per project total tab
🔴 [on main] notice that pagination doesn't work
🟢 [on PR] notice that pagination does work
🟢 [on PR] notice that table displays the correct data with no regressions
ℹ️ have a MMO with different member's permissions
🟢 notice that the table has no regressions related to MMOs

magicznyleszek · 2025-09-30T09:37:21Z

@duvld you forgot to add task id in PR title :)

jamesrkiger · 2025-10-01T11:05:04Z

Let's give the table a set height so the sort dropdowns don't get cut off if you only have one asset like here:

This would also make it so the table doesn't jump sizes when you switch from a page of 10 results to a page of 1 result, for example.

Also, there is currently some horizontal overflow, seemingly no matter how wide my screen is. It's fine if there is horizontal overflow on small screens, but can you fix the column sizes so there is no overflow on normal screens?

jsapp/js/account/usage/assetUsage.api.ts

jamesrkiger

Left a couple comments, but it's looking good so far.

jsapp/js/account/usage/usageProjectBreakdown.tsx

jsapp/js/account/usage/assetUsage.api.ts

commit 82c907d Merge: 307c0cc 8c71fd2 Author: Anji <[email protected]> Date: Tue Oct 14 10:59:11 2025 -0400 Merge branch 'dev-987-asset-usage-limit-offset-pagination' into anji/dev-1013-mirgrate-to-universal-table commit 307c0cc Merge: d1bbb52 6e62f77 Author: Anji <[email protected]> Date: Tue Oct 14 10:58:28 2025 -0400 Merge branch 'dev-987-asset-usage-limit-offset-pagination' into anji/dev-1013-mirgrate-to-universal-table Conflicts: jsapp/js/account/usage/usageProjectBreakdown.tsx commit d1bbb52 Author: Kalvis Kalniņš <[email protected]> Date: Thu Oct 9 18:45:26 2025 +0300 fixup! WIP, BROKEN: query error typing mismatch commit 1314ccc Author: Kalvis Kalniņš <[email protected]> Date: Thu Oct 9 18:40:33 2025 +0300 refactor(UniversalTable): parameterize error type commit 133d677 Author: Anji <[email protected]> Date: Thu Oct 9 11:33:00 2025 -0400 WIP, BROKEN: query error typing mismatch commit bda0843 Author: Leszek <[email protected]> Date: Tue Sep 30 21:04:47 2025 +0200 remove duplicated column commit f175294 Author: Anji <[email protected]> Date: Tue Sep 30 13:44:05 2025 -0400 Add count commit a13fdf4 Author: Anji <[email protected]> Date: Tue Sep 30 13:26:51 2025 -0400 add proper link to project label commit d58cc9f Author: Anji <[email protected]> Date: Thu Sep 25 15:06:54 2025 -0400 WIP title link commit 94f54d6 Merge: 9d98814 4e7c317 Author: Anji Tong <[email protected]> Date: Thu Sep 25 17:23:27 2025 +0000 Merge branch 'dev-987-asset-usage-limit-offset-pagination' into anji/dev-1013-mirgrate-to-universal-table commit 9d98814 Author: Anji <[email protected]> Date: Thu Sep 18 19:22:31 2025 -0400 biome commit 0772fad Author: Anji <[email protected]> Date: Thu Sep 18 19:21:22 2025 -0400 fix(assetUsage): migrate asset usage table commit 3fc4877 Author: Anji <[email protected]> Date: Thu Sep 18 18:58:35 2025 -0400 WIP: table migrated, TODO: remove old code

…dev-1013-mirgrate-to-universal-table

Akuukis

see the failing CI, code review inline, not tested.

Akuukis · 2025-11-12T08:05:58Z

jsapp/js/account/usage/usageProjectBreakdown.tsx

  const usageName: ProjectFieldDefinition = {
    name: 'name',
-    label: t('##count## Projects').replace('##count##', projectData.count),
+    label: getUsageNameLabel(),


why bother a separate function for a oneliner that's used once? Also, here's the correct way to check whether response succeeded or errored.

Suggested change

label: getUsageNameLabel(),

label: queryResult.data.status === 200 ? t('##count## Projects').replace('##count##', queryResult.data.data.count.toString()) : t('Projects')

This still needs a check for undefined on queryResult.data, changed it to this:

label: (queryResult.data && queryResult.data.status === 200) ? t('##count## Projects').replace('##count##', queryResult.data.data.count.toString()) : t('Projects'),

Akuukis · 2025-11-12T09:43:06Z

jsapp/js/account/usage/usageProjectBreakdown.tsx

+    // HACK FIX: a bit of a roundabout way to incorporate what the backend expects without diving too deep into changing
+    // existing types


This comment is not helpful as it answers nothing actually. It logically begs the questions "why don't you dive deep into fixing existing types then" and "which backend expectations" and "which existing types", all of which are not answered.

How about "TODO: align props with backend pagination params to simplify away this helper."?

Yes that sounds a lot better, thanks

Akuukis · 2025-11-12T09:47:32Z

jsapp/js/account/usage/usageProjectBreakdown.tsx

+      placeholderData: keepPreviousData,
+      // We might want to improve this in future, for now let's not retry
+      retry: false,
+      // The `refetchOnWindowFocus` option is `true` by default, I'm setting it
+      // here so we don't forget about it.
+      refetchOnWindowFocus: true,
+      throwOnError: () => {
+        notify(t('There was an error getting the list.'), 'error') // TODO: update message in backend (DEV-1218).
+        return false
+      },


are you sure you need any of these options here? I see they are verbatim copied from the useOrganizationsMembersList. By default use defaults which is no options.

Removed other options except for the error throwing

Akuukis · 2025-11-12T09:48:44Z

jsapp/js/account/usage/usageProjectBreakdown.tsx

  const updateOrder = (newOrder: ProjectsTableOrder) => {
    setOrder(newOrder)
  }


why bother an identity function, just use setOrder directly instead of updateOrder?

I don't know, I didn't write this code. I don't want to change old code if I'm editing an existing file so I usually just leave the logic as is. Perhaps it was originally supposed to hold more logic here, but Ruth never got to it. Do you think we should simplify this?

Akuukis · 2025-11-12T09:54:59Z

jsapp/js/projects/projectsTable/sortableProjectColumnHeader.tsx

+  /**
+   * Note: There is only one use case so far where we don't want this to be resizeable, so we set the default to show
+   * the resizer as to not change any other code. See DEV-1255.
+   */
+  notResizeable?: boolean


positive naming is better than negative naming, please consider names like fixedWidth instead. AFAIK all flags are false by default, so lack of fixedWidth should be self-explanatory.

this comment is not answering anything. Which usecase? Why? Won't there be another usecase in future? I looked into it, consider renaming it to something like "For compatibility with react-table set fixedWidth because X otherwise Y".

That's clearer--thank you

jsapp/js/account/usage/usageProjectBreakdown.tsx

jamesrkiger

Left one comment that needs a quick fix

jamesrkiger

I pushed a small change to the getQueryParams function because I noticed that it was resulting in requests with a query param of ordering=undefined when "default sort" was selected. Can you take a quick look and make sure you like that solution? Other than that, lgtm.

duvld self-assigned this Sep 18, 2025

duvld added the Front end label Sep 18, 2025

duvld requested review from jamesrkiger and magicznyleszek as code owners September 18, 2025 23:28

duvld changed the title ~~fix(assetUsage): migrate asset usage table~~ fix(assetUsage): migrate asset usage table DEV-1013 Sep 30, 2025

duvld marked this pull request as draft September 30, 2025 15:00

duvld marked this pull request as ready for review September 30, 2025 17:28

jamesrkiger reviewed Oct 1, 2025

View reviewed changes

jsapp/js/account/usage/assetUsage.api.ts Show resolved Hide resolved

jamesrkiger requested changes Oct 1, 2025

View reviewed changes

Akuukis requested changes Oct 1, 2025

View reviewed changes

jsapp/js/account/usage/usageProjectBreakdown.tsx Outdated Show resolved Hide resolved

jsapp/js/account/usage/assetUsage.api.ts Show resolved Hide resolved

magicznyleszek removed their request for review October 3, 2025 09:31

duvld force-pushed the anji/dev-1013-mirgrate-to-universal-table branch from 82c907d to 2de2fa1 Compare November 5, 2025 15:55

duvld force-pushed the anji/dev-1013-mirgrate-to-universal-table branch from 2de2fa1 to 09b5623 Compare November 5, 2025 15:59

duvld added 4 commits November 5, 2025 11:32

WIP types

5ab9a1c

WIP new type

9852448

biome

2d4af31

Merge branch 'dev-987-asset-usage-limit-offset-pagination' into anji/…

0162aae

…dev-1013-mirgrate-to-universal-table

duvld marked this pull request as draft November 6, 2025 14:58

duvld and others added 5 commits November 6, 2025 11:59

WIP sorting

e7fec86

Adjust pagination/order handling

d5be69a

Update hack fix for status sorting

e438379

add min height to UniversalTable

ee19860

feat(usage): mirgrate usage breakdown table to universal table

9625233

duvld changed the title ~~fix(assetUsage): migrate asset usage table DEV-1013~~ fix(usage): mirgrate usage breakdown table to universal table DEV-1013 DEV-1247 Nov 10, 2025

duvld marked this pull request as ready for review November 10, 2025 18:26

duvld requested review from Akuukis and jamesrkiger November 11, 2025 18:41

Akuukis changed the title ~~fix(usage): mirgrate usage breakdown table to universal table DEV-1013 DEV-1247~~ fix(usage): migrate usage breakdown table to universal table DEV-1013 DEV-1247 Nov 12, 2025

Akuukis requested changes Nov 12, 2025

View reviewed changes

jamesrkiger reviewed Nov 12, 2025

View reviewed changes

jsapp/js/account/usage/usageProjectBreakdown.tsx Outdated Show resolved Hide resolved

jamesrkiger requested changes Nov 12, 2025

View reviewed changes

PR changes

016ddd8

duvld requested review from Akuukis and jamesrkiger November 19, 2025 02:20

Don't send undefined query param

4202d0d

jamesrkiger approved these changes Nov 19, 2025

View reviewed changes

Lint

25f4e24

	label: getUsageNameLabel(),
	label: queryResult.data.status === 200 ? t('##count## Projects').replace('##count##', queryResult.data.data.count.toString()) : t('Projects')

		// HACK FIX: a bit of a roundabout way to incorporate what the backend expects without diving too deep into changing
		// existing types

Uh oh!

fix(usage): migrate usage breakdown table to universal table DEV-1013 DEV-1247 #6269

Are you sure you want to change the base?

fix(usage): migrate usage breakdown table to universal table DEV-1013 DEV-1247 #6269

Conversation

duvld commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗒️ Checklist

📣 Summary

💭 Notes

👀 Preview steps

Uh oh!

magicznyleszek commented Sep 30, 2025

Uh oh!

jamesrkiger commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jamesrkiger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Akuukis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Akuukis Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jamesrkiger left a comment

Choose a reason for hiding this comment

Uh oh!

jamesrkiger left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

duvld commented Sep 18, 2025 •

edited

Loading

jamesrkiger commented Oct 1, 2025 •

edited

Loading

Akuukis left a comment •

edited

Loading

Akuukis Nov 12, 2025 •

edited

Loading