Refactor permission checks and enhance access control across views

app/controllers/affiliated_resources_controller.rb

@@ -4,7 +4,11 @@ class AffiliatedResourcesController < ApplicationController
 
   def index
     @affiliated_resources = @department.affiliated_resources.ordered
-    authorize AffiliatedResource.new(department: @department)
+    affiliated_resource = AffiliatedResource.new(department: @department)
+    authorize affiliated_resource
+    @can_create_affiliated_resources = policy(affiliated_resource).create?
+    @can_update_affiliated_resources = policy(affiliated_resource).update?
+    @can_destroy_affiliated_resources = policy(affiliated_resource).destroy?
   end
 
   def new

app/controllers/appointments_controller.rb

@@ -5,6 +5,8 @@ class AppointmentsController < ApplicationController
   def index
     @appointments = @program.appointments.includes(:vip, :student).order(:start_time)
     authorize Appointment.new(program: @program)
+    @can_view_appointments = policy(Appointment.new(program: @program)).show?
+    @can_create_appointments = policy(Appointment.new(program: @program)).create?
   end
 
   def show

app/controllers/calendar_events_controller.rb

@@ -3,8 +3,13 @@ class CalendarEventsController < ApplicationController
   before_action :set_calendar_event, only: [ :show, :edit, :update, :destroy ]
 
   def index
-    @calendar_events = @program.calendar_events.order(:start_time)
-    authorize CalendarEvent.new(program: @program)
+    calendar_event = CalendarEvent.new(program: @program)
+    @calendar_events = @program.calendar_events
+      .includes(calendar_event_faculties: :vip)
+      .order(:start_time)
+    authorize calendar_event
+    @can_create_calendar_events = policy(calendar_event).create?
+    @can_update_calendar_events = policy(calendar_event).update?
   end
 
   def show

app/controllers/questionnaires_controller.rb

@@ -5,6 +5,9 @@ class QuestionnairesController < ApplicationController
   def index
     @questionnaires = @program.questionnaires.includes(:questions)
     authorize Questionnaire.new(program: @program)
+    @can_create_questionnaires = policy(Questionnaire.new(program: @program)).create?
+    @can_view_questionnaires = policy(Questionnaire.new(program: @program)).show?
+    @can_update_questionnaires = policy(Questionnaire.new(program: @program)).update?
   end
 
   def show

app/controllers/students_controller.rb

@@ -8,8 +8,9 @@ class StudentsController < ApplicationController
   def index
     @sort_column = params[:sort].presence_in(SORTABLE_COLUMNS) || "email_address"
     @sort_direction = params[:direction].presence_in(SORT_DIRECTIONS) || "asc"
-    @students = sorted_students
+    set_students_and_enrollment_dates(sorted_students)
     authorize @program, :show?
+    @can_update_program = policy(@program).update?
   end
 
   def export
@@ -74,7 +75,8 @@ def create
 
     # Email address is required
     unless params[:email_address].present?
-      @students = @program.students.order(:email_address)
+      set_students_and_enrollment_dates(@program.students.order(:email_address))
+      @can_update_program = policy(@program).update?
       flash.now[:alert] = "Email address is required."
       render :index, status: :unprocessable_entity
       return
@@ -84,7 +86,8 @@ def create
 
     # Check if UMID is provided (required)
     unless params[:umid].present?
-      @students = @program.students.order(:email_address)
+      set_students_and_enrollment_dates(@program.students.order(:email_address))
+      @can_update_program = policy(@program).update?
       flash.now[:alert] = "UMID is required."
       render :index, status: :unprocessable_entity
       return
@@ -97,7 +100,8 @@ def create
 
     # Check if UMID is already taken by another user
     if formatted_umid.present? && User.where.not(id: user.id).exists?(umid: formatted_umid)
-      @students = @program.students.order(:email_address)
+      set_students_and_enrollment_dates(@program.students.order(:email_address))
+      @can_update_program = policy(@program).update?
       flash.now[:alert] = "This UMID is already in use by another student."
       render :index, status: :unprocessable_entity
       return
@@ -114,7 +118,8 @@ def create
       user.must_change_password = true
 
       unless user.save
-        @students = @program.students.order(:email_address)
+        set_students_and_enrollment_dates(@program.students.order(:email_address))
+        @can_update_program = policy(@program).update?
         flash.now[:alert] = "Error creating user: #{user.errors.full_messages.join(', ')}"
         render :index, status: :unprocessable_entity
         return
@@ -143,7 +148,8 @@ def create
         StudentMailer.welcome(user, @program, is_new_student: was_new_record).deliver_later
         redirect_to department_program_students_path(@department, @program), notice: "Student was successfully added to the program."
       else
