Merge pull request #292 from cs169/187186207/iter1-golden-repo-fixes

187186207/iter1 golden repo fixes

.github/workflows/specs.yml

@@ -1,6 +1,6 @@
 name: All Specs
 
-on: [push, pull_request]
+on: [push, pull_request, workflow_dispatch]
 
 jobs:
   specs:

app/controllers/email_templates_controller.rb

@@ -12,10 +12,15 @@ def edit
   end
 
   def update
-    template = EmailTemplate.find(params[:id])
-    template.update(template_params)
-    template.save!
-    redirect_to email_templates_path
+    @email_template = EmailTemplate.find(params[:id])
+    @email_template.update(template_params)
+    if @email_template.save
+      flash[:success] = "Updated #{@email_template.title} template successfully."
+      redirect_to email_templates_path
+    else
+      flash.now[:alert] = "Failed to save #{@email_template.title} template: #{@email_template.errors.full_messages.join(", ")}"
+      render "edit"
+    end
   end
 
   def create
@@ -27,11 +32,11 @@ def create
     end
     load_ordered_email_templates
 
-    if @email_template.save!
+    if @email_template.save
       flash[:success] = "Created #{@email_template.title} successfully."
       redirect_to email_templates_path
     else
-      flash[:alert] = "Failed to submit information :("
+      flash.now[:alert] = "Failed to submit information: #{@email_template.errors.full_messages.join(", ")}"
       render "new"
     end
   end
@@ -49,7 +54,7 @@ def destroy
 
   private
   def template_params
-    params.require(:email_template).permit(:body, :subject, :title)
+    params.require(:email_template).permit(:body, :subject, :title, :to)
   end
 
   def load_ordered_email_templates

app/controllers/pages_controller.rb

@@ -94,7 +94,8 @@ def page_params
   #     teacher_snap: @teacher.snap,
   #     teacher_school_website: @teacher.school.website,
   #     piazza_password: Rails.application.secrets[:piazza_password],
-  #     reason: @reason
+  #     denial_reason: @denial_reason
+  #     request_reason: @request_reason
   #   }.with_indifferent_access
   # end
 

app/controllers/teachers_controller.rb

@@ -130,7 +130,7 @@ def update
   def request_info
     @teacher.info_needed!
     if !params[:skip_email].present?
-      TeacherMailer.request_info_email(@teacher, params[:reason]).deliver_now
+      TeacherMailer.request_info_email(@teacher, params[:request_reason]).deliver_now
     end
     redirect_to root_path
   end
@@ -141,12 +141,10 @@ def validate
     redirect_to root_path
   end
 
-  # TODO: Handle the more info / intermediate status route.
   def deny
     @teacher.denied!
     if !params[:skip_email].present?
-      # TODO: Update dropdown to select the email template.
-      TeacherMailer.deny_email(@teacher, params[:reason]).deliver_now
+      TeacherMailer.deny_email(@teacher, params[:denial_reason]).deliver_now
     end
     redirect_to root_path
   end
@@ -159,8 +157,12 @@ def destroy
 
   def resend_welcome_email
     if @teacher.validated? || @is_admin
+      flash[:success] = "Welcome email resent successfully!"
       TeacherMailer.welcome_email(@teacher).deliver_now
+    else
+      flash[:alert] = "Error resending welcome email. Please ensure that your account has been validated by an administrator."
     end
+    redirect_back(fallback_location: dashboard_path)
   end
 
   def import

app/mailers/teacher_mailer.rb

@@ -11,34 +11,38 @@ class TeacherMailer < ApplicationMailer
   def welcome_email(teacher)
     @teacher = teacher
     set_body
-    mail to: teacher.email_name,
+    set_recipients
+    mail to: @recipients, # ActionMailer accepts comma-separated lists of emails
          cc: CONTACT_EMAIL,
          subject: email_template.subject
   end
 
-  def deny_email(teacher, reason)
+  def deny_email(teacher, denial_reason)
     @teacher = teacher
-    @reason = reason
+    @denial_reason = denial_reason
     set_body
