From 032a9efee53603df9e0a2e32d05e7c39a274be52 Mon Sep 17 00:00:00 2001
From: Stephen Chudleigh <stephen@smartlogic.io>
Date: Fri, 8 Nov 2024 11:22:30 -0800
Subject: [PATCH] merge origin/dev

---
 app/controllers/manage_submissions_controller.rb | 10 ++++++----
 app/helpers/manage_submissions_helper.rb         |  8 ++------
 .../manage_submissions/_phases_table.html.erb    |  4 ++--
 .../_submissions_table.html.erb                  | 10 +++++-----
 .../manage_submissions/by_challenge_phase.erb    | 16 ----------------
 .../by_challenge_phase.html.erb                  | 10 ++++++++++
 app/views/manage_submissions/show.html.erb       | 14 ++++----------
 config/routes.rb                                 | 10 ++++++++--
 spec/requests/manage_submissions_spec.rb         | 16 +++++++---------
 spec/system/manage_submissions_spec.rb           |  4 +++-
 10 files changed, 47 insertions(+), 55 deletions(-)
 delete mode 100644 app/views/manage_submissions/by_challenge_phase.erb
 create mode 100644 app/views/manage_submissions/by_challenge_phase.html.erb

diff --git a/app/controllers/manage_submissions_controller.rb b/app/controllers/manage_submissions_controller.rb
index 5b5f736e..4d08388a 100644
--- a/app/controllers/manage_submissions_controller.rb
+++ b/app/controllers/manage_submissions_controller.rb
@@ -7,7 +7,9 @@ def index
   end
 
   def show
-    @submission = Submission.find(params[:id])
+    @challenge = current_user.challenge_manager_challenges.find(params[:challenge_id])
+    @phase = @challenge.phases.find(params[:phase_id])
+    @submission = @phase.submissions.find(params[:id])
   end
 
   def update
@@ -22,9 +24,9 @@ def update
   end
 
   def by_challenge_phase
-    @phase = Phase.where(id: params[:phase_id],
-                         challenge_id: current_user.challenge_manager_challenges.collect(&:id)).first
-    @submissions = @phase ? @phase.submissions : []
+    @challenge = current_user.challenge_manager_challenges.find(params[:challenge_id])
+    @phase = @challenge.phases.find(params[:id])
+    @submissions = @phase.submissions
   end
 
   def submission_params
diff --git a/app/helpers/manage_submissions_helper.rb b/app/helpers/manage_submissions_helper.rb
index 331a32ee..26876590 100644
--- a/app/helpers/manage_submissions_helper.rb
+++ b/app/helpers/manage_submissions_helper.rb
@@ -2,14 +2,10 @@
 
 module ManageSubmissionsHelper
   def eligible_for_evaluation?(submission)
-    submission.judging_status.in?(%w[qualified selected winner])
-  end
-
-  def selected_to_advance?(submission)
     submission.judging_status.in?(%w[selected winner])
   end
 
-  def assigned_to_user?(user, submission)
-    submission.challenge_id.in?(user.challenge_manager_challenges.collect(&:id))
+  def selected_to_advance?(submission)
+    submission.judging_status.in?(%w[winner])
   end
 end
diff --git a/app/views/manage_submissions/_phases_table.html.erb b/app/views/manage_submissions/_phases_table.html.erb
index 3120a17f..241f29c1 100644
--- a/app/views/manage_submissions/_phases_table.html.erb
+++ b/app/views/manage_submissions/_phases_table.html.erb
@@ -39,11 +39,11 @@
                   Manage Evaluators
                 </button>
                 <% unless phase.submissions.empty? %>
-                  <a href=<%= "/manage_submissions/by_challenge_phase/#{phase.id}" %>>
+                  <%= link_to(challenge_manage_submission_path(challenge, phase)) do %>
                     <button class="usa-button font-body-2xs text-no-wrap">
                       View Submissions
                     </button>
-                  </a>
+                  <% end %>
                 <% end %>  
               </div>  
             </td>  
diff --git a/app/views/manage_submissions/_submissions_table.html.erb b/app/views/manage_submissions/_submissions_table.html.erb
index 8cb9a222..ad6ba9cd 100644
--- a/app/views/manage_submissions/_submissions_table.html.erb
+++ b/app/views/manage_submissions/_submissions_table.html.erb
@@ -24,7 +24,7 @@
                             class: "usa-icon--size-3 margin-right-1",
                             alt: ""
                         )%>