-        @students = @program.students.order(:email_address)
+        set_students_and_enrollment_dates(@program.students.order(:email_address))
+        @can_update_program = policy(@program).update?
         flash.now[:alert] = "Error enrolling student: #{student_program.errors.full_messages.join(', ')}"
         render :index, status: :unprocessable_entity
       end
@@ -185,6 +191,16 @@ def sorted_students
     end
   end
 
+  def set_students_and_enrollment_dates(students_scope)
+    @students = students_scope
+    user_id_subquery = @students.except(:order).select(:id)
+
+    @student_program_created_at_by_user_id = StudentProgram
+      .where(program: @program, user_id: user_id_subquery)
+      .pluck(:user_id, :created_at)
+      .to_h
+  end
+
   def set_program
     @program = Program.find(params[:program_id])
     @department = @program.department

app/controllers/vips_controller.rb

@@ -9,7 +9,10 @@ def index
     @vips = apply_sort(base)
     @sort_column = sort_column
     @sort_direction = sort_direction
-    authorize Vip.new(program: @program)
+    vip = Vip.new(program: @program)
+    authorize vip
+    @can_create_vips = policy(vip).create?
+    @can_update_vips = policy(vip).update?
   end
 
   def show

app/policies/appointment_policy.rb

@@ -1,23 +1,57 @@
 class AppointmentPolicy < ApplicationPolicy
   def index?
     return false unless user
-    user.super_admin? || user.department_admin?
+    # Department admins can only access appointment indexes for programs in their own department.
+    return true if user.super_admin?
+
+    record_program = record.respond_to?(:program) ? record.program : nil
+    return false unless record_program
+
+    user.department_admin_for?(record_program.department)
   end
 
   def show?
-    user&.super_admin? || user&.department_admin_for?(record.program.department)
+    return false unless user
+
+    return true if user.super_admin?
+
+    record_program = record.respond_to?(:program) ? record.program : nil
+    return false unless record_program
+
+    user.department_admin_for?(record_program.department)
   end
 
   def create?
-    user&.super_admin? || user&.department_admin_for?(record.program.department)
+    return false unless user
+
+    return true if user.super_admin?
+
+    record_program = record.respond_to?(:program) ? record.program : nil
+    return false unless record_program
+
+    user.department_admin_for?(record_program.department)
   end
 
   def update?
-    user&.super_admin? || user&.department_admin_for?(record.program.department)
+    return false unless user
+
+    return true if user.super_admin?
+
+    record_program = record.respond_to?(:program) ? record.program : nil
+    return false unless record_program
+
+    user.department_admin_for?(record_program.department)
   end
 
   def destroy?
-    user&.super_admin? || user&.department_admin_for?(record.program.department)
+    return false unless user
+
+    return true if user.super_admin?
+
+    record_program = record.respond_to?(:program) ? record.program : nil
+    return false unless record_program
+
+    user.department_admin_for?(record_program.department)
   end
 
   class Scope < Scope

app/policies/calendar_event_policy.rb

@@ -1,6 +1,12 @@
 class CalendarEventPolicy < ApplicationPolicy
   def index?
-    user&.super_admin? || user&.department_admin?
+    return false unless user
+    return true if user.super_admin?
+
+    record_program = record.respond_to?(:program) ? record.program : nil
+    return false unless record_program
+
+    user.department_admin_for?(record_program.department)
   end
 
   def show?

app/policies/questionnaire_policy.rb

@@ -1,6 +1,12 @@
 class QuestionnairePolicy < ApplicationPolicy
   def index?
-    user&.super_admin? || user&.department_admin?
+    return false unless user
+    return true if user.super_admin?
+
+    record_program = record.respond_to?(:program) ? record.program : nil
+    return false unless record_program
+
+    user.department_admin_for?(record_program.department)
   end
 
   def show?

app/views/affiliated_resources/index.html.erb

@@ -4,7 +4,7 @@
       <h1 class="text-3xl font-bold">Affiliated Resources</h1>
       <p class="text-gray-600 mt-1">Department: <%= link_to @department.name, department_path(@department), class: "text-blue-600 hover:text-blue-800" %></p>
     </div>
-    <% if policy(AffiliatedResource.new(department: @department)).create? %>
+    <% if @can_create_affiliated_resources %>
       <%= link_to "New Resource", new_department_affiliated_resource_path(@department), class: "bg-blue-600 text-white btn-responsive rounded hover:bg-blue-700" %>
     <% end %>
   </div>
@@ -19,10 +19,10 @@
               <%= link_to resource.url, safe_url(resource.url), target: "_blank", class: "text-blue-600 hover:text-blue-800 text-sm" %>
             </div>
             <div>