-    mail to: @teacher.email_name,
+    set_recipients
+    mail to: @recipients,
          cc: CONTACT_EMAIL,
          subject: email_template.subject
   end
 
-  def request_info_email(teacher, reason)
+  def request_info_email(teacher, request_reason)
     @teacher = teacher
-    @reason = reason
+    @request_reason = request_reason
     set_body
-    mail to: @teacher.email_name,
+    set_recipients
+    mail to: @recipients,
          cc: CONTACT_EMAIL,
          subject: email_template.subject
   end
 
   def form_submission(teacher)
     @teacher = teacher
     set_body
+    set_recipients
     if @teacher.not_reviewed?
-      mail to: CONTACT_EMAIL,
+      mail to: @recipients,
            subject: email_template.subject
     end
   end
@@ -48,7 +52,9 @@ def liquid_assigns
     base_rules = {
       bjc_password: Rails.application.secrets[:bjc_password],
       piazza_password: Rails.application.secrets[:piazza_password],
-      reason: @reason
+      # TODO: Review if below two are needed, or can they be refractored?
+      denial_reason: @denial_reason,
+      request_reason: @request_reason
     }
     base_rules.merge!(@teacher.email_attributes)
     base_rules.with_indifferent_access
@@ -58,7 +64,13 @@ def email_template
     @email_template ||= EmailTemplate.find_by(title: action_name.titlecase)
   end
 
+  # renders the email body with the {{parameter}} things
   def set_body
     @body = Liquid::Template.parse(email_template.body).render(liquid_assigns).html_safe
   end
+
+  # renders the list of recipients with the {{parameter}} things
+  def set_recipients
+    @recipients = Liquid::Template.parse(email_template.to).render(liquid_assigns).html_safe
+  end
 end

app/models/email_template.rb

@@ -14,14 +14,16 @@
 #  required   :boolean          default(FALSE)
 #  subject    :string
 #  title      :string
+#  to         :string
 #  created_at :datetime         not null
 #  updated_at :datetime         not null
 #
 class EmailTemplate < ApplicationRecord
   validates :title,
             inclusion: TeacherMailer.instance_methods(false).map { |method| method.to_s.titlecase },
             if: -> { self.required? }
-  validates :body, presence: true
+  validates :body, presence: { message: "cannot be blank" }
+  validates :to, presence: { message: "cannot be blank" }
 
   before_destroy :prevent_deleting_required_emails
 

app/views/email_templates/_form.html.erb

@@ -2,6 +2,8 @@
   <div class="form">
     <label for="email_template_title"><h3>Title</h3></label>
     <%= form.text_field :title, class: "form-control", readonly: email_template.required? %>
+    <label for="email_template_to"><h3>To</h3></label>
+    <%= form.text_field :to, class: "form-control" %>
     <label for="email_template_subject"><h3>Subject</h3></label>
     <%= form.text_field :subject, class: "form-control" %>
     <label for="email_template_body"><h3>Body</h3></label>
@@ -12,7 +14,7 @@
     <%= form.submit "Submit", class: "btn btn-primary" %>
   </div>
 <% end %>
-
+<%# This renders the section on "Allowed Tags" %>
 <%= render 'liquid_fields' %>
 
 <script type="text/javascript">

app/views/email_templates/_liquid_fields.erb

@@ -24,8 +24,12 @@
       <td>(The current piazza sign up password)</td>
     </tr>
     <tr>
-      <td><code>{{reason}}</code></td>
+      <td><code>{{denial_reason}}</code></td>
       <td>(Only on denial emails.)</td>
     </tr>
+    <tr>
+      <td><code>{{request_reason}}</code></td>
+      <td>(Only on request emails.)</td>
+    </tr>
   </tbody>
 </table>

app/views/main/_deny_modal.html.erb

@@ -5,23 +5,16 @@
         <h4 class="modal-title"></h4>
         <button type="button" class="close" data-dismiss="modal"><span aria-hidden="true">&times;</span><span class="sr-only">Close</span></button>
       </div>
