Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CS169 Iteration 1 Pull Request #291

Closed
wants to merge 46 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
d3b2f5e
Allow manually triggering specs
cycomachead Feb 12, 2024
c343618
change reason to denial reason
ArushC Feb 15, 2024
754d4c7
updated relevant deny cucumber test to use denial_reason instead of r…
ArushC Feb 15, 2024
092cce2
rubocop fix
ArushC Feb 15, 2024
2bab1d0
migration adds 'to' feld to email template database
ArushC Feb 18, 2024
b1173d2
added to field to view and added a file I forgot to add from prev mig…
ArushC Feb 18, 2024
870491a
update backend to send to all emails specified in To field in email t…
ArushC Feb 20, 2024
c231e07
fixed rspec test failures and rubocop by updating seed data which get…
ArushC Feb 20, 2024
be305ef
added test with barney the purple dinosaur
ArushC Feb 20, 2024
2088b07
rubocop fix
ArushC Feb 20, 2024
4ce4a2d
Add request reason email template
perryzjc Feb 20, 2024
15da9b5
Fix reason naming
perryzjc Feb 20, 2024
5da7ca6
Add request reason to liquid template engine
perryzjc Feb 20, 2024
2ee19ed
Merge pull request #29 from cs169/187044290-reason-to-denial-reason
ArushC Feb 20, 2024
725ae84
fix request info email template
realmichaeltao Feb 21, 2024
f99087d
Add rspec test for request reason email
perryzjc Feb 21, 2024
7a8e473
change prepopulated TO field for form submission email
ArushC Feb 21, 2024
1b42b58
added validations and more descriptive, useful error messages to emai…
ArushC Feb 21, 2024
96267f9
rubocop fixes
ArushC Feb 21, 2024
96c18ec
added cucumber tests and added untracked migration file
ArushC Feb 23, 2024
177c1c6
add flash message to resend welcome email
JacksonXu33 Feb 23, 2024
4862e0b
Merge pull request #26 from cs169/cycomachead-patch-1
cycomachead Feb 23, 2024
3aefd1e
add flash message after welcome resend
JacksonXu33 Feb 25, 2024
9248494
Added button UI, modified UI logic, added comments for backend
razztech Feb 26, 2024
928c57b
Merge pull request #31 from cs169/feature/187044317-add-email-resend-…
ArushC Feb 26, 2024
d4db5b3
Linked UI to email template
razztech Feb 26, 2024
21718e5
Update UI logic for request info and pass the test
perryzjc Feb 27, 2024
325fb70
Merge pull request #32 from cs169/187044299/feature/request_reason
ArushC Feb 27, 2024
ac57d48
rebase against main and fix merge conflict in seed data
ArushC Feb 27, 2024
d9d096d
Merge pull request #30 from cs169/187044314-add-to-field
perryzjc Feb 27, 2024
c64216f
edit redirect to use current path
JacksonXu33 Feb 27, 2024
d1a4b1d
Merge pull request #33 from cs169/feature/187044317-edit-resend-welco…
ArushC Feb 28, 2024
9eeb0dc
Fix email template reseed problem
perryzjc Feb 28, 2024
82c5475
Add rspec test for email reseed behavior
perryzjc Feb 28, 2024
d00c889
Refract rspec test of email reseed to be more modular
perryzjc Feb 28, 2024
9fb64e9
Add RSpec db check to ensure tests use bjc_teachers_test for safety
perryzjc Feb 28, 2024
2f30dc1
Revert "Add RSpec db check to ensure tests use bjc_teachers_test for …
perryzjc Feb 28, 2024
7464961
Adjust Rspec test to avoid purge old email
perryzjc Feb 28, 2024
755a96b
Update email reseed implementation to avoid purge old email templates
perryzjc Feb 28, 2024
771324f
Merge pull request #34 from cs169/187113121/bugfix/email_reseed
ArushC Feb 29, 2024
dad60c5
Refract Rspec test for email reseed
perryzjc Mar 2, 2024
54bd3fd
finish refract email reseed test cases
realmichaeltao Mar 2, 2024
5bffa76
move email reseed test cases to models folder
realmichaeltao Mar 2, 2024
ba4a96b
Fix rubocop check
perryzjc Mar 2, 2024
0fe0be5
Change the way of hashing for updating email template seed
perryzjc Mar 2, 2024
df52d67
Merge pull request #35 from cs169/187156425/bugfix/refract-test-email…
ArushC Mar 2, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/specs.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: All Specs

