Progress loading for manage submissions #2287
Conversation
📝 WalkthroughWalkthroughThis update introduces new user interface and control flow enhancements for regrading submissions. It adds a new event handler for regrade actions that verifies user intent and provides immediate visual feedback by disabling buttons and showing a loading icon. On the server side, modifications were made to improve submission ID filtering and update response handling for regrade requests. Additionally, the related CSS is enhanced with new classes and animations, and the HTML structure for the regrade buttons is streamlined. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant B as Browser
participant JS as manage_submissions.js
participant S as Server (AssessmentAutograde)
U->>B: Click "Regrade All" button/link
B->>JS: Trigger click event handler
JS->>JS: Prompt confirmation for regrading all submissions
alt User confirms
JS->>B: Start refresh interval (5s)
JS->>JS: Disable button & update loading icon
JS->>S: AJAX POST to regradeAll (with CSRF token)
S-->>JS: Respond with JSON (redirect or status)
JS->>B: Clear refresh interval
alt Response includes redirect URL
JS->>B: Navigate to the provided URL
else Response includes messages/errors
JS->>B: Display alert with message/error
end
else User cancels
JS->>B: Abort action
end
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/assets/javascripts/manage_submissions.js (2)
332-376: Well-implemented AJAX handler for regrade operations.The new event handler for regrade operations effectively implements the PR objectives by providing visual feedback during the operation. The user confirmation, button state management, and loading icon implementation work well together.
A few suggestions to consider:
- The page refresh interval might be disruptive if operations take longer than expected.
- Consider adding more detailed error information in the error handler.
A more user-friendly approach might be to replace the automatic refresh with a one-time redirect after successful completion:
- let refreshInterval = setInterval(() => { - location.reload(); - }, 5000); // Then in success handler: - clearInterval(refreshInterval); + // No need to clear interval since we're not using oneAnd enhance the error handling:
error: function () { - clearInterval(refreshInterval); alert("An error occurred while regrading."); + // Restore button state + button.removeClass("disabled"); + link.removeClass("disabled-link"); + icon.removeClass("loading-icon"); + icon.text("cached"); }
378-431: Consider extracting common AJAX functionality.There's some duplication between the new regrade handler and the functionality in
changeButtonStates. Consider extracting the common AJAX setup, beforeSend, success, and error handling into a reusable function.+ function performAjaxAction(url, requestData, button, icon, successCallback) { + let refreshInterval = setInterval(() => { + location.reload(); + }, 5000); + + $.ajax({ + url: url, + type: "POST", + contentType: "application/json", + data: requestData ? JSON.stringify(requestData) : null, + dataType: "json", + headers: { + "X-CSRF-Token": $('meta[name="csrf-token"]').attr("content"), + }, + beforeSend: function () { + button.addClass('disabled'); + if (icon) { + icon.text("autorenew"); + icon.addClass("loading-icon"); + } + }, + success: function (response) { + clearInterval(refreshInterval); + if (response.redirect) { + window.location.href = response.redirect; + return; + } + if (response.error) { + alert(response.error); + } + if (response.success) { + alert(response.success); + } + if (successCallback) successCallback(response); + }, + error: function () { + clearInterval(refreshInterval); + alert("An error occurred while processing the request."); + button.removeClass('disabled'); + if (icon) { + icon.text("cached"); + icon.removeClass("loading-icon"); + } + }, + }); + }Then you could use it in both places:
// For regrade all performAjaxAction("regradeAll", null, button, icon); // For other actions performAjaxAction(endpoint, requestData, button, icon, function(response) { selectedSubmissions = []; changeButtonStates(true); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/assets/javascripts/manage_submissions.js(2 hunks)app/assets/stylesheets/manage_submissions.css.scss(1 hunks)app/controllers/assessment/autograde.rb(4 hunks)app/views/submissions/index.html.erb(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/assets/javascripts/manage_submissions.js (1)
bin/setup.sh (1)
confirm(41-52)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (8)
app/assets/stylesheets/manage_submissions.css.scss (1)
78-94: Well-structured CSS additions for loading and disabled states.The new CSS classes provide appropriate visual feedback for loading operations and disabled links. The spinner animation uses standard CSS properties and the disabled link style addresses all necessary aspects.
I particularly like the comprehensive approach to disabled links, handling both visual appearance (color, opacity) and interaction (pointer-events, cursor).
app/views/submissions/index.html.erb (3)
87-89: Good structural change for "Regrade All" button.Converting this UI element to a simple anchor with IDs for both the button and icon makes it more accessible to the new JavaScript event handler. The changes align well with the PR objective of implementing a loading indicator.
95-97: Well-structured "Regrade Selected" button with proper IDs.Adding an ID to the icon is a good practice that makes it accessible for dynamic changes in the JavaScript. This allows the loading icon to be shown precisely where needed.
123-129: Clean implementation of the regrade trigger link.This span with a trigger link is cleanly implemented, allowing the JavaScript to properly hook into the UI. The title attribute provides good accessibility.
app/controllers/assessment/autograde.rb (3)
108-108: Good defensive programming with compact.Adding
.compactensures that any nil values are removed from the submissions array, which improves the robustness of the submission filtering process.
195-198: Improved response handling for AJAX support.Replacing the direct redirect with a
respond_toblock enables proper handling of both HTML and JSON responses, which is essential for the new AJAX functionality introduced in the JavaScript.
265-266: Enhanced type checking in _is_i? method.The updated method now properly handles both Integer objects and numeric strings, making it more versatile and robust. This change improves the overall reliability of submission ID processing.
app/assets/javascripts/manage_submissions.js (1)
424-430: Good UI feedback implementation in beforeSend.The implementation properly disables buttons and adds a loading icon during AJAX operations, providing immediate visual feedback to users as specified in the PR objectives.
KesterTan
requested review from
a team,
jlge and
evanyeyeye
and removed request for
a team and
jlge
Motivation and Context
Buttons takes a long time for a large class, instructors don't get any visual feedback when buttons are clicked. This might result in them clicking buttons another time which increases the time spent waiting for regrade-alls etc. This is now fixed by adding a loading symbol when the button is clicked.
Description
How Has This Been Tested?
Types of changes
Checklist:
overcommit --install && overcommit --signto use pre-commit hook for linting