-              <% if policy(resource).update? %>
+              <% if @can_update_affiliated_resources %>
                 <%= link_to "Edit", edit_department_affiliated_resource_path(@department, resource), class: "text-indigo-600 hover:text-indigo-800 mr-4" %>
               <% end %>
-              <% if policy(resource).destroy? %>
+              <% if @can_destroy_affiliated_resources %>
                 <%= link_to "Delete", department_affiliated_resource_path(@department, resource), method: :delete,
                     data: { confirm: "Are you sure?" }, class: "text-red-600 hover:text-red-800" %>
               <% end %>

app/views/appointments/index.html.erb

@@ -39,7 +39,7 @@
           </div>
         </div>
       <% end %>
-      <% if policy(Appointment.new(program: @program)).create? %>
+      <% if @can_create_appointments %>
         <div class="flex space-x-2">
           <%= link_to "Bulk Upload", bulk_upload_department_program_appointments_path(@department, @program), class: "bg-purple-600 text-white btn-responsive rounded hover:bg-purple-700" %>
           <%= link_to "Schedule Builder", schedule_builder_department_program_appointments_path(@department, @program), class: "bg-blue-600 text-white btn-responsive rounded hover:bg-blue-700" %>
@@ -88,7 +88,7 @@
               <th class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Office Number</th>
               <th class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Student</th>
               <th class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Status</th>
-              <% if policy(Appointment.new(program: @program)).show? %>
+              <% if @can_view_appointments %>
                 <th class="px-6 py-3 text-right text-xs font-medium text-gray-500 uppercase tracking-wider">Actions</th>
               <% end %>
             </tr>
@@ -122,7 +122,7 @@
                     <span class="px-2 py-1 text-xs font-semibold rounded-full bg-blue-100 text-blue-800">Booked</span>
                   <% end %>
                 </td>
-                <% if policy(appointment).show? %>
+                <% if @can_view_appointments %>
                   <td class="px-6 py-4 whitespace-nowrap text-right text-sm font-medium">
                     <%= link_to "View", department_program_appointment_path(@department, @program, appointment), class: "text-indigo-600 hover:text-indigo-800" %>
                   </td>

app/views/calendar_events/index.html.erb

@@ -7,7 +7,7 @@
       </p>
     </div>
     <div class="flex space-x-2">
-      <% if policy(CalendarEvent.new(program: @program)).create? %>
+      <% if @can_create_calendar_events %>
         <%= link_to "Bulk Upload", bulk_upload_department_program_calendar_events_path(@program.department, @program), class: "bg-purple-600 text-white btn-responsive rounded hover:bg-purple-700" %>
         <%= link_to "New Event", new_department_program_calendar_event_path(@program.department, @program), class: "bg-green-600 text-white btn-responsive rounded hover:bg-green-700" %>
       <% end %>
@@ -48,7 +48,7 @@
               <th class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">End Time</th>
               <th class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Mandatory</th>
               <th class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Faculty</th>
-              <% if policy(CalendarEvent.new(program: @program)).update? %>
+              <% if @can_update_calendar_events %>
                 <th class="px-6 py-3 text-right text-xs font-medium text-gray-500 uppercase tracking-wider">Actions</th>
               <% end %>
             </tr>
@@ -75,7 +75,7 @@
                     <span class="text-gray-400">None</span>
                   <% end %>
                 </td>
-                <% if policy(event).update? %>
+                <% if @can_update_calendar_events %>
                   <td class="px-6 py-4 whitespace-nowrap text-right text-sm font-medium">
                     <%= link_to "Edit", edit_department_program_calendar_event_path(@program.department, @program, event), class: "text-indigo-600 hover:text-indigo-800 mr-4" %>
                     <%= link_to "Delete", department_program_calendar_event_path(@program.department, @program, event),

app/views/questionnaires/index.html.erb

@@ -6,7 +6,7 @@
         Program: <%= link_to @program.name, department_program_path(@department, @program), class: "text-blue-600 hover:text-blue-800" %>
       </p>
     </div>
-    <% if policy(Questionnaire.new(program: @program)).create? %>
+    <% if @can_create_questionnaires %>
       <%= link_to "New Questionnaire", new_department_program_questionnaire_path(@department, @program), class: "bg-blue-600 text-white btn-responsive rounded hover:bg-blue-700" %>
     <% end %>
   </div>
@@ -22,14 +22,14 @@
                   <%= link_to questionnaire.name, [@department, @program, questionnaire], class: "text-blue-600 hover:text-blue-800" %>
                 </h3>
                 <p class="text-sm text-gray-500">
-                  <%= questionnaire.questions.count %> question(s)
+                  <%= questionnaire.questions.size %> question(s)
                 </p>
               </div>
               <div class="flex gap-2">
