SAK-52177 Gradebook preview grading calculator for hypothetical grades #14296
Conversation
![accesslint[bot]](https://avatars.githubusercontent.com/in/1502?s=60&v=4){kind=small}
| <div class="row"> | ||
| <div class="col-sm-12"> | ||
| <h2 wicket:id="heading">Grade Report for Tony Stark</h2> | ||
| <button type="button" class="btn btn-link btn-xs gb-whatif-toggle pull-right"><wicket:message key="button.whatif.toggle" /></button> |
There was a problem hiding this comment.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
| if ($input.length === 0) { | ||
| const starting = $row.data("grade") ? ("" + $row.data("grade")).trim() | ||
| : $row.find(".gb-summary-grade-score-raw").text().trim(); | ||
| $input = $('<input type="text" class="form-control form-control-sm gb-whatif-input d-none mt-1" />'); |
There was a problem hiding this comment.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
| if ($input.length === 0) { | ||
| const starting = $row.data("grade") ? ("" + $row.data("grade")).trim() | ||
| : $row.find(".gb-summary-grade-score-raw").text().trim(); | ||
| $input = $('<input type="text" class="form-control form-control-sm gb-whatif-input d-none mt-1" />'); |
There was a problem hiding this comment.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
| if ($input.length === 0) { | ||
| const starting = $row.data("grade") ? ("" + $row.data("grade")).trim() | ||
| : $row.find(".gb-summary-grade-score-raw").text().trim(); | ||
| $input = $('<input type="text" class="form-control form-control-sm gb-whatif-input d-none mt-1" />'); |
There was a problem hiding this comment.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
WalkthroughAdds a What‑if course grade preview feature: new API and business-service methods, grading-service implementation, UI templates and data attributes, client-side JavaScript for interactive calculation, and i18n keys; preview calculations use hypothetical scores without persisting changes. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java (1)
64-76: Avoid storingstudentUuid(and other render-derived flags) as mutable component state for Ajax handlers.
studentUuid(Line 75-76, set at Line 118) can become stale or race under concurrent Ajax requests / reused panel instances. Also,someAssignmentsReleasedis a field and should be reset at the start ofonBeforeRender()now that it gatesdata-whatif-enabled.Also applies to: 115-119
♻️ Duplicate comments (2)
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/StudentPage.html (1)
10-10: Update to Bootstrap 5 classes; ensure accessible name is always present.
btn-xs/pull-rightare legacy; preferbtn-sm/float-end(and optionally spacing utilities). Also, ifbutton.whatif.togglewere missing/empty, the button could end up unlabeled—worth ensuring a non-empty label oraria-label.- <button type="button" class="btn btn-link btn-xs gb-whatif-toggle pull-right"><wicket:message key="button.whatif.toggle" /></button> + <button type="button" class="btn btn-link btn-sm gb-whatif-toggle float-end"><wicket:message key="button.whatif.toggle" /></button>gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js (1)
269-389: Fix request race/cancellation (stale projected grade) + de-dupe handlers.With debouncing alone, you can still get out-of-order responses (slow network): request A returns after request B and overwrites the newer result. Also,
this.$content.on("input", ...)should be de-duped, and deactivate should cancel pending work.GradebookGradeSummary.prototype.setupWhatIfCalculator = function() { + let debounceTimer = null; + let lastRequestId = 0; + let abortController = null; + const $toggle = $("body").find(".portletBody .gb-whatif-toggle"); const callbackUrl = this.$content.attr("data-whatif-url"); const enabled = this.$content.attr("data-whatif-enabled") === "true"; @@ - let debounceTimer = null; const requestPreview = () => { clearTimeout(debounceTimer); debounceTimer = setTimeout(() => { + lastRequestId += 1; + const requestId = lastRequestId; + if (abortController) { + abortController.abort(); + } + abortController = new AbortController(); + const payload = collectScores(); fetch(callbackUrl, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8" }, - body: "whatIf=" + encodeURIComponent(JSON.stringify(payload)) + body: "whatIf=" + encodeURIComponent(JSON.stringify(payload)), + signal: abortController.signal }) .then(response => response.ok ? response.json() : Promise.reject()) .then(data => { + if (requestId !== lastRequestId) { + return; + } if (data.error) { showError(data.error); $result.addClass("d-none"); return; } @@ }) .catch(() => { + if (requestId !== lastRequestId) { + return; + } showError(this.$content.attr("data-whatif-error") || "Unable to calculate what-if grade right now."); }); }, 250); }; @@ - this.$content.on("input", ".gb-whatif-input", requestPreview); + this.$content.off("input", ".gb-whatif-input").on("input", ".gb-whatif-input", requestPreview); @@ const deactivate = () => { + clearTimeout(debounceTimer); + if (abortController) { + abortController.abort(); + abortController = null; + } this.$content.find(".gb-whatif-input").addClass("d-none"); $help.addClass("d-none"); $result.addClass("d-none"); clearError(); };Separately, consider adding an
aria-labelto the dynamically created inputs (the accesslint theme is “missing accessible name/label”): e.g., derive from the assignment title cell text and set$input.attr("aria-label", ...).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
gradebookng/api/src/main/java/org/sakaiproject/grading/api/GradingService.java(1 hunks)gradebookng/bundle/src/main/bundle/gradebookng.properties(2 hunks)gradebookng/bundle/src/main/bundle/gradebookng_fr_FR.properties(2 hunks)gradebookng/impl/src/main/java/org/sakaiproject/grading/impl/GradingServiceImpl.java(2 hunks)gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.java(1 hunks)gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/StudentPage.html(1 hunks)gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/GradeSummaryTablePanel.java(1 hunks)gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.html(1 hunks)gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java(7 hunks)gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{html,jsp,vm,xhtml,xml}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{html,jsp,vm,xhtml,xml}: Use Bootstrap 5.2 as the preferred UI framework for styling in Sakai frontends
Leverage Bootstrap 5 components for consistent UI/UX
Files:
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/StudentPage.htmlgradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.html
**/*.{html,jsp,vm,xhtml,xml,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
Ensure all UI components work across different screen sizes (Responsive Design)
Files:
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/StudentPage.htmlgradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.html
**/*.{js,html,jsp,vm,xhtml,xml}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
Use the web components in the webcomponents/ directory when possible in Sakai frontends
Files:
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/StudentPage.htmlgradebookng/tool/src/webapp/scripts/gradebook-grade-summary.jsgradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.html
**/*.{html,jsp,jspx,xml,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer kebab-case for values of HTML class and id attributes
Files:
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/StudentPage.htmlgradebookng/tool/src/webapp/scripts/gradebook-grade-summary.jsgradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.html
**/*.java
📄 CodeRabbit inference engine (.cursor/rules/logging-rule.mdc)
**/*.java: Use SLF4J parameterized logging (logger.info("Value is: {}", value)) instead of string concatenation (logger.info("Value is: " + value))
Log messages and code comments should be in English. Log messages should never be translated.
**/*.java: Java: Never use local variable type inference (var). Always declare explicit types. Yes:Map<String, Integer> counts = new HashMap<>();No:var counts = new HashMap<String, Integer>();
When proposing Java code, spell out full types in local variable declarations,forloops, and try-with-resources
When editing Java, prefer clarity over brevity; avoid introducing language features that aren't widely used in the repo
Treat any PR or suggestion containing Javavaras non-compliant. Recommend replacing with explicit types before merge
**/*.java: Use Java 17 for trunk development (Java 11 was used for Sakai 22 and Sakai 23)
Do not use local variable type inference (var) in Java code. Always declare explicit types (e.g.,List<String> names = new ArrayList<>();notvar names = new ArrayList<String>();). Enforced by Checkstyle rule duringmvn validate
Files:
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.javagradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/GradeSummaryTablePanel.javagradebookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.javagradebookng/api/src/main/java/org/sakaiproject/grading/api/GradingService.javagradebookng/impl/src/main/java/org/sakaiproject/grading/impl/GradingServiceImpl.java
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.js: Use clean, standard modern JavaScript in Sakai frontends
Update jQuery code to modern JavaScript when making changes, if the changes are minimal
Prefer ES6+ features (arrow functions, template literals, destructuring, etc.) in JavaScript
Write modular, reusable JavaScript components in Sakai frontends
Minimize use of global variables and functions (Avoid Global Scope) in JavaScript
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Target evergreen browsers; assume ES2022+ features and browser APIs likefetchkeepalive are present; avoid legacy branches, UA sniffing, or fallbacks unless a specific evergreen gap is documented
Replace jQuery with modern DOM APIs when touching code; new work should not add jQuery dependencies
Compose Lit components, ES modules, and encapsulated helpers; keep state local and explicit with modular code
Prefer module scope or class fields; expose intentional APIs instead of incidental globals; avoid global side channels
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Internal reactive state in Lit components should stay prefixed with
_and is only surfaced through getters/setters when required
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
🧠 Learnings (3)
📓 Common learnings
Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-10-07T15:11:27.298Z
Learning: In samigo’s Total Scores view (samigo/samigo-app/src/webapp/jsf/evaluation/totalScores.jsp), mailto links were hidden after commit dee05746 (PR #12312, SAK-49674) added a render check requiring email.fromEmailAddress to be non-empty; PR #14154 (SAK-52058) restores visibility by checking only description.email.
📚 Learning: 2025-10-07T15:11:27.298Z
Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-10-07T15:11:27.298Z
Learning: In samigo’s Total Scores view (samigo/samigo-app/src/webapp/jsf/evaluation/totalScores.jsp), mailto links were hidden after commit dee05746 (PR #12312, SAK-49674) added a render check requiring email.fromEmailAddress to be non-empty; PR #14154 (SAK-52058) restores visibility by checking only description.email.
Applied to files:
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/StudentPage.html
📚 Learning: 2025-11-24T19:12:27.663Z
Learnt from: CR
Repo: sakaiproject/sakai PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:12:27.663Z
Learning: Define custom HTML elements for Sakai-specific functionality
Applied to files:
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.html
🧬 Code graph analysis (2)
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js (1)
gradebookng/tool/src/webapp/scripts/gradebook-gbgrade-table.js (6)
assignmentId(947-947)assignmentId(1220-1220)assignmentId(1377-1377)data(689-689)data(3192-3195)data(3250-3250)
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/GradeSummaryTablePanel.java (1)
gradebookng/tool/src/webapp/scripts/gradebook-gbgrade-table.js (1)
assignment(3071-3071)
🔇 Additional comments (8)
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/GradeSummaryTablePanel.java (1)
262-264: Avoid leaking unreleased grade data viadata-grade/ normalize attribute values.Even if the grade is already rendered elsewhere, adding
data-grademakes it trivially consumable by any script. Please double-check that, in student view,rawGradeis never populated for non-released/non-viewable items (and that excused/dropped semantics are OK to expose). Also consider string-normalizingdata-pointsto avoidnull/locale surprises.- assignmentItem.add(new AttributeModifier("data-assignment-id", assignment.getId())); - assignmentItem.add(new AttributeModifier("data-points", assignment.getPoints())); - assignmentItem.add(new AttributeModifier("data-grade", StringUtils.defaultString(rawGrade))); + assignmentItem.add(new AttributeModifier("data-assignment-id", String.valueOf(assignment.getId()))); + assignmentItem.add(new AttributeModifier("data-points", assignment.getPoints() == null ? "" : String.valueOf(assignment.getPoints()))); + assignmentItem.add(new AttributeModifier("data-grade", StringUtils.defaultString(rawGrade)));gradebookng/api/src/main/java/org/sakaiproject/grading/api/GradingService.java (1)
672-684: New API looks fine; verify all implementers/consumers are updated and contract is explicit.Consider tightening Javadoc to specify whether
whatIfScoresmay contain blanks/omitted ids, and whether values are interpreted as raw entry type vs display strings (e.g., percentages with/without%). Also ensure allGradingServiceimplementers compile with this new method.gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.java (1)
574-587: Delegation is clean; verify authorization boundary (studentUuid).Please confirm the caller (e.g., the Wicket callback handler) and/or
GradingServiceImpl.calculateCourseGradePreviewprevents a student from calculating a preview for someone else by passing an arbitrarystudentUuid.gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.html (1)
24-31: Nice, minimal scaffolding for help/error/result states.gradebookng/bundle/src/main/bundle/gradebookng.properties (1)
140-141: i18n additions look good; try to avoid hardcoded English fallbacks in JS.Ensure the UI always provides the needed
data-whatif-*strings (or server JSON errors) so the client doesn’t fall back to"Unable to calculate what-if grade right now.".Also applies to: 239-242
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js (1)
254-258: Good integration point (student view); watch for re-init duplication.If the student view re-initializes
GradebookGradeSummarywithout replacing$content, make suresetupWhatIfCalculator()doesn’t accumulate event handlers.gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java (2)
26-54: Imports look fine for the new Ajax/JSON flow.
85-94: Ajax behavior wiring looks good.
| button.whatif.toggle=Calculateur hypoth\u00e9tique | ||
|
|
There was a problem hiding this comment.
Keep property formatting consistent (spaces around =).
This file mostly uses key=value, but the new whatif.* entries use key = value. Consider standardizing to the existing style to reduce diff noise.
Also applies to: 205-208
🤖 Prompt for AI Agents
In gradebookng/bundle/src/main/bundle/gradebookng_fr_FR.properties around lines
110-111 (and also apply the same change to lines 205-208), the new whatif.*
entries use "key = value" spacing while the rest of the file uses "key=value";
update those lines to remove the spaces around the equal sign so they match the
existing "key=value" format (standardize formatting only, do not change keys or
translated values).
gradebookng/impl/src/main/java/org/sakaiproject/grading/impl/GradingServiceImpl.java
Show resolved
Hide resolved
gradebookng/impl/src/main/java/org/sakaiproject/grading/impl/GradingServiceImpl.java
Show resolved
Hide resolved
...g/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java (3)
118-118: Consider derivingstudentUuidper-request for robustness.The
studentUuidis captured duringonBeforeRenderand stored as an instance field. While this works for the primary student view use case, the value could become stale if the panel model changes between render and an AJAX request (e.g., in instructor review modes with student navigation).For improved robustness, derive the student UUID from the model inside
handleWhatIfRequest():private void handleWhatIfRequest() { final RequestCycle requestCycle = RequestCycle.get(); final StringValue payload = requestCycle.getRequest().getRequestParameters().getParameterValue("whatIf"); + final Map<String, Object> modelData = (Map<String, Object>) getDefaultModelObject(); + final String studentUuid = (String) modelData.get("studentUuid"); // ... rest of method using local studentUuid instead of this.studentUuid
278-279: PrefertoOptionalString()for safer parameter extraction.Using
payload.toString()can return"null"as a string if the value is null. While the null checks before this line should prevent issues, usingtoOptionalString()is the idiomatic Wicket approach and provides additional safety.- if (payload != null && !payload.isNull() && StringUtils.isNotBlank(payload.toString())) { - final Map<String, String> raw = mapper.readValue(payload.toString(), new TypeReference<Map<String, String>>() {}); + final String payloadStr = payload.toOptionalString(); + if (StringUtils.isNotBlank(payloadStr)) { + final Map<String, String> raw = mapper.readValue(payloadStr, new TypeReference<Map<String, String>>() {});
296-297: Security concern:includeNonReleasedItems=truemay expose unreleased grades to students.Passing
trueforincludeNonReleasedItemsallows the what-if calculation to include assignments that haven't been released to students. This could inadvertently reveal information about unreleased graded items in the what-if preview, which students shouldn't see.Consider passing
falseto ensure the what-if calculation only considers released assignments:final CourseGradeTransferBean preview = this.businessService.calculateWhatIfCourseGrade(currentGradebookUid, currentSiteId, - this.studentUuid, whatIfMap, true); + this.studentUuid, whatIfMap, false);This ensures the preview calculation respects the same visibility rules as the normal grade display.
🧹 Nitpick comments (2)
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js (1)
347-365: Add explicit error rejection reason for better debugging.When the response is not OK,
Promise.reject()is called without an argument, making it difficult to distinguish between network failures and server errors in the catch handler.- .then(response => response.ok ? response.json() : Promise.reject()) + .then(response => response.ok ? response.json() : Promise.reject(new Error(`Server error: ${response.status}`))) .then(data => { // ... }) - .catch(() => { - showError(this.$content.attr("data-whatif-error") || "Unable to calculate what-if grade right now."); + .catch(error => { + console.warn("What-if calculation failed:", error); + showError(this.$content.attr("data-whatif-error") || "Unable to calculate what-if grade right now."); });gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java (1)
273-274: Consider reusingObjectMapperinstance for better performance.Creating a new
ObjectMapperinstance for each request is inefficient.ObjectMapperis thread-safe after configuration and should be reused as a static field.public class StudentGradeSummaryGradesPanel extends BasePanel { private static final long serialVersionUID = 1L; private static final Logger log = LoggerFactory.getLogger(StudentGradeSummaryGradesPanel.class); + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); // ... in handleWhatIfRequest(): - final ObjectMapper mapper = new ObjectMapper(); - final ObjectNode result = mapper.createObjectNode(); + final ObjectNode result = OBJECT_MAPPER.createObjectNode();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
gradebookng/bundle/src/main/bundle/gradebookng.properties(2 hunks)gradebookng/bundle/src/main/bundle/gradebookng_fr_FR.properties(2 hunks)gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/StudentPage.html(1 hunks)gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java(7 hunks)gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/StudentPage.html
- gradebookng/bundle/src/main/bundle/gradebookng_fr_FR.properties
🧰 Additional context used
📓 Path-based instructions (6)
**/*.java
📄 CodeRabbit inference engine (.cursor/rules/logging-rule.mdc)
**/*.java: Use SLF4J parameterized logging (logger.info("Value is: {}", value)) instead of string concatenation (logger.info("Value is: " + value))
Log messages and code comments should be in English. Log messages should never be translated.
**/*.java: Java: Never use local variable type inference (var). Always declare explicit types. Yes:Map<String, Integer> counts = new HashMap<>();No:var counts = new HashMap<String, Integer>();
When proposing Java code, spell out full types in local variable declarations,forloops, and try-with-resources
When editing Java, prefer clarity over brevity; avoid introducing language features that aren't widely used in the repo
Treat any PR or suggestion containing Javavaras non-compliant. Recommend replacing with explicit types before merge
**/*.java: Use Java 17 for trunk development (Java 11 was used for Sakai 22 and Sakai 23)
Do not use local variable type inference (var) in Java code. Always declare explicit types (e.g.,List<String> names = new ArrayList<>();notvar names = new ArrayList<String>();). Enforced by Checkstyle rule duringmvn validate
Files:
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.js: Use clean, standard modern JavaScript in Sakai frontends
Update jQuery code to modern JavaScript when making changes, if the changes are minimal
Prefer ES6+ features (arrow functions, template literals, destructuring, etc.) in JavaScript
Write modular, reusable JavaScript components in Sakai frontends
Minimize use of global variables and functions (Avoid Global Scope) in JavaScript
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*.{js,html,jsp,vm,xhtml,xml}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
Use the web components in the webcomponents/ directory when possible in Sakai frontends
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Target evergreen browsers; assume ES2022+ features and browser APIs likefetchkeepalive are present; avoid legacy branches, UA sniffing, or fallbacks unless a specific evergreen gap is documented
Replace jQuery with modern DOM APIs when touching code; new work should not add jQuery dependencies
Compose Lit components, ES modules, and encapsulated helpers; keep state local and explicit with modular code
Prefer module scope or class fields; expose intentional APIs instead of incidental globals; avoid global side channels
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Internal reactive state in Lit components should stay prefixed with
_and is only surfaced through getters/setters when required
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*.{html,jsp,jspx,xml,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer kebab-case for values of HTML class and id attributes
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
🧠 Learnings (1)
📓 Common learnings
Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-10-07T15:11:27.298Z
Learning: In samigo’s Total Scores view (samigo/samigo-app/src/webapp/jsf/evaluation/totalScores.jsp), mailto links were hidden after commit dee05746 (PR #12312, SAK-49674) added a render check requiring email.fromEmailAddress to be non-empty; PR #14154 (SAK-52058) restores visibility by checking only description.email.
🧬 Code graph analysis (1)
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js (1)
gradebookng/tool/src/webapp/scripts/gradebook-gbgrade-table.js (6)
assignmentId(947-947)assignmentId(1220-1220)assignmentId(1377-1377)data(689-689)data(3192-3195)data(3250-3250)
🔇 Additional comments (5)
gradebookng/bundle/src/main/bundle/gradebookng.properties (1)
140-140: LGTM! The new i18n keys for the What-if calculator feature follow the existing naming conventions and provide clear, user-friendly messages. Thewhatif.label.prefixkey properly supports accessibility by providing a prefix for ARIA labels on the input fields.Also applies to: 239-243
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js (2)
269-278: LGTM! The early return pattern correctly handles cases where the feature should be disabled: when the toggle button doesn't exist, when no callback URL is configured, or when the feature is explicitly disabled. Hiding the toggle when conditions aren't met provides a clean user experience.
383-396: LGTM! The event handling properly manages accessibility state witharia-pressedand visual state with theactiveclass. The toggle correctly activates or deactivates the What-if calculator based on current state.gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java (2)
85-93: LGTM! The AJAX behavior is correctly wired inonInitialize(), and the data attributes properly expose the callback URL and configuration to the client-side JavaScript. ThewhatIfEnabledflag appropriately gates the feature based on having released assignments and a calculable course grade.Also applies to: 258-263
308-324: LGTM! The response JSON construction properly handles nullable fields with individual null checks, and uses theCourseGradeFormatterfor consistent course grade display formatting that matches the rest of the gradebook UI.
| if ($input.length === 0) { | ||
| const starting = $row.data("grade") ? ("" + $row.data("grade")).trim() | ||
| : $row.find(".gb-summary-grade-score-raw").text().trim(); | ||
| $input = $('<input type="text" class="form-control form-control-sm gb-whatif-input d-none mt-1" />'); |
There was a problem hiding this comment.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
| if ($input.length === 0) { | ||
| const starting = $row.data("grade") ? ("" + $row.data("grade")).trim() | ||
| : $row.find(".gb-summary-grade-score-raw").text().trim(); | ||
| $input = $('<input type="text" class="form-control form-control-sm gb-whatif-input d-none mt-1" />'); |
There was a problem hiding this comment.
Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js (1)
254-266: Prevent duplicated delegated handlers when the view is re-initialized.
this.$content.on("input", ".gb-whatif-input", requestPreview);will stack ifsetupStudentView()runs again for the same DOM (modal reopen / partial refresh). Prefer namespaced events + explicitoff()beforeon().- this.$content.on("input", ".gb-whatif-input", requestPreview); + this.$content.off("input.gbWhatIf", ".gb-whatif-input"); + this.$content.on("input.gbWhatIf", ".gb-whatif-input", requestPreview);Also consider unbinding on modal close/destroy if this component has a lifecycle hook.
Also applies to: 269-395
🧹 Nitpick comments (3)
gradebookng/impl/src/main/java/org/sakaiproject/grading/impl/GradingServiceImpl.java (1)
3509-3543: Avoid N+1 DB calls for assignment lookups in what-if preview.
Inside thewhatIfScoresloop,getAssignmentWithoutStatsByID(...)is called per assignment id (Line 3516), which can get expensive for large gradebooks.Consider preloading needed assignments in bulk (e.g., fetch by ids once via
gradingPersistenceManagerif available, or build a map fromexistingRecords+ one bulk fetch for missing ids).gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java (2)
74-75: Remove unusedstudentUuidfield.The AJAX behavior setup is correct. However, the
studentUuidinstance field (declared at line 75, assigned at line 118) is never used. ThehandleWhatIfRequest()method correctly derivesmodelStudentIdfrom the model on each request (lines 277-278), following the past review recommendation. The instance field can be safely removed.Apply this diff to remove the dead code:
- private AbstractDefaultAjaxBehavior whatIfCalculationBehavior; - private String studentUuid; + private AbstractDefaultAjaxBehavior whatIfCalculationBehavior;And remove the assignment:
final Map<String, Object> modelData = (Map<String, Object>) getDefaultModelObject(); final String userId = (String) modelData.get("studentUuid"); - this.studentUuid = userId; final Gradebook gradebook = getGradebook();Also applies to: 85-93, 118-118
333-345: Error handling is functional; consider using exception types.The error handling correctly catches and logs exceptions, with proper SLF4J parameterized logging at line 340. However, line 334 matches the exception message string
"invalidGrade", which is fragile—exception messages can change, and control flow should rely on exception types or error codes instead.If the business service can throw a more specific exception type (e.g.,
InvalidGradeException), consider catching that instead:+ } catch (InvalidGradeException e) { + result.put("error", getString("whatif.error.invalid")); } catch (IllegalArgumentException e) { - if ("invalidGrade".equals(e.getMessage())) { - result.put("error", getString("whatif.error.invalid")); - } else { - result.put("error", getString("whatif.error.generic")); - } + result.put("error", getString("whatif.error.generic"));Alternatively, keep the current implementation if refactoring the exception hierarchy is not feasible at this time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
gradebookng/impl/src/main/java/org/sakaiproject/grading/impl/GradingServiceImpl.java(2 hunks)gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java(7 hunks)gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.java
📄 CodeRabbit inference engine (.cursor/rules/logging-rule.mdc)
**/*.java: Use SLF4J parameterized logging (logger.info("Value is: {}", value)) instead of string concatenation (logger.info("Value is: " + value))
Log messages and code comments should be in English. Log messages should never be translated.
**/*.java: Java: Never use local variable type inference (var). Always declare explicit types. Yes:Map<String, Integer> counts = new HashMap<>();No:var counts = new HashMap<String, Integer>();
When proposing Java code, spell out full types in local variable declarations,forloops, and try-with-resources
When editing Java, prefer clarity over brevity; avoid introducing language features that aren't widely used in the repo
Treat any PR or suggestion containing Javavaras non-compliant. Recommend replacing with explicit types before merge
**/*.java: Use Java 17 for trunk development (Java 11 was used for Sakai 22 and Sakai 23)
Do not use local variable type inference (var) in Java code. Always declare explicit types (e.g.,List<String> names = new ArrayList<>();notvar names = new ArrayList<String>();). Enforced by Checkstyle rule duringmvn validate
Files:
gradebookng/impl/src/main/java/org/sakaiproject/grading/impl/GradingServiceImpl.javagradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.js: Use clean, standard modern JavaScript in Sakai frontends
Update jQuery code to modern JavaScript when making changes, if the changes are minimal
Prefer ES6+ features (arrow functions, template literals, destructuring, etc.) in JavaScript
Write modular, reusable JavaScript components in Sakai frontends
Minimize use of global variables and functions (Avoid Global Scope) in JavaScript
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*.{js,html,jsp,vm,xhtml,xml}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
Use the web components in the webcomponents/ directory when possible in Sakai frontends
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Target evergreen browsers; assume ES2022+ features and browser APIs likefetchkeepalive are present; avoid legacy branches, UA sniffing, or fallbacks unless a specific evergreen gap is documented
Replace jQuery with modern DOM APIs when touching code; new work should not add jQuery dependencies
Compose Lit components, ES modules, and encapsulated helpers; keep state local and explicit with modular code
Prefer module scope or class fields; expose intentional APIs instead of incidental globals; avoid global side channels
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Internal reactive state in Lit components should stay prefixed with
_and is only surfaced through getters/setters when required
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
**/*.{html,jsp,jspx,xml,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer kebab-case for values of HTML class and id attributes
Files:
gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-10-07T15:11:27.298Z
Learning: In samigo’s Total Scores view (samigo/samigo-app/src/webapp/jsf/evaluation/totalScores.jsp), mailto links were hidden after commit dee05746 (PR #12312, SAK-49674) added a render check requiring email.fromEmailAddress to be non-empty; PR #14154 (SAK-52058) restores visibility by checking only description.email.
📚 Learning: 2025-10-07T15:11:27.298Z
Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-10-07T15:11:27.298Z
Learning: In samigo’s Total Scores view (samigo/samigo-app/src/webapp/jsf/evaluation/totalScores.jsp), mailto links were hidden after commit dee05746 (PR #12312, SAK-49674) added a render check requiring email.fromEmailAddress to be non-empty; PR #14154 (SAK-52058) restores visibility by checking only description.email.
Applied to files:
gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java
🔇 Additional comments (7)
gradebookng/impl/src/main/java/org/sakaiproject/grading/impl/GradingServiceImpl.java (2)
2764-2788:convertWhatIfGrade(...)conversion logic looks correct (locale-aware + letter→percent→points).
This now behaves consistently with stored “points earned” semantics and avoids non-locale parsing.
3485-3487: Inconsistent null return for permission/visibility check - verify caller context.
BothgetAllCourseGrades()(line 3485-3487) andgetCourseGradeForStudents()(line 3565-3581) returnnullwhen the course grade isn't displayed for non-staff users. This differs from other methods in the service that throw exceptions for similar permission failures, creating inconsistent error handling. Confirm whether callers are prepared for null returns without NPE; if these are exposed through REST/controller layer, ensure the controller converts null to a structured JSON error rather than allowing it to marshal asnullor cause a 500.gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java (5)
26-29: LGTM: Imports and logger setup follow best practices.The Jackson imports support JSON handling, Wicket imports enable AJAX behavior, and the SLF4J logger follows standard initialization patterns.
Also applies to: 44-46, 52-53, 64-64
258-264: LGTM: Data attributes correctly expose what-if configuration.The attributes provide the callback URL, i18n messages, and feature enablement flag, allowing the client-side JavaScript to initialize the what-if calculator appropriately.
269-286: LGTM: Past review recommendations successfully addressed.The method correctly:
- Derives
modelStudentIdfrom the model on each request (lines 277-278) rather than using a stale instance field- Uses
payload.toOptionalString()for safe parameter extraction (line 285)- Validates the student ID before proceeding (lines 279-283)
287-302: LGTM: Robust payload parsing with proper error handling.The logic handles null/blank payloads, invalid JSON, and non-numeric assignment IDs gracefully. The parameterized logging at line 297 follows SLF4J best practices per coding guidelines.
304-332: LGTM: Business logic correctly invoked with proper filtering.Line 305 passes
falseforincludeNonReleasedItems, ensuring the backend filters non-released items per the past review recommendation. The result formatting comprehensively includes all relevant grade fields.
| let debounceTimer = null; | ||
| const requestPreview = () => { | ||
| clearTimeout(debounceTimer); | ||
| debounceTimer = setTimeout(() => { | ||
| const payload = collectScores(); | ||
| fetch(callbackUrl, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8" }, | ||
| body: "whatIf=" + encodeURIComponent(JSON.stringify(payload)) | ||
| }) | ||
| .then(response => response.ok ? response.json() : Promise.reject()) | ||
| .then(data => { | ||
| if (data.error) { | ||
| showError(data.error); | ||
| $result.addClass("d-none"); | ||
| return; | ||
| } | ||
| clearError(); | ||
| if (data.courseGrade) { | ||
| $gradeTarget.text(data.courseGrade); | ||
| $result.removeClass("d-none"); | ||
| } else { | ||
| showError(this.$content.attr("data-whatif-error") || "Unable to calculate what-if grade right now."); | ||
| $result.addClass("d-none"); | ||
| } | ||
| }) | ||
| .catch(() => { | ||
| showError(this.$content.attr("data-whatif-error") || "Unable to calculate what-if grade right now."); | ||
| }); | ||
| }, 250); | ||
| }; |
There was a problem hiding this comment.
Guard against out-of-order responses overwriting newer “what-if” results.
With debounce, you can still have overlapping in-flight requests (slow server + continuous typing). A late response can overwrite a newer grade/error state.
Minimal fix: track a monotonically increasing request id and ignore stale responses.
let debounceTimer = null;
+ let requestSeq = 0;
const requestPreview = () => {
clearTimeout(debounceTimer);
debounceTimer = setTimeout(() => {
+ const seq = ++requestSeq;
const payload = collectScores();
fetch(callbackUrl, {
method: "POST",
headers: { "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8" },
body: "whatIf=" + encodeURIComponent(JSON.stringify(payload))
})
.then(response => response.ok ? response.json() : Promise.reject())
.then(data => {
+ if (seq !== requestSeq) { return; }
if (data.error) {
showError(data.error);
$result.addClass("d-none");
return;
}
clearError();
if (data.courseGrade) {
$gradeTarget.text(data.courseGrade);
$result.removeClass("d-none");
} else {
showError(this.$content.attr("data-whatif-error") || "Unable to calculate what-if grade right now.");
$result.addClass("d-none");
}
})
.catch(() => {
+ if (seq !== requestSeq) { return; }
showError(this.$content.attr("data-whatif-error") || "Unable to calculate what-if grade right now.");
});
}, 250);
};(Alternative: AbortController.)
| fetch(callbackUrl, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8" }, | ||
| body: "whatIf=" + encodeURIComponent(JSON.stringify(payload)) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for CSRF token patterns in the codebase
rg -n "csrf|CSRF|X-CSRF|sakai\.csrf|sakai_csrf" --type js --type html --type vm | head -100Repository: sakaiproject/sakai
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Search for data-whatif-url and whatIf usage
rg -n "data-whatif-url|whatIf=" --type js --type html | head -50Repository: sakaiproject/sakai
Length of output: 326
🏁 Script executed:
#!/bin/bash
# Search for whatIf-related methods in gradebookng
rg -n "calculateWhatIfCourseGrade|calculateCourseGradePreview|whatIf" gradebookng --type js | head -100Repository: sakaiproject/sakai
Length of output: 198
🏁 Script executed:
#!/bin/bash
# Look for the gradebook-grade-summary.js file and examine context around lines 340-344
fd "gradebook-grade-summary.js" --type fRepository: sakaiproject/sakai
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Read the file to see the full context around the fetch request
cat -n gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js | sed -n '260,360p'Repository: sakaiproject/sakai
Length of output: 4342
🏁 Script executed:
#!/bin/bash
# Search for CSRF tokens without the problematic filetype
rg -n "csrf|CSRF|X-CSRF|sakai\.csrf|sakai_csrf" | head -50Repository: sakaiproject/sakai
Length of output: 7954
🏁 Script executed:
#!/bin/bash
# Search for Sakai AJAX helpers or fetch utilities
rg -n "sakai\.[a-z]+ajax|fetch|XMLHttpRequest" gradebookng --type js | grep -i "helper\|util\|ajax" | head -30Repository: sakaiproject/sakai
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check what's in the HTML that loads this script (look for data attributes)
fd "\.html|\.jsp|\.jspx" gradebookng | xargs grep -l "gradebook-grade-summary" | head -5Repository: sakaiproject/sakai
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for the what-if endpoint in gradebookng backend
fd "\.java" gradebookng | xargs grep -l "whatif\|whatIf\|WhatIf" | head -10Repository: sakaiproject/sakai
Length of output: 424
🏁 Script executed:
#!/bin/bash
# Search more broadly for POST handlers in gradebookng
fd "\.java" gradebookng | xargs grep -l "doPost\|RequestMapping.*POST" | head -10Repository: sakaiproject/sakai
Length of output: 44
🏁 Script executed:
#!/antml/bash
# Look for the HTML/JSP that renders the what-if section to see if CSRF token is passed
fd "\.(html|jsp|jspx)" gradebookng | xargs grep -l "whatif\|whatIf\|data-whatif-url" | head -10Repository: sakaiproject/sakai
Length of output: 247
🏁 Script executed:
#!/bin/bash
# Check if gradebook-grade-summary.js script tag has CSRF token nearby in HTML
fd "\.(html|jsp|jspx)" gradebookng | xargs grep -B5 -A5 "gradebook-grade-summary" | head -50Repository: sakaiproject/sakai
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check StudentGradeSummaryGradesPanel.java for the what-if endpoint
grep -n "whatif\|whatIf\|WhatIf" gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.javaRepository: sakaiproject/sakai
Length of output: 1424
🏁 Script executed:
#!/bin/bash
# Look at StudentGradeSummaryGradesPanel.java to see how it sets up the data attribute
cat -n gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.java | head -150Repository: sakaiproject/sakai
Length of output: 7729
🏁 Script executed:
#!/antml/bash
# Check the HTML template for what-if section
cat -n gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.html | grep -B10 -A10 "whatif\|whatIf"Repository: sakaiproject/sakai
Length of output: 1583
🏁 Script executed:
#!/antml/bash
# Search for the actual endpoint handler (might be AJAX or REST)
rg -n "handleCalculateWhatIf|whatif|whatIf" gradebookng/tool/src/java --type java | head -30Repository: sakaiproject/sakai
Length of output: 3448
🏁 Script executed:
#!/antml/bash
# Check if there's a CSRF token in any data attribute or if Wicket config disables CSRF
rg -n "csrf.*false|wicket.*csrf|disable.*csrf" gradebookng --type java --type html | head -20Repository: sakaiproject/sakai
Length of output: 44
🏁 Script executed:
#!/bash
# Check the full handleWhatIfRequest method to see if there's CSRF token validation
sed -n '269,350p' gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/StudentGradeSummaryGradesPanel.javaRepository: sakaiproject/sakai
Length of output: 3331
🏁 Script executed:
#!/bash
# Verify Wicket version and default CSRF handling
fd "pom.xml" gradebookng/tool | xargs grep -A5 -B5 "wicket"Repository: sakaiproject/sakai
Length of output: 1188
🏁 Script executed:
#!/bash
# Check if there's a CsrfPreventionRequestCycleListener or CSRF validation in Wicket config
rg -n "CsrfPrevention|CSRF|csrf" gradebookng/tool/src/java --type java | grep -i "listener\|config\|application" | head -20Repository: sakaiproject/sakai
Length of output: 44
🏁 Script executed:
#!/bash
# Look for the GradebookPage or Application class to see if CSRF is configured
fd "GradebookPage\.java|.*Application\.java" gradebookng/tool/src/javaRepository: sakaiproject/sakai
Length of output: 229
🏁 Script executed:
#!/bash
# Check the application configuration for CSRF settings
cat gradebookng/tool/src/java/org/sakaiproject/gradebookng/GradebookNgApplication.java | head -100Repository: sakaiproject/sakai
Length of output: 3776
🏁 Script executed:
#!/bash
# Also check GradebookPage.java for any CSRF configuration
rg -n "csrf|CSRF|AbstractDefaultAjaxBehavior" gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/GradebookPage.javaRepository: sakaiproject/sakai
Length of output: 206
This fetch() call will fail in production due to missing CSRF token.
Wicket's AbstractDefaultAjaxBehavior (used here) enforces CSRF protection by default. The endpoint expects requests to include Wicket's CSRF token (either as a request parameter or header), but the raw fetch() call bypasses Wicket's automatic token injection.
Either:
- Use Wicket's AJAX mechanism instead of raw
fetch(), or - Extract and include the CSRF token from the page in the request headers/body
🤖 Prompt for AI Agents
In gradebookng/tool/src/webapp/scripts/gradebook-grade-summary.js around lines
340-344, the raw fetch() call omits Wicket's CSRF token so the POST will be
rejected in production; fix by reading the token already rendered on the page
(e.g., the Wicket CSRF token stored in a meta tag or hidden input) and include
it with the request — either append it as a form parameter (e.g., add
&wicket:token=... to the body) or set it as the expected header before calling
fetch(), or switch to using Wicket's AJAX helper so the framework injects the
token automatically.
|
This sounds like a cool feature. I'd like to see #14290 merged before this though, personally. The letter grading PR has been in for ages, way before the feature freeze date and I can't afford the time to rework it around these changes if they go in first. I'm employed part time on Sakai now so I just wouldn't have the time to update my PR again. |
ern
changed the title
3 participants
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.