+      <!-- default action is deny, but it can be dynamically changed. Check the js at the bottom of this file -->
       <%= form_tag deny_teacher_path(0), method: :post, class: "form-horizontal" do %>
         <div class="modal-body">
-            <%= label_tag 'reason', 'Reason', class: 'control-label' %>
-            <%= text_field_tag :reason, params[:reason], class: "form-control" %>
-            <!-- <div class="form-group">
-              <label for="reason-select">Reason for Denial (Select)</label>
-              <select id="reason-select" name="reason-select" class="form-control">
-                <option value="">Select a reason</option>
-                <option value="Incomplete Application">Incomplete Application</option>
-                <option value="Insufficient Experience">Insufficient Experience</option>
-                <option value="Unfavorable References">Unfavorable References</option>
-              </select>
-            </div> -->
-            <div class="form-check">
-              <%= check_box_tag :skip_email, 'skip_email', false, class: 'form-check-input' %>
-              <%= label_tag 'skip_email', 'Skip email notifcation?', class: 'form-check-label' %>
-            </div>
+          <%= hidden_field_tag :action_type, '', class: "form-control action-type" %>
+          <%= label_tag 'action_reason', 'Reason', class: 'control-label' %>
+          <%= text_field_tag :action_reason_placeholder, '', class: "form-control action-reason", id: "action_reason" %>
+          <div class="form-check">
+            <%= check_box_tag :skip_email, 'skip_email', false, class: 'form-check-input' %>
+            <%= label_tag 'skip_email', 'Skip email notifcation?', class: 'form-check-label' %>
+          </div>
         </div>
         <div class="modal-footer">
           <button type="button" class="btn btn-danger" data-dismiss="modal">Cancel</button>
@@ -33,31 +26,34 @@
 </div>
 
 <script type="text/javascript">
-  let modal = $('.js-denialModal');
-  modal.on('shown.bs.modal', function (event) {
-    let button = $(event.relatedTarget);
-    let teacherId = button.data('teacher-id');
-    let teacherName = button.data('teacher-name');
-    let newAction = null;
-    let newTitle = null;
-    switch(button.data('modal-type')) {
-      case "deny":
-        newAction = `/teachers/${teacherId}/deny`;
-        newTitle = 'Deny ' + teacherName;
-        break;
-      case "request_info":
-        newAction = `/teachers/${teacherId}/request_info`;
-        newTitle = 'Request Info from ' + teacherName;
-        break;
-      default:
-        newAction = `/teachers/${teacherId}/deny`;
-        newTitle = 'Deny ' + teacherName;
-    }
-    modal.find('.form-horizontal').attr('action', newAction);
-    modal.find('.modal-title').text(newTitle);
-  });
-  modal.on('hidden.bs.modal', function () {
-    $('input[name="reason"]').val('');
-    modal.find('.modal-title').text('');
-  });
+    let modal = $('.js-denialModal');
+    modal.on('shown.bs.modal', function (event) {
+        let button = $(event.relatedTarget);
+        let teacherId = button.data('teacher-id');
+        let teacherName = button.data('teacher-name');
+        let actionType = button.data('modal-type');
+        let newAction, newTitle, inputName;
+
+        switch(actionType) {
+            case "request_info":
+                newAction = `/teachers/${teacherId}/request_info`;
+                newTitle = 'Request Info from ' + teacherName;
+                inputName = 'request_reason';
+                break;
+            case "deny":
+            default:
+                newAction = `/teachers/${teacherId}/deny`;
+                newTitle = 'Deny ' + teacherName;
+                inputName = 'denial_reason';
+        }
+
+        modal.find('.form-horizontal').attr('action', newAction);
+        modal.find('.modal-title').text(newTitle);
+        modal.find('#action_reason').attr('name', inputName);
+    });
+
+    modal.on('hidden.bs.modal', function () {
+        modal.find('#action_reason').val('').attr('name', 'action_reason_placeholder');
+        modal.find('.modal-title').text('');
+    });
 </script>