on: [push, pull_request]
on: [push, pull_request, workflow_dispatch]

jobs:
specs:
Expand Down
15 changes: 10 additions & 5 deletions app/controllers/email_templates_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@ def edit
def update
template = EmailTemplate.find(params[:id])
template.update(template_params)
template.save!
redirect_to email_templates_path
if template.save
flash[:success] = "Updated #{template.title} template successfully."
redirect_to email_templates_path
else
flash[:alert] = "Failed to save #{template.title} template: " + template.errors.full_messages.join(", ")
redirect_to edit_email_template_path(params[:id])
end
end

def create
Expand All @@ -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[:alert] = "Failed to submit information: " + @email_template.errors.full_messages.join(", ")
render "new"
end
end
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 7 additions & 2 deletions app/controllers/teachers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -146,7 +146,8 @@ 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
puts params[:denial_reason]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please clean this up. :)

TeacherMailer.deny_email(@teacher, params[:denial_reason]).deliver_now
end
redirect_to root_path
end
Expand All @@ -159,8 +160,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
Expand Down
30 changes: 21 additions & 9 deletions app/mailers/teacher_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
4 changes: 3 additions & 1 deletion app/models/email_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion app/views/email_templates/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
<%# This part of the page is the email template form %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would delete a comment like this.

<%= form_with(model: email_template, local: true) do |form| %>
<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>
Expand All @@ -12,7 +15,7 @@
<%= form.submit "Submit", class: "btn btn-primary" %>
</div>
<% end %>

<%# This renders the section on "Allowed Tags" %>
<%= render 'liquid_fields' %>

<script type="text/javascript">
Expand Down
6 changes: 5 additions & 1 deletion app/views/email_templates/_liquid_fields.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
98 changes: 56 additions & 42 deletions app/views/main/_deny_modal.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,25 @@
<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-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>
</div>
<div class="modal-footer">
<button type="button" class="btn btn-danger" data-dismiss="modal">Cancel</button>
Expand All @@ -33,31 +35,43 @@
</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); // Button that triggered the modal
let teacherId = button.data('teacher-id');
let teacherName = button.data('teacher-name');
let actionType = button.data('modal-type'); // Action type, e.g., "deny", "request_info"
let newAction, newTitle, inputName;

// Determine the action and title based on the button clicked
switch(actionType) {
case "deny":
newAction = `/teachers/${teacherId}/deny`;
newTitle = 'Deny ' + teacherName;
inputName = 'denial_reason'; // The dynamic part for the input's name attribute
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete the comment at the end of the line.

break;
case "request_info":
newAction = `/teachers/${teacherId}/request_info`;
newTitle = 'Request Info from ' + teacherName;
inputName = 'request_reason'; // Change accordingly
break;
default:
newAction = `/teachers/${teacherId}/deny`;
newTitle = 'Deny ' + teacherName;
inputName = 'denial_reason'; // Fallback option
}

// Update the form's action and the modal's title
modal.find('.form-horizontal').attr('action', newAction);
modal.find('.modal-title').text(newTitle);

// Dynamically change the name attribute of the input field
modal.find('#action_reason').attr('name', inputName);
});

// Reset the input field when the modal is closed
modal.on('hidden.bs.modal', function () {
modal.find('#action_reason').val('').attr('name', 'action_reason_placeholder');
modal.find('.modal-title').text('');
});
</script>
Loading
Loading