Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
"@tremor/react": "^1.8.2",
"ace-builds": "^1.42.1",
"acorn": "^8.9.0",
"ag-grid-community": "^34.1.1",
"ag-grid-react": "^34.1.1",
"ag-grid-community": "^35.0.0",
"ag-grid-react": "^35.0.0",
Comment on lines 55 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The upgrade to ag-grid v35 introduces automatic overlays that conflict with the code's use of deprecated manual overlay APIs like hideOverlay() and showNoRowsOverlay().
Severity: MEDIUM

Suggested Fix

Update the code to use the modern ag-grid v35 API. Either remove the deprecated manual calls to hideOverlay() and showNoRowsOverlay() and rely on the new automatic overlays, or explicitly suppress the new behavior using the suppressOverlays grid option (e.g., suppressOverlays=['noMatchingRows']) if manual control is still desired.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: package.json#L52-L55

Potential issue: The upgrade to `ag-grid` v35 introduces a new automatic overlay system
for tables. However, components like `GradingSubmissionsTable.tsx` and
`OverallLeaderboard.tsx` continue to use deprecated manual overlay control APIs
(`hideOverlay()`, `showNoRowsOverlay()`), which were deprecated in v32. This conflict
between the new automatic behavior and the old manual calls can lead to duplicate or
flickering overlays. For example, when a floating filter in `GradingSubmissionsTable`
yields no results, both the new automatic overlay and the manually triggered one might
appear, creating a confusing user experience.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude code says: This sentry bot comment is partially valid but not critical. Here's the analysis:

What's actually happening:

  • GradingSubmissionsTable.tsx:308 calls gridRef.current!.api.hideOverlay()
  • GradingSubmissionsTable.tsx:311 calls gridRef.current!.api.showNoRowsOverlay()
  • It also uses overlayLoadingTemplate and overlayNoRowsTemplate props (lines 136-138)

However, these APIs are deprecated but not removed in v35 — if they were removed, TypeScript compilation would have failed, and CI passed cleanly. So this is a deprecation warning
situation, not a breakage.

The real risk is the behavioral one the bot describes: ag-grid v35's new automatic overlay system could conflict with these manual calls, potentially causing duplicate/flickering
overlays in the grading table. This wouldn't be caught by tests since it's a visual/UX issue.

Verdict: The PR is still safe to merge — it won't break the build or crash anything. But there's a minor UX risk of overlay quirks in the grading submissions table.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should check if we can migrate to ag-grid v35. Some more investigation needed.

Comment on lines 55 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The code calls api.hideOverlay() and api.showNoRowsOverlay(), which are likely removed in ag-grid v35, causing a runtime TypeError in the grading submissions table.
Severity: HIGH

Suggested Fix

Remove the imperative calls to api.hideOverlay() and api.showNoRowsOverlay() from the useEffect hook. Rely on the declarative loading prop, which is already passed to AgGridReact, to control the loading overlay. The 'no rows' overlay is handled automatically by the grid when data is empty, so the explicit call is unnecessary.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: package.json#L52-L55

Potential issue: The upgrade to `ag-grid` v35 is likely to cause a runtime error in
`GradingSubmissionsTable.tsx`. The `useEffect` hook in this component calls
`api.hideOverlay()` and `api.showNoRowsOverlay()`. These imperative API methods for
managing overlays are likely deprecated or removed in v35, which favors a declarative
approach using the `loading` prop. If these methods no longer exist on the grid API, the
application will throw a `TypeError` whenever the component's `useEffect` hook is
triggered by changes to submissions or request counters, breaking the grading
submissions table functionality.

"array-move": "^4.0.0",
"browserfs": "^1.4.3",
"classnames": "^2.3.2",
Expand Down
32 changes: 16 additions & 16 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4621,32 +4621,32 @@ __metadata:
languageName: node
linkType: hard

"ag-charts-types@npm:12.3.1":
version: 12.3.1
resolution: "ag-charts-types@npm:12.3.1"
checksum: 10c0/e2db0006ddb96f7d5766749cbfd15371725027dacd7fd516c511ad69e72acbbd6bca6ec96c445ca7c962c05e7d915503572a050f92485c6a8d3efaabbfcf0b59
"ag-charts-types@npm:13.2.1":
version: 13.2.1
resolution: "ag-charts-types@npm:13.2.1"
checksum: 10c0/2a5c69354c2dbefdb4d6c65531cbd9931ba02bbc637dd60f53cb004f64328d2ffc5542e04955ccb3a9b17d57b0351b0852163b0c7c01c9685246c3ec2ccb8e06
languageName: node
linkType: hard

"ag-grid-community@npm:34.3.1, ag-grid-community@npm:^34.1.1":
version: 34.3.1
resolution: "ag-grid-community@npm:34.3.1"
"ag-grid-community@npm:35.2.1, ag-grid-community@npm:^35.0.0":
version: 35.2.1
resolution: "ag-grid-community@npm:35.2.1"
dependencies:
ag-charts-types: "npm:12.3.1"
checksum: 10c0/13e0ccaaac10dbbbb8fcee664e4bbb70cef14e84b662fa1c9e294993dd1fda857be9ff8c6e13d82550513816a62d528489b08c06add5d5b7f0344481301b79b9
ag-charts-types: "npm:13.2.1"
checksum: 10c0/e75c5c8cf43aad10d74456549c414f6a88ba456fa1ce919913fbe9f944c6621dcdbe86e84a3f8faaf7c48c7695a8de402b9e63472bdf001e3c52b3d1199c9813
languageName: node
linkType: hard

"ag-grid-react@npm:^34.1.1":
version: 34.3.1
resolution: "ag-grid-react@npm:34.3.1"
"ag-grid-react@npm:^35.0.0":
version: 35.2.1
resolution: "ag-grid-react@npm:35.2.1"
dependencies:
ag-grid-community: "npm:34.3.1"
ag-grid-community: "npm:35.2.1"
prop-types: "npm:^15.8.1"
peerDependencies:
react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0
react-dom: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0
checksum: 10c0/d0efe5678c3721c571d673fabd44e4d871beabe7e5444dbb4d84ddf70757cad813a6276914d1240f8f838fb8a82f19e675c1a6317c1c4e90b9bad29019ea15b5
checksum: 10c0/b096a20da431d432525691db1282e898089afb9ed485c6ae5cf88a44d8e09392f7b8a742614cf9b09de1645d8403d0ca2a9a70daaf927eb4d6c2d0ed96d05582
languageName: node
linkType: hard

Expand Down Expand Up @@ -7447,8 +7447,8 @@ __metadata:
"@vitest/ui": "npm:4.1.2"
ace-builds: "npm:^1.42.1"
acorn: "npm:^8.9.0"
ag-grid-community: "npm:^34.1.1"
ag-grid-react: "npm:^34.1.1"
ag-grid-community: "npm:^35.0.0"
ag-grid-react: "npm:^35.0.0"
array-move: "npm:^4.0.0"
browserfs: "npm:^1.4.3"
buffer: "npm:^6.0.3"
Expand Down
Loading