-                        Eligible for Evaluation
+                        <span class="text-top">Eligible for Evaluation</span>
                     </div>    
                 <% else  %>
                     <input type="checkbox" disabled class="display-none mobile-lg:display-block">
@@ -34,7 +34,7 @@
                             class: "usa-icon--size-3 margin-right-1",
                             alt: ""
                         )%>
-                        Not Eligible for Evaluation
+                        <span class="text-top">Not Eligible for Evaluation</span>
                     </div>    
                 <% end %>
               </div>  
@@ -49,7 +49,7 @@
                             class: "usa-icon--size-3 margin-right-1",
                             alt: ""
                         )%>
-                        Selected to Advance
+                        <span class="text-top">Selected to Advance</span>
                     </div>
                 <% else  %>
                     <input type="checkbox" disabled class="display-none mobile-lg:display-block">
@@ -59,13 +59,13 @@
                             class: "usa-icon--size-3 margin-right-1",
                             alt: ""
                         )%>
-                        Not Selected to Advance
+                        <span class="text-top">Not Selected to Advance</span>
                     </div>    
                 <% end %>
               </div>  
             </td>
             <td data-label="Assigned Evaluators">
-                No evaluators assigned to this submission.
+                No evaluators assigned to this submission
             </td>
             <td>
                 N/A
diff --git a/app/views/manage_submissions/by_challenge_phase.erb b/app/views/manage_submissions/by_challenge_phase.erb
deleted file mode 100644
index 1b047a43..00000000
--- a/app/views/manage_submissions/by_challenge_phase.erb
+++ /dev/null
@@ -1,16 +0,0 @@
-<% if @phase %>  
-  <h1><%= challenge_phase_title(@phase.challenge, @phase) %></h1>
-  <p class="text-normal">View challenge submissions and manage evaluation progress.</p>
-
-  <% if @submissions.empty? %>
-    <div class="text-normal">
-      <p>This challenge phase does not currently have any submissions.</p>
-    </div>
-  <% else %>
-    <%= render partial: "submissions_table", locals: { submissions: @submissions } %>
-  <% end %>
-<% else %>
-  <div class="text-normal">
-    <p>You are not assigned to this challenge.</p>
-  </div>
-<% end %>  
\ No newline at end of file
diff --git a/app/views/manage_submissions/by_challenge_phase.html.erb b/app/views/manage_submissions/by_challenge_phase.html.erb
new file mode 100644
index 00000000..16ab4729
--- /dev/null
+++ b/app/views/manage_submissions/by_challenge_phase.html.erb
@@ -0,0 +1,10 @@
+<h1><%= challenge_phase_title(@phase.challenge, @phase) %></h1>
+<p class="text-normal">View challenge submissions and manage evaluation progress.</p>
+
+<% if @submissions.empty? %>
+  <div class="text-normal">
+    <p>This challenge phase does not currently have any submissions.</p>
+  </div>
+<% else %>
+  <%= render partial: "submissions_table", locals: { submissions: @submissions } %>
+<% end %>
\ No newline at end of file
diff --git a/app/views/manage_submissions/show.html.erb b/app/views/manage_submissions/show.html.erb
index 4105d911..189557d8 100644
--- a/app/views/manage_submissions/show.html.erb
+++ b/app/views/manage_submissions/show.html.erb
@@ -1,11 +1,5 @@
-<% if assigned_to_user?(current_user, @submission) %>
-    <h1>Submission ID <%= @submission.id %></h1>
-    <p class="text-normal">View submission information and assign evaluators to evaluate the submission.</p>
+<h1>Submission ID <%= @submission.id %></h1>
+<p class="text-normal">View submission information and assign evaluators to evaluate the submission.</p>
 
-    <%= render partial: "submission_materials", locals: { submission: @submission } %>
-
-    <%= render partial: "comment_form", locals: { submission: @submission } %>
-
-<% else %>
-    <p class="text-normal">You are not assigned to manage this submission.</p>
-<% end %>        
\ No newline at end of file
+<%= render partial: "submission_materials", locals: { submission: @submission } %>
+<%= render partial: "comment_form", locals: { submission: @submission } %>
\ No newline at end of file
diff --git a/config/routes.rb b/config/routes.rb
index c2115d25..41199964 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -14,8 +14,14 @@
   get '/evaluation_forms/confirmation', to: 'evaluation_forms#confirmation'
   resources :evaluation_forms
   post '/evaluation_forms/clone', to: 'evaluation_forms#create_from_existing'
