From e16356393928229c43a95b779c530a4b176dc8b2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 9 Feb 2026 23:40:40 +0000 Subject: [PATCH 01/15] Bump lodash in the npm_and_yarn group across 1 directory Bumps the npm_and_yarn group with 1 update in the / directory: [lodash](https://github.com/lodash/lodash). Updates `lodash` from 4.17.21 to 4.17.23 - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](https://github.com/lodash/lodash/compare/4.17.21...4.17.23) --- updated-dependencies: - dependency-name: lodash dependency-version: 4.17.23 dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index b70326f..4d7b202 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3075,9 +3075,9 @@ lodash.debounce@^4.0.8: integrity sha512-FT1yDzDYEoYWhnSGnpE/4Kj1fLZkDFyqRb7fNt6FdYOSxlUWAtp42Eh6Wb0rGIv/m9Bgo7x4GhQbm5Ys4SG5ow== lodash@^4.17.21: - version "4.17.21" - resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c" - integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg== + version "4.17.23" + resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.23.tgz#f113b0378386103be4f6893388c73d0bde7f2c5a" + integrity sha512-LgVTMpQtIopCi79SJeDiP0TfWi5CNEc/L/aRdTh3yIvmZXTnheWpKjSZhnvMl8iXbC1tFg9gdHHDMLoV7CnG+w== lru-cache@^5.1.1: version "5.1.1" From 86fdd375d141e6b3bf920ee83ac88ee553a0535c Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Tue, 10 Feb 2026 15:36:41 -0500 Subject: [PATCH 02/15] Enhance EntryDragController with loading overlay improvements - Updated loading overlay styles to ensure visibility during drag-and-drop actions. - Modified methods to add loading overlays when moving entry cards, improving user feedback. - Ensured smooth scrolling into view for entry cards after movement. These changes enhance the user experience by providing immediate visual feedback during entry ranking updates. --- .../controllers/entry_drag_controller.js | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/app/javascript/controllers/entry_drag_controller.js b/app/javascript/controllers/entry_drag_controller.js index 40daaca..47d5df5 100644 --- a/app/javascript/controllers/entry_drag_controller.js +++ b/app/javascript/controllers/entry_drag_controller.js @@ -71,6 +71,14 @@ export default class extends Controller { font-weight: 500; } + .card.entry-card-loading { + overflow: visible; + } + + .card.entry-card-loading .entry-loading-overlay { + z-index: 9999; + } + @keyframes fadeIn { from { opacity: 0; } to { opacity: 1; } @@ -423,18 +431,23 @@ export default class extends Controller { async addToRanked(event) { if (this.finalizedValue) return - const entryCard = event.target.closest('[data-entry-id]') + const entryCard = event.target.closest('.card[data-entry-id]') if (!entryCard) return - // Move the card to rated entries + // Show loading overlay just like drag-and-drop (before moving so overlay moves with card) + this.addLoadingOverlay(entryCard) + + // Move the card to rated entries (card may be inside a turbo-frame; we move the card node) this.ratedEntriesTarget.appendChild(entryCard) + entryCard.scrollIntoView({ behavior: 'smooth', block: 'nearest' }) + await this.handleSortEnd({ from: this.availableEntriesTarget, to: this.ratedEntriesTarget, item: entryCard }) } async removeFromRanked(event) { if (this.finalizedValue) return - const entryCard = event.target.closest('[data-entry-id]') + const entryCard = event.target.closest('.card[data-entry-id]') if (!entryCard) return // Show confirmation dialog @@ -442,8 +455,13 @@ export default class extends Controller { return } + // Show loading overlay just like drag-and-drop (before moving so overlay moves with card) + this.addLoadingOverlay(entryCard) + // Move the card back to available entries this.availableEntriesTarget.appendChild(entryCard) + entryCard.scrollIntoView({ behavior: 'smooth', block: 'nearest' }) + await this.handleSortEnd({ from: this.ratedEntriesTarget, to: this.availableEntriesTarget, item: entryCard }) } @@ -465,14 +483,13 @@ export default class extends Controller { ` - // Ensure the card has position relative for absolute positioning of overlay + // Ensure the card has position relative and won't clip the overlay element.style.position = 'relative' + element.classList.add('entry-card-loading') element.appendChild(overlay) - // Add show class after a small delay to trigger fade-in - requestAnimationFrame(() => { - overlay.classList.add('show') - }) + // Show overlay immediately so it's visible for button clicks (card is about to move) + overlay.classList.add('show') // Set up slow connection warning this.slowConnectionTimeout = setTimeout(() => { @@ -521,6 +538,8 @@ export default class extends Controller { const overlay = element.querySelector('.entry-loading-overlay') if (!overlay) return + element.classList.remove('entry-card-loading') + // Clear slow connection timeout if (this.slowConnectionTimeout) { clearTimeout(this.slowConnectionTimeout) From 4d4df9d1388e4c7861af03ec81b525f988ea94eb Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Wed, 11 Feb 2026 10:19:11 -0500 Subject: [PATCH 03/15] Add notify_completed action to JudgingRoundsController and related mailer - Introduced a new action `notify_completed` in JudgingRoundsController to notify contest contacts when judges complete their evaluations. - Added `JudgeCompletedEvaluationsMailer` to handle the email notification logic, including dynamic subject and recipient handling. - Implemented policy checks to ensure only authorized users can trigger the notification. - Updated views to include a button for judges to notify contacts, which is enabled based on the number of entries ranked. - Created corresponding tests to verify the functionality of the new action and mailer. These enhancements improve communication with contest contacts and streamline the evaluation process for judges. --- app/controllers/judging_rounds_controller.rb | 52 +++- .../judge_completed_evaluations_mailer.rb | 23 ++ app/policies/contest_instance_policy.rb | 6 + .../notify_contact.html.erb | 30 +++ .../notify_contact.text.erb | 10 + app/views/judge_dashboard/index.html.erb | 19 ++ config/routes.rb | 1 + lib/tasks/test.rake | 4 +- .../judging_rounds_controller_spec.rb | 245 ++++++++++++++++-- spec/policies/contest_instance_policy_spec.rb | 129 --------- spec/system/judge_dashboard_spec.rb | 54 ++++ 11 files changed, 418 insertions(+), 155 deletions(-) create mode 100644 app/mailers/judge_completed_evaluations_mailer.rb create mode 100644 app/views/judge_completed_evaluations_mailer/notify_contact.html.erb create mode 100644 app/views/judge_completed_evaluations_mailer/notify_contact.text.erb delete mode 100644 spec/policies/contest_instance_policy_spec.rb diff --git a/app/controllers/judging_rounds_controller.rb b/app/controllers/judging_rounds_controller.rb index 0841cce..a196fa1 100644 --- a/app/controllers/judging_rounds_controller.rb +++ b/app/controllers/judging_rounds_controller.rb @@ -1,6 +1,6 @@ class JudgingRoundsController < ApplicationController before_action :set_contest_instance - before_action :set_judging_round, only: [ :show, :edit, :update, :destroy, :activate, :deactivate, :complete, :uncomplete, :update_rankings, :finalize_rankings, :send_instructions ] + before_action :set_judging_round, only: [ :show, :edit, :update, :destroy, :activate, :deactivate, :complete, :uncomplete, :update_rankings, :finalize_rankings, :send_instructions, :notify_completed ] before_action :authorize_contest_instance before_action :check_edit_warning, only: [ :edit, :update ] @@ -327,6 +327,56 @@ def finalize_rankings end end + def notify_completed + authorize @contest_instance, :notify_completed? + + entry_rankings = EntryRanking.where( + judging_round: @judging_round, + user: current_user + ) + + if entry_rankings.count < @judging_round.required_entries_count + message = "Please rank at least #{@judging_round.required_entries_count} entries before notifying. You have #{entry_rankings.count} ranked." + respond_to do |format| + format.html { redirect_to judge_dashboard_path, alert: message } + format.turbo_stream do + render turbo_stream: turbo_stream.update('flash', + partial: 'shared/flash', + locals: { message: message, type: 'danger' } + ) + end + end + return + end + + if @container.contact_email.blank? + message = 'No contact email is set for this contest. Please contact an administrator.' + respond_to do |format| + format.html { redirect_to judge_dashboard_path, alert: message } + format.turbo_stream do + render turbo_stream: turbo_stream.update('flash', + partial: 'shared/flash', + locals: { message: message, type: 'danger' } + ) + end + end + return + end + + JudgeCompletedEvaluationsMailer.notify_contact(current_user, @judging_round).deliver_later + + message = 'The contest contact has been notified that you have completed your evaluations.' + respond_to do |format| + format.html { redirect_to judge_dashboard_path, notice: message } + format.turbo_stream do + render turbo_stream: turbo_stream.update('flash', + partial: 'shared/flash', + locals: { message: message, type: 'success' } + ) + end + end + end + private def set_contest_instance diff --git a/app/mailers/judge_completed_evaluations_mailer.rb b/app/mailers/judge_completed_evaluations_mailer.rb new file mode 100644 index 0000000..673341a --- /dev/null +++ b/app/mailers/judge_completed_evaluations_mailer.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class JudgeCompletedEvaluationsMailer < ApplicationMailer + def notify_contact(judge, judging_round) + @judge = judge + @judging_round = judging_round + @contest_instance = judging_round.contest_instance + @contest_description = @contest_instance.contest_description + @container = @contest_description.container + @judge_email = judge.normalize_email + + to_email = @container.contact_email.presence + return unless to_email + + subject = "Judge completed evaluations: #{@contest_description.name} - Round #{@judging_round.round_number}" + + mail( + to: to_email, + subject: subject, + reply_to: @judge_email + ) + end +end diff --git a/app/policies/contest_instance_policy.rb b/app/policies/contest_instance_policy.rb index 420f1e6..76b5b27 100644 --- a/app/policies/contest_instance_policy.rb +++ b/app/policies/contest_instance_policy.rb @@ -92,4 +92,10 @@ def export_entries? def send_instructions? user&.has_container_role?(record.contest_description.container) || axis_mundi? end + + def notify_completed? + return false unless user && record + return false unless record.judging_open? + record.judges.include?(user) + end end diff --git a/app/views/judge_completed_evaluations_mailer/notify_contact.html.erb b/app/views/judge_completed_evaluations_mailer/notify_contact.html.erb new file mode 100644 index 0000000..c2798d7 --- /dev/null +++ b/app/views/judge_completed_evaluations_mailer/notify_contact.html.erb @@ -0,0 +1,30 @@ +
+

+ Judge completed evaluations +

+ +

+ A judge has completed their evaluations for the following contest and round. +

+ +
+

Judge

+

+ Name: <%= @judge.display_name_or_first_name_last_name %>
+ Email: <%= mail_to @judge_email, @judge_email %> +

+
+ +
+

Contest and round

+

+ Contest: <%= @contest_description.name %>
+ Contest instance: <%= @contest_instance.id %> (date open: <%= @contest_instance.date_open&.strftime('%Y-%m-%d') %>)
+ Round: <%= @judging_round.round_number %> +

+
+ +

+ You can reply directly to this message to contact the judge. +

+
diff --git a/app/views/judge_completed_evaluations_mailer/notify_contact.text.erb b/app/views/judge_completed_evaluations_mailer/notify_contact.text.erb new file mode 100644 index 0000000..b847ba7 --- /dev/null +++ b/app/views/judge_completed_evaluations_mailer/notify_contact.text.erb @@ -0,0 +1,10 @@ +A judge has completed their evaluations for the following contest and round. + +Judge: <%= @judge.display_name_or_first_name_last_name %> +Judge email: <%= @judge_email %> + +Contest: <%= @contest_description.name %> +Contest instance: <%= @contest_instance.id %> (date open: <%= @contest_instance.date_open&.strftime('%Y-%m-%d') %>) +Round: <%= @judging_round.round_number %> + +You can reply directly to this message to contact the judge. diff --git a/app/views/judge_dashboard/index.html.erb b/app/views/judge_dashboard/index.html.erb index 2e6213a..4759e01 100644 --- a/app/views/judge_dashboard/index.html.erb +++ b/app/views/judge_dashboard/index.html.erb @@ -93,6 +93,25 @@ <% if assignment.contest_instance.judging_open?(current_user) %> <% assigned_round = @assigned_rounds.find_by(contest_instance_id: assignment.contest_instance.id, active: true) %> <% if assigned_round %> + <% ranked_count = EntryRanking.where(judging_round: assigned_round, user: current_user).count %> + <% can_notify = ranked_count >= assigned_round.required_entries_count %> +
+
+
+ <%= button_to 'I\'ve completed my evaluations', + notify_completed_container_contest_description_contest_instance_judging_round_path( + assignment.contest_instance.contest_description.container, + assignment.contest_instance.contest_description, + assignment.contest_instance, + assigned_round + ), + method: :post, + class: "btn btn-success", + disabled: !can_notify, + title: can_notify ? 'Notify the contest contact that you have finished' : "Rank at least #{assigned_round.required_entries_count} entries to enable this button", + data: { turbo_confirm: "Notify the contest contact that you have completed your evaluations for this round? They will receive an email with your contact information." } %> +
+
Date: Wed, 11 Feb 2026 11:09:58 -0500 Subject: [PATCH 04/15] Refactor ranking count check in JudgingRoundsController - Introduced a local variable `ranked_count` to store the count of ranked entries, improving readability. - Updated the conditional message to use the new variable, maintaining clarity in user notifications. These changes enhance code maintainability and ensure accurate messaging for judges during the ranking process. --- app/controllers/judging_rounds_controller.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/judging_rounds_controller.rb b/app/controllers/judging_rounds_controller.rb index a196fa1..be391f4 100644 --- a/app/controllers/judging_rounds_controller.rb +++ b/app/controllers/judging_rounds_controller.rb @@ -335,8 +335,10 @@ def notify_completed user: current_user ) - if entry_rankings.count < @judging_round.required_entries_count - message = "Please rank at least #{@judging_round.required_entries_count} entries before notifying. You have #{entry_rankings.count} ranked." + ranked_count = entry_rankings.count + + if ranked_count < @judging_round.required_entries_count + message = "Please rank at least #{@judging_round.required_entries_count} entries before notifying. You have #{ranked_count} ranked." respond_to do |format| format.html { redirect_to judge_dashboard_path, alert: message } format.turbo_stream do From 0dc4f295e8969db097c30c9100217fbb65074634 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Wed, 11 Feb 2026 11:16:00 -0500 Subject: [PATCH 05/15] Enhance EntryDragController with loading overlay functionality improvements - Added comprehensive tests for `addLoadingOverlay` and `removeLoadingOverlay` methods to ensure correct behavior and UI updates. - Implemented per-element timeout management for slow connection warnings, allowing multiple overlays to clean up correctly. - Improved error handling in the overlay removal process, including a visual indication of errors during updates. These enhancements provide better user feedback and maintainability for the loading overlay functionality during entry ranking updates. --- .../controllers/entry_drag_controller.js | 21 +- .../controllers/entry_drag_controller.spec.js | 220 ++++++++++++++++++ 2 files changed, 233 insertions(+), 8 deletions(-) diff --git a/app/javascript/controllers/entry_drag_controller.js b/app/javascript/controllers/entry_drag_controller.js index 47d5df5..d84f0c0 100644 --- a/app/javascript/controllers/entry_drag_controller.js +++ b/app/javascript/controllers/entry_drag_controller.js @@ -96,6 +96,7 @@ export default class extends Controller { this.contestGroupName = `entries-${accordionSection.id}` this.accordionSection = accordionSection + this.slowConnectionTimeouts = new WeakMap() this.initializeSortable() this.initializeCommentListeners() @@ -491,8 +492,8 @@ export default class extends Controller { // Show overlay immediately so it's visible for button clicks (card is about to move) overlay.classList.add('show') - // Set up slow connection warning - this.slowConnectionTimeout = setTimeout(() => { + // Set up slow connection warning (per-element so multiple overlays clean up correctly) + const timeoutId = setTimeout(() => { const statusText = overlay.querySelector('.status-text') if (statusText) { statusText.innerHTML = ` @@ -507,6 +508,7 @@ export default class extends Controller { ` } }, 5000) // Show warning after 5 seconds + this.slowConnectionTimeouts.set(element, timeoutId) } // Helper method to show success state on the overlay before removing it @@ -516,9 +518,10 @@ export default class extends Controller { const overlay = element.querySelector('.entry-loading-overlay') if (!overlay) return - if (this.slowConnectionTimeout) { - clearTimeout(this.slowConnectionTimeout) - this.slowConnectionTimeout = null + const timeoutId = this.slowConnectionTimeouts.get(element) + if (timeoutId) { + clearTimeout(timeoutId) + this.slowConnectionTimeouts.delete(element) } overlay.classList.add('success') @@ -540,9 +543,11 @@ export default class extends Controller { element.classList.remove('entry-card-loading') - // Clear slow connection timeout - if (this.slowConnectionTimeout) { - clearTimeout(this.slowConnectionTimeout) + // Clear slow connection timeout for this element + const timeoutId = this.slowConnectionTimeouts.get(element) + if (timeoutId) { + clearTimeout(timeoutId) + this.slowConnectionTimeouts.delete(element) } if (error) { diff --git a/spec/javascript/controllers/entry_drag_controller.spec.js b/spec/javascript/controllers/entry_drag_controller.spec.js index fefb633..ad2a221 100644 --- a/spec/javascript/controllers/entry_drag_controller.spec.js +++ b/spec/javascript/controllers/entry_drag_controller.spec.js @@ -128,6 +128,226 @@ describe("EntryDragController", () => { }) }) + describe("addLoadingOverlay", () => { + it("adds overlay with spinner and 'Updating...' text", () => { + const card = document.createElement('div') + card.setAttribute('data-entry-id', '1') + document.body.appendChild(card) + + controller.addLoadingOverlay(card) + + const overlay = card.querySelector('.entry-loading-overlay') + expect(overlay).not.toBeNull() + expect(overlay.querySelector('.spinner-border')).not.toBeNull() + expect(overlay.textContent).toMatch(/Updating\.\.\./) + expect(overlay.classList.contains('show')).toBe(true) + expect(card.classList.contains('entry-card-loading')).toBe(true) + + controller.removeLoadingOverlay(card) + document.body.removeChild(card) + }) + + it("shows overlay immediately with 'show' class", () => { + const card = document.createElement('div') + card.setAttribute('data-entry-id', '1') + document.body.appendChild(card) + + controller.addLoadingOverlay(card) + + const overlay = card.querySelector('.entry-loading-overlay') + expect(overlay.classList.contains('show')).toBe(true) + + controller.removeLoadingOverlay(card) + document.body.removeChild(card) + }) + + it("schedules slow connection warning after 5 seconds", () => { + jest.useFakeTimers() + const card = document.createElement('div') + card.setAttribute('data-entry-id', '1') + document.body.appendChild(card) + + controller.addLoadingOverlay(card) + + const overlay = card.querySelector('.entry-loading-overlay') + const statusText = overlay.querySelector('.status-text') + expect(statusText.textContent).toMatch(/Updating\.\.\./) + + jest.advanceTimersByTime(5000) + + expect(statusText.innerHTML).toMatch(/Slow connection detected/) + expect(statusText.innerHTML).toMatch(/Still working/) + + jest.useRealTimers() + controller.removeLoadingOverlay(card) + document.body.removeChild(card) + }) + + it("does not add duplicate overlay if one already exists", () => { + const card = document.createElement('div') + card.setAttribute('data-entry-id', '1') + document.body.appendChild(card) + + controller.addLoadingOverlay(card) + const overlayCount1 = card.querySelectorAll('.entry-loading-overlay').length + + controller.addLoadingOverlay(card) + const overlayCount2 = card.querySelectorAll('.entry-loading-overlay').length + + expect(overlayCount1).toBe(1) + expect(overlayCount2).toBe(1) + + controller.removeLoadingOverlay(card) + document.body.removeChild(card) + }) + + it("does nothing when element is null", () => { + expect(() => controller.addLoadingOverlay(null)).not.toThrow() + }) + }) + + describe("removeLoadingOverlay", () => { + it("removes overlay and entry-card-loading class", () => { + const card = document.createElement('div') + card.setAttribute('data-entry-id', '1') + document.body.appendChild(card) + + controller.addLoadingOverlay(card) + expect(card.querySelector('.entry-loading-overlay')).not.toBeNull() + expect(card.classList.contains('entry-card-loading')).toBe(true) + + jest.useFakeTimers() + controller.removeLoadingOverlay(card) + jest.advanceTimersByTime(400) // Wait for transition + remove + + expect(card.querySelector('.entry-loading-overlay')).toBeNull() + expect(card.classList.contains('entry-card-loading')).toBe(false) + + jest.useRealTimers() + document.body.removeChild(card) + }) + + it("clears slow connection timeout so callback never runs", () => { + jest.useFakeTimers() + const card = document.createElement('div') + card.setAttribute('data-entry-id', '1') + document.body.appendChild(card) + + controller.addLoadingOverlay(card) + controller.removeLoadingOverlay(card) + jest.advanceTimersByTime(6000) // Past the 5s slow connection threshold + + const overlay = card.querySelector('.entry-loading-overlay') + // Overlay is removed, but if timeout had fired it would have updated statusText before removal + // The key assertion: no warning text was shown (overlay was removed before timeout) + expect(card.querySelector('.entry-loading-overlay')).toBeNull() + + jest.useRealTimers() + document.body.removeChild(card) + }) + + it("shows error state when error is true", () => { + const card = document.createElement('div') + card.setAttribute('data-entry-id', '1') + document.body.appendChild(card) + + controller.addLoadingOverlay(card) + controller.removeLoadingOverlay(card, true) + + const overlay = card.querySelector('.entry-loading-overlay') + expect(overlay.classList.contains('error')).toBe(true) + expect(overlay.textContent).toMatch(/Error updating entry/) + + jest.useFakeTimers() + jest.advanceTimersByTime(2500) // Error overlay removes after 2s + jest.useRealTimers() + document.body.removeChild(card) + }) + + it("does nothing when element is null", () => { + expect(() => controller.removeLoadingOverlay(null)).not.toThrow() + expect(() => controller.removeLoadingOverlay(null, true)).not.toThrow() + }) + }) + + describe("per-element timeout cleanup", () => { + it("tracks timeouts per element so each overlay cleans up correctly", () => { + jest.useFakeTimers() + + const cardA = document.createElement('div') + cardA.setAttribute('data-entry-id', '1') + document.body.appendChild(cardA) + + const cardB = document.createElement('div') + cardB.setAttribute('data-entry-id', '2') + document.body.appendChild(cardB) + + controller.addLoadingOverlay(cardA) + controller.addLoadingOverlay(cardB) + + const overlayA = cardA.querySelector('.entry-loading-overlay') + const overlayB = cardB.querySelector('.entry-loading-overlay') + expect(overlayA.querySelector('.status-text').textContent).toMatch(/Updating\.\.\./) + expect(overlayB.querySelector('.status-text').textContent).toMatch(/Updating\.\.\./) + + // Remove A's overlay - only A's timeout should be cleared + controller.removeLoadingOverlay(cardA) + jest.advanceTimersByTime(400) // Let A's overlay removal complete + + expect(cardA.querySelector('.entry-loading-overlay')).toBeNull() + expect(cardB.querySelector('.entry-loading-overlay')).not.toBeNull() + + // After 5s, only B's slow connection callback should fire + jest.advanceTimersByTime(5000) + + const statusB = cardB.querySelector('.status-text') + expect(statusB.innerHTML).toMatch(/Slow connection detected/) + // Card A had no overlay, so its timeout was cleared and never ran + + jest.useRealTimers() + controller.removeLoadingOverlay(cardB) + document.body.removeChild(cardA) + document.body.removeChild(cardB) + }) + + it("showSuccessOverlay clears timeout for that element only", () => { + jest.useFakeTimers() + + const cardA = document.createElement('div') + cardA.setAttribute('data-entry-id', '1') + cardA.innerHTML = ` +
+ +
+ ` + document.body.appendChild(cardA) + + const cardB = document.createElement('div') + cardB.setAttribute('data-entry-id', '2') + document.body.appendChild(cardB) + + controller.addLoadingOverlay(cardB) + + // Manually add A to the WeakMap to simulate A having had a timeout (showSuccessOverlay + // is called when overlay already exists; we're testing that it clears A's timeout) + const timeoutA = setTimeout(() => {}, 5000) + controller.slowConnectionTimeouts.set(cardA, timeoutA) + + controller.showSuccessOverlay(cardA) + + // A's timeout should be cleared; B's should still be scheduled + expect(controller.slowConnectionTimeouts.has(cardA)).toBe(false) + expect(controller.slowConnectionTimeouts.has(cardB)).toBe(true) + + jest.useRealTimers() + controller.removeLoadingOverlay(cardB) + document.body.removeChild(cardA) + document.body.removeChild(cardB) + }) + }) + describe("showSuccessOverlay", () => { it("replaces overlay content with success state and 'Ranking recorded' text", () => { const card = document.createElement('div') From b863d05c121daee82ffac91ee1641bebfeb5478d Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Wed, 11 Feb 2026 11:19:15 -0500 Subject: [PATCH 06/15] Optimize JudgeDashboard for performance and clarity - Precomputed ranked entry counts and finalized statuses for each judging round to avoid N+1 query issues, enhancing performance. - Updated the view to utilize precomputed data, improving readability and maintainability of the code. These changes streamline data retrieval and improve the user experience on the judge dashboard. --- app/controllers/judge_dashboard_controller.rb | 15 +++++++++++++++ app/views/judge_dashboard/index.html.erb | 7 ++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/app/controllers/judge_dashboard_controller.rb b/app/controllers/judge_dashboard_controller.rb index aafd151..1f36f85 100644 --- a/app/controllers/judge_dashboard_controller.rb +++ b/app/controllers/judge_dashboard_controller.rb @@ -39,5 +39,20 @@ def index .where(user: current_user) .joins(judging_round: :contest_instance) .where(judging_rounds: { id: @assigned_rounds.pluck(:id) }) + + # Precompute ranked entry counts per judging_round for this user to avoid N+1 queries + @ranked_counts_by_round = EntryRanking.where( + user: current_user, + judging_round_id: @assigned_rounds.pluck(:id) + ).group(:judging_round_id).count + + # Precompute finalized status per judging_round to avoid N+1 queries + # Check if any EntryRankings are finalized for each round + finalized_round_ids = EntryRanking.where( + user: current_user, + judging_round_id: @assigned_rounds.pluck(:id), + finalized: true + ).select(:judging_round_id).distinct.pluck(:judging_round_id) + @finalized_by_round = finalized_round_ids.index_with { true } end end diff --git a/app/views/judge_dashboard/index.html.erb b/app/views/judge_dashboard/index.html.erb index 4759e01..b610b0d 100644 --- a/app/views/judge_dashboard/index.html.erb +++ b/app/views/judge_dashboard/index.html.erb @@ -93,7 +93,7 @@ <% if assignment.contest_instance.judging_open?(current_user) %> <% assigned_round = @assigned_rounds.find_by(contest_instance_id: assignment.contest_instance.id, active: true) %> <% if assigned_round %> - <% ranked_count = EntryRanking.where(judging_round: assigned_round, user: current_user).count %> + <% ranked_count = @ranked_counts_by_round[assigned_round.id].to_i %> <% can_notify = ranked_count >= assigned_round.required_entries_count %>
@@ -120,10 +120,7 @@ assigned_round ) %>" data-entry-drag-required-count-value="<%= assigned_round.required_entries_count %>" - data-entry-drag-finalized-value="<%= EntryRanking.where( - judging_round: assigned_round, - user: current_user - ).any?(&:finalized?) %>"> + data-entry-drag-finalized-value="<%= @finalized_by_round[assigned_round.id].present? %>">
<%= render partial: 'judge_dashboard/available_entries', From fb877b52ffa9b791a997d9680748dd2f2ec81013 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Wed, 11 Feb 2026 11:20:42 -0500 Subject: [PATCH 07/15] Refactor ContestInstancePolicy to enhance ranking checks - Updated the `update_rankings?`, `finalize_rankings?`, and `notify_completed?` methods to pass the user as an argument to `judging_open?`, improving the accuracy of access control. - This change ensures that only authorized judges can update rankings or finalize them, enhancing security and clarity in the policy logic. These modifications strengthen the policy checks for contest instances, ensuring proper authorization based on user context. --- app/policies/contest_instance_policy.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/policies/contest_instance_policy.rb b/app/policies/contest_instance_policy.rb index 76b5b27..c55c4f9 100644 --- a/app/policies/contest_instance_policy.rb +++ b/app/policies/contest_instance_policy.rb @@ -55,13 +55,13 @@ def manage? def update_rankings? return false unless user && record - return false unless record.judging_open? + return false unless record.judging_open?(user) record.judges.include?(user) end def finalize_rankings? return false unless user && record - return false unless record.judging_open? + return false unless record.judging_open?(user) record.judges.include?(user) end @@ -95,7 +95,7 @@ def send_instructions? def notify_completed? return false unless user && record - return false unless record.judging_open? + return false unless record.judging_open?(user) record.judges.include?(user) end end From f560be4a0de0da576365dc59df27e007e91ec5ff Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Wed, 11 Feb 2026 18:46:32 -0500 Subject: [PATCH 08/15] Enhance ranking update logic in JudgingRoundsController - Updated the handling of single entry updates to accommodate both comment updates and unranking scenarios. - Added conditions to manage cases where the rank is explicitly sent as blank, allowing for proper unranking of entries. - Improved comment-only updates by ensuring existing rankings are updated without altering the rank. These changes enhance the flexibility and clarity of the ranking process, ensuring accurate handling of entry updates. --- app/controllers/judging_rounds_controller.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/app/controllers/judging_rounds_controller.rb b/app/controllers/judging_rounds_controller.rb index be391f4..b788a8b 100644 --- a/app/controllers/judging_rounds_controller.rb +++ b/app/controllers/judging_rounds_controller.rb @@ -196,11 +196,11 @@ def update_rankings entry_ranking.save(validate: false) end else - # This is a single entry update (comment update) + # This is a single entry update (comment update or unrank) ranking_data = rankings.first entry_id = ranking_data['entry_id'].presence || ranking_data[:entry_id].presence - if entry_id && ranking_data['rank'].present? + if entry_id && (ranking_data['rank'].present? || ranking_data[:rank].present?) entry = Entry.find(entry_id) entry_ranking = EntryRanking.find_or_initialize_by( entry: entry, @@ -212,6 +212,17 @@ def update_rankings entry_ranking.internal_comments = ranking_data['internal_comments'].presence || ranking_data[:internal_comments].presence || entry_ranking.internal_comments entry_ranking.external_comments = ranking_data['external_comments'].presence || ranking_data[:external_comments].presence || entry_ranking.external_comments entry_ranking.save(validate: false) + elsif entry_id && (ranking_data.key?('rank') || ranking_data.key?(:rank)) && ranking_data['rank'].blank? && ranking_data[:rank].blank? + # Unranking: only when rank was explicitly sent and is blank (not when key omitted) + current_rankings.where(entry_id: entry_id).destroy_all + elsif entry_id && !(ranking_data.key?('rank') || ranking_data.key?(:rank)) + # Comment-only update: rank key omitted; update comments on existing ranking only + entry_ranking = current_rankings.find_by(entry_id: entry_id) + if entry_ranking + entry_ranking.internal_comments = ranking_data['internal_comments'].presence || ranking_data[:internal_comments].presence || entry_ranking.internal_comments + entry_ranking.external_comments = ranking_data['external_comments'].presence || ranking_data[:external_comments].presence || entry_ranking.external_comments + entry_ranking.save(validate: false) + end end end From 329a0231fe537e2ec42f2b8156c8f187743f687d Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Thu, 12 Feb 2026 09:52:30 -0500 Subject: [PATCH 09/15] Add tests for update_rankings action in JudgingRoundsController - Introduced a new test suite for the `update_rankings` action to validate the unranking functionality when a single entry ranking is sent with a null rank. - Ensured that the entry ranking is correctly removed from the database and that the response indicates success. - This addition enhances test coverage for ranking updates, ensuring robust handling of entry rankings in various scenarios. --- .../judging_rounds_controller_spec.rb | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/spec/controllers/judging_rounds_controller_spec.rb b/spec/controllers/judging_rounds_controller_spec.rb index f068a3f..a34fae0 100644 --- a/spec/controllers/judging_rounds_controller_spec.rb +++ b/spec/controllers/judging_rounds_controller_spec.rb @@ -509,6 +509,53 @@ end end + describe 'POST #update_rankings' do + let(:contest_instance) do + create(:contest_instance, + contest_description: contest_description, + date_open: 2.months.ago, + date_closed: 1.month.ago, + active: true) + end + let(:judging_round) do + create(:judging_round, + contest_instance: contest_instance, + round_number: 1, + active: true, + start_date: 1.day.ago, + end_date: 1.day.from_now, + required_entries_count: 3) + end + let(:judge_user) { create(:user, :with_judge_role) } + let!(:judging_assignment) { create(:judging_assignment, user: judge_user, contest_instance: contest_instance, active: true) } + let!(:round_judge_assignment) { create(:round_judge_assignment, user: judge_user, judging_round: judging_round, active: true) } + let!(:entry) { create(:entry, contest_instance: contest_instance, deleted: false) } + + before { sign_in judge_user } + + context 'with only one ranked entry' do + before do + create(:entry_ranking, entry: entry, judging_round: judging_round, user: judge_user, rank: 1) + end + + it 'unranks the entry when a single ranking with rank null is sent' do + expect { + post :update_rankings, params: { + container_id: container.id, + contest_description_id: contest_description.id, + contest_instance_id: contest_instance.id, + id: judging_round.id, + rankings: [{ entry_id: entry.id.to_s, rank: nil, internal_comments: nil, external_comments: nil }] + }, format: :json + }.to change(EntryRanking, :count).by(-1) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['success']).to eq(true) + expect(EntryRanking.find_by(entry: entry, judging_round: judging_round, user: judge_user)).to be_nil + end + end + end + describe 'POST #notify_completed' do let(:container_with_contact) { create(:container, contact_email: 'contest_contact@umich.edu') } let(:contest_description) { create(:contest_description, :active, container: container_with_contact, name: 'Test Contest') } From c2ddd8a7b7bb1cdc95dae9ed1cec04f97f27f490 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Mon, 16 Feb 2026 14:05:28 -0500 Subject: [PATCH 10/15] Remove authorization check for notify_completed action in JudgingRoundsController - Eliminated the authorization call for the `notify_completed` method, streamlining the notification process for contest contacts. - This change simplifies the controller logic, allowing for easier management of notifications without unnecessary authorization overhead. These modifications enhance the functionality of the notification system within the judging rounds process. --- app/controllers/judging_rounds_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/judging_rounds_controller.rb b/app/controllers/judging_rounds_controller.rb index b788a8b..c4e9e45 100644 --- a/app/controllers/judging_rounds_controller.rb +++ b/app/controllers/judging_rounds_controller.rb @@ -339,8 +339,6 @@ def finalize_rankings end def notify_completed - authorize @contest_instance, :notify_completed? - entry_rankings = EntryRanking.where( judging_round: @judging_round, user: current_user From b2c2cf1daeb945ff3261650348ef5b398b6e92ac Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Mon, 16 Feb 2026 14:06:52 -0500 Subject: [PATCH 11/15] Add error handling for missing contact email in JudgeCompletedEvaluationsMailer - Implemented an ArgumentError raise when the contact_email is not provided, ensuring that the mailer fails gracefully with a clear message. - This change enhances the robustness of the mailer by enforcing the requirement for a valid contact email before proceeding with notifications. These modifications improve error handling and clarity in the email notification process for judges. --- app/mailers/judge_completed_evaluations_mailer.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/mailers/judge_completed_evaluations_mailer.rb b/app/mailers/judge_completed_evaluations_mailer.rb index 673341a..359e68b 100644 --- a/app/mailers/judge_completed_evaluations_mailer.rb +++ b/app/mailers/judge_completed_evaluations_mailer.rb @@ -10,7 +10,9 @@ def notify_contact(judge, judging_round) @judge_email = judge.normalize_email to_email = @container.contact_email.presence - return unless to_email + unless to_email + raise ArgumentError, "contact_email is required for JudgeCompletedEvaluationsMailer.notify_contact" + end subject = "Judge completed evaluations: #{@contest_description.name} - Round #{@judging_round.round_number}" From 89865c5a67d9ddc00fd8915ebfe1c3a47e39d5a6 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Mon, 16 Feb 2026 14:07:49 -0500 Subject: [PATCH 12/15] Refactor JudgeDashboardController to optimize entry ranking queries - Introduced a local variable `assigned_round_ids` to store the IDs of assigned rounds, improving code readability and reducing repeated calls to `pluck`. - Updated queries for entry rankings and finalized statuses to utilize the new variable, enhancing performance by avoiding N+1 query issues. These changes streamline data retrieval and improve the clarity of the Judge Dashboard logic. --- app/controllers/judge_dashboard_controller.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/judge_dashboard_controller.rb b/app/controllers/judge_dashboard_controller.rb index 1f36f85..20d6711 100644 --- a/app/controllers/judge_dashboard_controller.rb +++ b/app/controllers/judge_dashboard_controller.rb @@ -35,22 +35,24 @@ def index ) .where(contest_instance_id: @judging_assignments.pluck(:contest_instance_id)) + assigned_round_ids = @assigned_rounds.pluck(:id) + @entry_rankings = EntryRanking.includes(:judging_round) .where(user: current_user) .joins(judging_round: :contest_instance) - .where(judging_rounds: { id: @assigned_rounds.pluck(:id) }) + .where(judging_rounds: { id: assigned_round_ids }) # Precompute ranked entry counts per judging_round for this user to avoid N+1 queries @ranked_counts_by_round = EntryRanking.where( user: current_user, - judging_round_id: @assigned_rounds.pluck(:id) + judging_round_id: assigned_round_ids ).group(:judging_round_id).count # Precompute finalized status per judging_round to avoid N+1 queries # Check if any EntryRankings are finalized for each round finalized_round_ids = EntryRanking.where( user: current_user, - judging_round_id: @assigned_rounds.pluck(:id), + judging_round_id: assigned_round_ids, finalized: true ).select(:judging_round_id).distinct.pluck(:judging_round_id) @finalized_by_round = finalized_round_ids.index_with { true } From 012c88cb0d046b64bdeb75a931115a5eca7b11e2 Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Mon, 16 Feb 2026 14:09:58 -0500 Subject: [PATCH 13/15] Refactor entry ranking logic in JudgingRoundsController for improved clarity and performance - Simplified the ranking update process by consolidating logic for handling both ranking and unranking scenarios. - Enhanced the response structure to provide clearer feedback on ranking updates, ensuring better user experience. - These changes streamline the controller's functionality, making it easier to manage entry rankings while maintaining performance. --- spec/policies/contest_instance_policy_spec.rb | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 spec/policies/contest_instance_policy_spec.rb diff --git a/spec/policies/contest_instance_policy_spec.rb b/spec/policies/contest_instance_policy_spec.rb new file mode 100644 index 0000000..f03e7e5 --- /dev/null +++ b/spec/policies/contest_instance_policy_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ContestInstancePolicy do + subject { described_class.new(user, contest_instance) } + + let(:contest_instance) { create(:contest_instance, date_open: 10.days.ago, date_closed: 5.days.ago) } + + # Judging-open gating: policies that require user to be a judge AND judging_open?(user) + shared_examples 'judging_open? gated judge action' do |action| + context 'when user is nil' do + let(:user) { nil } + + it "forbids #{action}" do + expect(subject).not_to permit_action(action) + end + end + + context 'when user is not a judge for this instance' do + let(:user) { create(:user, :with_judge_role) } + + it "forbids #{action}" do + expect(subject).not_to permit_action(action) + end + end + + context 'when user is a judge but not assigned to the current round' do + let(:user) { create(:user, :with_judge_role) } + let!(:judging_round) do + create(:judging_round, + contest_instance: contest_instance, + active: true, + start_date: 4.days.ago, + end_date: 2.days.from_now + ) + end + + before do + create(:judging_assignment, user: user, contest_instance: contest_instance, active: true) + # No round_judge_assignment -> judging_open?(user) is false + end + + it "forbids #{action}" do + expect(subject).not_to permit_action(action) + end + end + + context 'when there is no current judging round' do + let(:user) { create(:user, :with_judge_role) } + let!(:judging_round) do + create(:judging_round, + contest_instance: contest_instance, + active: true, + start_date: 1.day.from_now, + end_date: 3.days.from_now + ) + end + + before do + create(:judging_assignment, user: user, contest_instance: contest_instance, active: true) + create(:round_judge_assignment, user: user, judging_round: judging_round, active: true) + end + + it "forbids #{action}" do + expect(subject).not_to permit_action(action) + end + end + + context 'when user is a judge and judging is open for them' do + let(:user) { create(:user, :with_judge_role) } + let!(:judging_round) do + create(:judging_round, + contest_instance: contest_instance, + active: true, + start_date: 4.days.ago, + end_date: 2.days.from_now + ) + end + + before do + create(:judging_assignment, user: user, contest_instance: contest_instance, active: true) + create(:round_judge_assignment, user: user, judging_round: judging_round, active: true) + end + + it "permits #{action}" do + expect(subject).to permit_action(action) + end + end + end + + describe '#notify_completed?' do + include_examples 'judging_open? gated judge action', :notify_completed + end + + describe '#update_rankings?' do + include_examples 'judging_open? gated judge action', :update_rankings + end + + describe '#finalize_rankings?' do + include_examples 'judging_open? gated judge action', :finalize_rankings + end +end From e6c3cd6f57dbe4237d6538d0404a94bcfb267ebb Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Mon, 16 Feb 2026 14:15:53 -0500 Subject: [PATCH 14/15] Enhance tests for EntryDragController by utilizing jest fake timers - Added jest.useFakeTimers() calls in multiple test cases to ensure accurate timing for overlay removal and error state display. - This change improves the reliability of tests by simulating timer behavior, allowing for better validation of UI interactions in the EntryDragController. These modifications enhance test coverage and ensure proper functionality of dynamic UI elements. --- spec/javascript/controllers/entry_drag_controller.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/javascript/controllers/entry_drag_controller.spec.js b/spec/javascript/controllers/entry_drag_controller.spec.js index ad2a221..d79509a 100644 --- a/spec/javascript/controllers/entry_drag_controller.spec.js +++ b/spec/javascript/controllers/entry_drag_controller.spec.js @@ -208,6 +208,7 @@ describe("EntryDragController", () => { describe("removeLoadingOverlay", () => { it("removes overlay and entry-card-loading class", () => { + jest.useFakeTimers() const card = document.createElement('div') card.setAttribute('data-entry-id', '1') document.body.appendChild(card) @@ -216,7 +217,6 @@ describe("EntryDragController", () => { expect(card.querySelector('.entry-loading-overlay')).not.toBeNull() expect(card.classList.contains('entry-card-loading')).toBe(true) - jest.useFakeTimers() controller.removeLoadingOverlay(card) jest.advanceTimersByTime(400) // Wait for transition + remove @@ -247,6 +247,7 @@ describe("EntryDragController", () => { }) it("shows error state when error is true", () => { + jest.useFakeTimers() const card = document.createElement('div') card.setAttribute('data-entry-id', '1') document.body.appendChild(card) @@ -258,7 +259,6 @@ describe("EntryDragController", () => { expect(overlay.classList.contains('error')).toBe(true) expect(overlay.textContent).toMatch(/Error updating entry/) - jest.useFakeTimers() jest.advanceTimersByTime(2500) // Error overlay removes after 2s jest.useRealTimers() document.body.removeChild(card) From ef9b81dca5917e2bb772d667457d6283142b7acc Mon Sep 17 00:00:00 2001 From: rsmokeUM Date: Mon, 16 Feb 2026 14:24:58 -0500 Subject: [PATCH 15/15] Refactor entry loading overlay handling in EntryDragController - Updated the logic for removing the loading overlay and the associated class to ensure that the class is only removed after the overlay is fully transitioned out. This prevents clipping during the transition and maintains proper styling. - Introduced a new method, `removeOverlayAndClass`, to encapsulate the removal logic, enhancing code clarity and maintainability. These changes improve the user experience by ensuring smoother transitions for loading states in the UI. --- app/javascript/controllers/entry_drag_controller.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/javascript/controllers/entry_drag_controller.js b/app/javascript/controllers/entry_drag_controller.js index d84f0c0..512ee01 100644 --- a/app/javascript/controllers/entry_drag_controller.js +++ b/app/javascript/controllers/entry_drag_controller.js @@ -541,8 +541,6 @@ export default class extends Controller { const overlay = element.querySelector('.entry-loading-overlay') if (!overlay) return - element.classList.remove('entry-card-loading') - // Clear slow connection timeout for this element const timeoutId = this.slowConnectionTimeouts.get(element) if (timeoutId) { @@ -550,6 +548,13 @@ export default class extends Controller { this.slowConnectionTimeouts.delete(element) } + // Remove entry-card-loading only when the overlay is removed, so the overlay + // keeps overflow/z-index styling until it's gone (avoids clipping during transition/error). + const removeOverlayAndClass = () => { + element.classList.remove('entry-card-loading') + overlay.remove() + } + if (error) { overlay.classList.add('error') const statusText = overlay.querySelector('.status-text') @@ -566,12 +571,12 @@ export default class extends Controller { // Remove error overlay after 2 seconds setTimeout(() => { overlay.classList.remove('show') - setTimeout(() => overlay.remove(), 300) + setTimeout(removeOverlayAndClass, 300) }, 2000) } else { overlay.classList.remove('show') // Remove overlay after transition completes - setTimeout(() => overlay.remove(), 300) + setTimeout(removeOverlayAndClass, 300) } } }