-                <% if policy(questionnaire).show? %>
+                <% if @can_view_questionnaires %>
                   <%= link_to "View Responses", responses_department_program_questionnaire_path(@department, @program, questionnaire), class: "bg-green-600 text-white btn-responsive rounded hover:bg-green-700" %>
                 <% end %>
-                <% if policy(questionnaire).update? %>
+                <% if @can_update_questionnaires %>
                   <%= link_to "Edit", edit_department_program_questionnaire_path(@department, @program, questionnaire), class: "bg-indigo-600 text-white btn-responsive rounded hover:bg-indigo-700" %>
                 <% end %>
               </div>

app/views/students/index.html.erb

@@ -8,7 +8,7 @@
     </div>
     <div class="flex gap-2 items-center">
       <%= link_to "Download CSV", export_department_program_students_path(@department, @program, format: :csv, sort: @sort_column || "email_address", direction: @sort_direction || "asc"), class: "bg-gray-600 text-white btn-responsive rounded hover:bg-gray-700" %>
-      <% if policy(@program).update? %>
+      <% if @can_update_program %>
         <%= link_to "Bulk Upload", bulk_upload_department_program_students_path(@department, @program), class: "bg-green-600 text-white btn-responsive rounded hover:bg-green-700" %>
       <% end %>
     </div>
@@ -18,7 +18,7 @@
       page_path: "students/index",
       default_text: "Add students to this program individually by email and UMID, or use Bulk Upload to add many at once. UMID is used as the initial password; students must change it on first login." %>
 
-  <% if policy(@program).update? %>
+  <% if @can_update_program %>
     <div class="bg-white shadow rounded-lg p-6 mb-6">
       <h2 class="text-xl font-semibold mb-4">Add Student</h2>
       <%= form_with url: department_program_students_path(@department, @program), method: :post, class: "space-y-4", data: { controller: "email-autocomplete", "email-autocomplete-search-url-value": search_department_program_students_path(@department, @program) } do |form| %>
@@ -80,7 +80,7 @@
               <th class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider"><%= sort_link("first_name", "First Name", current_sort: @sort_column || "email_address", current_direction: @sort_direction || "asc") %></th>
               <th class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider"><%= sort_link("umid", "UMID", current_sort: @sort_column || "email_address", current_direction: @sort_direction || "asc") %></th>
               <th class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider"><%= sort_link("enrolled", "Enrolled", current_sort: @sort_column || "email_address", current_direction: @sort_direction || "asc") %></th>
-              <% if policy(@program).update? %>
+              <% if @can_update_program %>
                 <th class="px-6 py-3 text-right text-xs font-medium text-gray-500 uppercase tracking-wider">Actions</th>
               <% end %>
             </tr>
@@ -101,9 +101,9 @@
                   <%= student.umid || "-" %>
                 </td>
                 <td class="px-6 py-4 whitespace-nowrap text-sm text-gray-500">
-                  <%= student.student_programs.find_by(program: @program).created_at.strftime("%B %d, %Y") %>
+                  <%= (@student_program_created_at_by_user_id || {})[student.id]&.strftime("%B %d, %Y") || "-" %>
                 </td>
-                <% if policy(@program).update? %>
+                <% if @can_update_program %>
                   <td class="px-6 py-4 whitespace-nowrap text-right text-sm font-medium">
                     <%= link_to "Edit", edit_department_program_student_path(@department, @program, student), class: "text-indigo-600 hover:text-indigo-800 mr-4" %>
                     <%= button_to "Remove", department_program_student_path(@department, @program, student),

app/views/vips/index.html.erb

@@ -5,7 +5,7 @@
       <p class="text-gray-600 mt-1">Program: <%= link_to @program.name, department_program_path(@department, @program), class: "text-blue-600 hover:text-blue-800" %></p>
     </div>
     <div class="flex gap-2">
-      <% if policy(Vip.new(program: @program)).create? %>
+      <% if @can_create_vips %>
         <%= link_to "Bulk Upload", bulk_upload_department_program_vips_path(@department, @program), class: "bg-purple-600 text-white btn-responsive rounded hover:bg-purple-700" %>
         <%= link_to "New VIP", new_department_program_vip_path(@department, @program), class: "bg-blue-600 text-white btn-responsive rounded hover:bg-blue-700" %>
       <% end %>
@@ -61,7 +61,7 @@
                   <% end %>
                 </td>
                 <td class="px-6 py-4 whitespace-nowrap text-right text-sm font-medium">
-                  <% if policy(vip).update? %>
+                  <% if @can_update_vips %>
                     <%= link_to "Edit", edit_department_program_vip_path(@department, @program, vip), class: "text-indigo-600 hover:text-indigo-800" %>
                   <% end %>
                 </td>