-  resources :manage_submissions, only: [:index, :show, :update]
-  get '/manage_submissions/by_challenge_phase/:phase_id', to: 'manage_submissions#by_challenge_phase'
+  resources :manage_submissions, only: [:index]
+  resources :challenges, only: [] do
+    resources :manage_submissions, only: [:show, :update] do
+      member do
+        get 'by_challenge_phase'
+      end
+    end
+  end
 
   # Reveal health status on /up that returns 200 if the app boots with no exceptions, otherwise 500.
   # Can be used by load balancers and uptime monitors to verify that the app is live.
diff --git a/spec/requests/manage_submissions_spec.rb b/spec/requests/manage_submissions_spec.rb
index 0fb08198..63d941d2 100644
--- a/spec/requests/manage_submissions_spec.rb
+++ b/spec/requests/manage_submissions_spec.rb
@@ -61,7 +61,7 @@
         challenge = create_challenge(user: challenge_user, title: "Boston Tea Party Cleanup")
         phase = create_phase(challenge_id: challenge.id)
 
-        get "/manage_submissions/by_challenge_phase/#{phase.id}"
+        get by_challenge_phase_challenge_manage_submission_path(challenge, phase)
         expect(response.body).to include("Boston Tea Party Cleanup")
 
         expect(response.body).to include("This challenge phase does not currently have any submissions.")
@@ -72,7 +72,7 @@
         phase = create_phase(challenge_id: challenge.id)
         submission = create(:submission, challenge: challenge)
 
-        get "/manage_submissions/by_challenge_phase/#{phase.id}"
+        get by_challenge_phase_challenge_manage_submission_path(challenge, phase)
         expect(response.body).to include("Boston Tea Party Cleanup")
         expect(response.body).to include(submission.id.to_s)
       end
@@ -81,9 +81,8 @@
         challenge = create_challenge(title: "Star Spangled Banister")
         phase = create_phase(challenge_id: challenge.id)
 
-        get "/manage_submissions/by_challenge_phase/#{phase.id}"
-        expect(response.body).not_to include("Star Spangled Banister")
-        expect(response.body).to include("You are not assigned to this challenge.")
+        get by_challenge_phase_challenge_manage_submission_path(challenge, phase)
+        expect(response).to have_http_status(:not_found)
       end
 
       it "renders a details page for an individual submission" do
@@ -91,7 +90,7 @@
         phase = create_phase(challenge_id: challenge.id)
         submission = create(:submission, challenge: phase.challenge, brief_description: "This submission has legs.")
 
-        get manage_submission_path(submission)
+        get challenge_manage_submission_path(challenge, submission, phase_id: phase.id)
         expect(response.body).to include(submission.id.to_s)
         expect(response.body).to include(submission.brief_description)
       end
@@ -101,9 +100,8 @@
         phase = create_phase(challenge_id: challenge.id)
         submission = create(:submission, challenge: phase.challenge, brief_description: "This submission has teeth.")
 
-        get manage_submission_path(submission)
-        expect(response.body).not_to include(submission.brief_description)
-        expect(response.body).to include("You are not assigned to manage this submission.")
+        get challenge_manage_submission_path(challenge, submission, phase_id: phase.id)
+        expect(response).to have_http_status(:not_found)
       end
     end
 
diff --git a/spec/system/manage_submissions_spec.rb b/spec/system/manage_submissions_spec.rb
index 12c40449..3b9c21ea 100644
--- a/spec/system/manage_submissions_spec.rb
+++ b/spec/system/manage_submissions_spec.rb
@@ -21,6 +21,7 @@
     it "manage submissions index page is accessible with one challenge" do
       challenge = create_challenge(user: user, title: "Boston Tea Party Cleanup")
       create_phase(challenge_id: challenge.id)
+
       visit manage_submissions_path
       expect(user.role).to eq("challenge_manager")
       expect(page).to have_content("Boston Tea Party Cleanup")
@@ -30,7 +31,8 @@
     it "manage submissions by challenge phase page is accessible with one challenge" do
       challenge = create_challenge(user: user, title: "Boston Tea Party Cleanup")
       phase = create_phase(challenge_id: challenge.id)
-      visit "/manage_submissions/by_challenge_phase/#{phase.id}"
+
+      visit by_challenge_phase_challenge_manage_submission_path(challenge, phase)
       expect(user.role).to eq("challenge_manager")
       expect(page).to have_content("Boston Tea Party Cleanup")
       expect(page).to(be_axe_clean)