CSC/ECE 517 Fall 2021 - E2142. Improve e-mail notifications: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
Line 216: Line 216:


==Issue3 (111) - Adding relevant links to reminder emails==
==Issue3 (111) - Adding relevant links to reminder emails==
'''Pre conditions'''
* There exists an assignment and a participant associated with that assignment
* The due date for the assignment is approaching
'''Expected Behavior'''
Exact issue stating the expected behavior can be found at https://github.com/expertiza/expertiza/issues/111 -
"E-mails about reviews should direct the user to the page where the review is found. Deadline reminders should include a link on where to go to perform the needed function."
So, when assignment reminder emails are sent to students, they should include a link to the relevant assignment.
'''Actual Behavior'''
This behavior was actually already implemented through one simple line in the app/models/user.rb file.<br><br>'''user.rb'''
<pre>
after_create :email_welcome
</pre>
This line calls the 'email_welcome' function every time a user is created, which sends a welcome email to the newly created user.<br><br>'''user.rb'''
<pre>
# Function which has a MailerHelper which sends the mail welcome email to the user after signing up
def email_welcome
  #this will send an account creation notification to user via email.
  MailerHelper.send_mail_to_user(self, "Your Expertiza account and password has been created", "user_welcome", password).deliver_now
end
</pre>
This functionality is also completely tested in the spec/models/assignment_participant_spec.rb file show below. This test shows that when the import function is called an email is delivered.<br><br>'''assignment_participant_spec.rb'''
<pre>
context 'when new user needs to be created' do
  let(:row) do
    {name: 'no one', fullname: 'no one', email: 'name@email.com', role:'user_role_name', parent: 'user_parent_name'}
  end
  let(:attributes) do
    {role_id: 1, name: 'no one', fullname: 'no one', email: 'name@email.com', email_on_submission: 'name@email.com',
      email_on_review: 'name@email.com', email_on_review_of_review: 'name@email.com'}
  end
  let(:test_user) do
    {name: 'abc', email: 'abcbbc@gmail.com'}
  end
  it 'create the user and number of mails sent should be 1' do
    ActionMailer::Base.deliveries.clear
    allow(ImportFileHelper).to receive(:define_attributes).with(row).and_return(attributes)
    allow(ImportFileHelper).to receive(:create_new_user) do
      test_user = User.new(name: 'abc', fullname: 'abc bbc', email: 'abcbbc@gmail.com')
      test_user.id = 123
      test_user.save!
      test_user
    end
    #allow(ImportFileHelper).to receive(:create_new_user).with(attributes, {}).and_return()
    allow(Assignment).to receive(:find).with(1).and_return(assignment)
    allow(User).to receive(:exists?).with(name: 'no one').and_return(false)
    allow(participant).to receive(:set_handle).and_return('handle')
    allow(AssignmentParticipant).to receive(:exists?).and_return(false)
    allow(AssignmentParticipant).to receive(:create).and_return(participant)
    allow(AssignmentParticipant).to receive(:set_handle)
    expect{(AssignmentParticipant.import(row, nil, {}, 1))}.to change { ActionMailer::Base.deliveries.count }.by(1)
  end
end
</pre>


==Testing==
==Testing==

Revision as of 02:54, 21 October 2021

E2142. Improve e-mail notifications

Brief Introduction

Problem Statement

The following tasks were accomplished in this project:

  • Issue1: Send new account welcome email to user, when imported from CSV through assignment page.
  • Issue2: Don't send email to reviewers for a new submission after review deadline has passed.
  • Issue3: Adding relevant links to reminder emails.

Issue1 (717) - Send new account welcome email to user, when imported from CSV through assignment page

Pre conditions

  • There exists an assignment and an instructor
  • The instructor navigates to the 'Manage Assignments' page
  • The instructor clicks 'Add Participants' for the existing assignment
  • The instructor clicks 'Import assignment participants'
  • The instructor imports a CSV file containing a user who does not exist yet

Expected Behavior

Exact issue stating the expected behavior can be found at https://github.com/expertiza/expertiza/issues/717 - "When students' accounts are created by importing a CSV on the Users page, they receive e-mails with their user-ID and password. But if an account is created by adding them as participants to an assignment when they don't already have an account, e-mail is not sent. Students should receive e-mails upon account creation, regardless of how their account is created."

So, when students accounts are created in any way, they should receive a welcome email. This includes adding them through importing a CSV.

Actual Behavior

This behavior was actually already implemented through one simple line in the app/models/user.rb file.

user.rb

after_create :email_welcome

This line calls the 'email_welcome' function every time a user is created, which sends a welcome email to the newly created user.

user.rb

# Function which has a MailerHelper which sends the mail welcome email to the user after signing up
def email_welcome
  #this will send an account creation notification to user via email.
  MailerHelper.send_mail_to_user(self, "Your Expertiza account and password has been created", "user_welcome", password).deliver_now
end

This functionality is also completely tested in the spec/models/assignment_participant_spec.rb file show below. This test shows that when the import function is called an email is delivered.

assignment_participant_spec.rb

context 'when new user needs to be created' do
  let(:row) do
    {name: 'no one', fullname: 'no one', email: 'name@email.com', role:'user_role_name', parent: 'user_parent_name'}
  end
  let(:attributes) do
    {role_id: 1, name: 'no one', fullname: 'no one', email: 'name@email.com', email_on_submission: 'name@email.com',
      email_on_review: 'name@email.com', email_on_review_of_review: 'name@email.com'}
  end
  let(:test_user) do
    {name: 'abc', email: 'abcbbc@gmail.com'}
  end
  it 'create the user and number of mails sent should be 1' do
    ActionMailer::Base.deliveries.clear
    allow(ImportFileHelper).to receive(:define_attributes).with(row).and_return(attributes)
    allow(ImportFileHelper).to receive(:create_new_user) do
      test_user = User.new(name: 'abc', fullname: 'abc bbc', email: 'abcbbc@gmail.com')
      test_user.id = 123
      test_user.save!
      test_user
    end
    #allow(ImportFileHelper).to receive(:create_new_user).with(attributes, {}).and_return()
    allow(Assignment).to receive(:find).with(1).and_return(assignment)
    allow(User).to receive(:exists?).with(name: 'no one').and_return(false)
    allow(participant).to receive(:set_handle).and_return('handle')
    allow(AssignmentParticipant).to receive(:exists?).and_return(false)
    allow(AssignmentParticipant).to receive(:create).and_return(participant)
    allow(AssignmentParticipant).to receive(:set_handle)
    expect{(AssignmentParticipant.import(row, nil, {}, 1))}.to change { ActionMailer::Base.deliveries.count }.by(1)
  end
end

Issue2 (345) - Don't send email to reviewers for a new submission after review deadline has passed

Pre conditions

  • There is an assignment, a student as a participant, a participant as a reviewer who is reviewing the this particular assignment
  • a review is made for a submission by the student
  • review deadline has passed
  • the student is allowed to submit after the review deadline has passed

Expected Behavior

Exact issue stating the expected behavior can be found at https://github.com/expertiza/expertiza/issues/345 - "Evidently, if a submission is revised after review, the system e-mails the reviewer saying to revise the review. This is just fine ... except if the last round of review has been completed. The message telling reviewers to revise their reviews should not be sent after the last review deadline has passed."

So basically, When the student makes a new submission after the LAST review deadline has passed. The reviewer should not receive an email, asking the reviewer to update the review according to the new submission.

Actual Behavior

According to the current Code scenario on the beta branch if the deadline has passed, a student(participant) cannot make new submissions. So, automatically if the participant doe not make a new submission then the reviewer wont receive an email. So this issue does not really exist by the virtue of the state of the system. the stack trace for the check if the submission is allowed or not is as follows - assignment.rb

def submission_allowed(topic_id = nil)
    check_condition('submission_allowed_id', topic_id)
end

assignment.rb

  def check_condition(column, topic_id = nil)
    next_due_date = DueDate.get_next_due_date(self.id, topic_id)
    return false if next_due_date.nil?
    right_id = next_due_date.send column
    right = DeadlineRight.find(right_id)
    right && (right.name == 'OK' || right.name == 'Late')
  end

due_date.rb

    def self.get_next_due_date(assignment_id, topic_id = nil)
    if Assignment.find(assignment_id).staggered_deadline?
      next_due_date = TopicDueDate.find_by(['parent_id = ? and due_at >= ?', topic_id, Time.zone.now])
      # if certion TopicDueDate is not exist, we should query next corresponding AssignmentDueDate.
      # eg. Time.now is 08/28/2016
      # One topic uses following deadlines:
      # TopicDueDate      08/01/2016
      # TopicDueDate      08/02/2016
      # TopicDueDate      08/03/2016
      # AssignmentDueDate 09/04/2016
      # In this case, we cannot find due_at later than Time.now in TopicDueDate.
      # So we should find next corrsponding AssignmentDueDate, starting with the 4th one, not the 1st one!
      if next_due_date.nil?
        topic_due_date_size = TopicDueDate.where(parent_id: topic_id).size
        following_assignment_due_dates = AssignmentDueDate.where(parent_id: assignment_id)[topic_due_date_size..-1]
        unless following_assignment_due_dates.nil?
          following_assignment_due_dates.each do |assignment_due_date|
            if assignment_due_date.due_at >= Time.zone.now
              next_due_date = assignment_due_date
              break
            end
          end
        end
      end
    else
      next_due_date = AssignmentDueDate.find_by(['parent_id = ? && due_at >= ?', assignment_id, Time.zone.now])
    end
    next_due_date
  end

So this is issue is resolved by the state of the system itself.
BUT Surprisingly on beta branch Reviewers aren't getting any emails at all on beta branch, in submitted_content_controller, in function "submit_file", this lines is supposed to send email to the reviewers to notify them about the new submission

 participant.assignment.email(participant.id) rescue nil 

BUT here email function is undefined. we checked for email function in assignment class, there was none. The error "undefined function on assignment" is always rescued and hence no errors were raised previously and hence it wasn't highlighted before.
So, in beta branch reviewers are not being notified anytime a new submission is made to an assignment that they have previously reviewed. So we fixed that Taking inspiration from the Master branch the previous team's work we added following code to send an email notifying the reviewer of an new submission.

in submitted_content_controller.rb, in the functions, submit_file, submit_hyperlink

Team.mail_assigned_reviewers(@participant)

participant.rb

def mail_assigned_reviewers()
    team = self.team
    maps = ResponseMap.where(reviewed_object_id: self.assignment.id, reviewee_id: team.id, type: 'ReviewResponseMap')
    unless maps.nil?
      maps.each do |map|
        reviewer = User.find(Participant.find(map.reviewer_id).user_id)
        Mailer.sync_message(
            {
                :to => reviewer.email,
                subject:  "Assignment '#{self.assignment.name}': A submission has been updated since you last reviewed it",
                cc: User.find_by(self.assignment.instructor_id).email,
                :body => {
                    :obj_name => self.assignment.name,
                    :link => "https://expertiza.ncsu.edu/response/new?id=#{map.id}",
                    :type => 'submission',
                    :first_name => ApplicationHelper::get_user_first_name(User.find(Participant.find(map.reviewer_id).user_id)),
                    :partial_name => 'updated_submission_since_review'
                }
            }
        ).deliver_now
      end
    end
  end

app/views/mailer/partials/_updated_submission_since_review_plain.html.erb

Hi <%= @first_name %>,

A <%=@type%> you were reviewing for Assignment '<%= @obj_name %>' has been updated since you last review.
Hence, to update your review please click on the following link.
<% if @link != nil %><%= @link %><% end %>

app/views/mailer/partials/_updated_submission_since_review_html.html.erb

Hi <%= @first_name %>,</br>
<p>
  A <%=@type%> you were reviewing for Assignment '<%= @obj_name %>' has been updated since you last review.<br>
  Hence, to update your review please click on the following link.<br>
  <% if @link != nil %><a href="<%= @link %>"><%= @link %></a><% end %>
</p>  

added a line in app/mailers/mailer.rb

 @link = defn[:body][:link]

Issue3 (111) - Adding relevant links to reminder emails

Pre conditions

  • There exists an assignment and a participant associated with that assignment
  • The due date for the assignment is approaching

Expected Behavior

Exact issue stating the expected behavior can be found at https://github.com/expertiza/expertiza/issues/111 - "E-mails about reviews should direct the user to the page where the review is found. Deadline reminders should include a link on where to go to perform the needed function."

So, when assignment reminder emails are sent to students, they should include a link to the relevant assignment.

Actual Behavior

This behavior was actually already implemented through one simple line in the app/models/user.rb file.

user.rb

after_create :email_welcome

This line calls the 'email_welcome' function every time a user is created, which sends a welcome email to the newly created user.

user.rb

# Function which has a MailerHelper which sends the mail welcome email to the user after signing up
def email_welcome
  #this will send an account creation notification to user via email.
  MailerHelper.send_mail_to_user(self, "Your Expertiza account and password has been created", "user_welcome", password).deliver_now
end

This functionality is also completely tested in the spec/models/assignment_participant_spec.rb file show below. This test shows that when the import function is called an email is delivered.

assignment_participant_spec.rb

context 'when new user needs to be created' do
  let(:row) do
    {name: 'no one', fullname: 'no one', email: 'name@email.com', role:'user_role_name', parent: 'user_parent_name'}
  end
  let(:attributes) do
    {role_id: 1, name: 'no one', fullname: 'no one', email: 'name@email.com', email_on_submission: 'name@email.com',
      email_on_review: 'name@email.com', email_on_review_of_review: 'name@email.com'}
  end
  let(:test_user) do
    {name: 'abc', email: 'abcbbc@gmail.com'}
  end
  it 'create the user and number of mails sent should be 1' do
    ActionMailer::Base.deliveries.clear
    allow(ImportFileHelper).to receive(:define_attributes).with(row).and_return(attributes)
    allow(ImportFileHelper).to receive(:create_new_user) do
      test_user = User.new(name: 'abc', fullname: 'abc bbc', email: 'abcbbc@gmail.com')
      test_user.id = 123
      test_user.save!
      test_user
    end
    #allow(ImportFileHelper).to receive(:create_new_user).with(attributes, {}).and_return()
    allow(Assignment).to receive(:find).with(1).and_return(assignment)
    allow(User).to receive(:exists?).with(name: 'no one').and_return(false)
    allow(participant).to receive(:set_handle).and_return('handle')
    allow(AssignmentParticipant).to receive(:exists?).and_return(false)
    allow(AssignmentParticipant).to receive(:create).and_return(participant)
    allow(AssignmentParticipant).to receive(:set_handle)
    expect{(AssignmentParticipant.import(row, nil, {}, 1))}.to change { ActionMailer::Base.deliveries.count }.by(1)
  end
end

Testing

Testing Reviewe's Email manually

Step 1: Create new assignment [Manage --> Assignment --> + Button (to create new assignment)]

Step 2: Fill the details for the assignments.

Step 3: Navigate to due dates.

Step 4: Change the number of review rounds to 2.

Step 5: Select "Yes" in the dropdown for review allowed during submission and select "Yes" for submission during the review.

Step 6: Add two users to the assignment(author and reviewer).

Step 7: Log in with some user credentials (author credential).

Step 8: Make a new submission to this assignment.

Step 9: Log in with another user (reviewer).

Step 10: Submit a review of the assignment submission.

Step 11: Login as an author again.

Step 12: Edit the submission.

Step 13: After this check the mailbox of the reviewer [development mail for development].

Step 14: Reviewer should get the mail to re-review the work.

Step 15: Change the due date to some date and time which has passed.

Step 16: Now the author should not be able to make a new submission.

Testing Reviewe's Email via rspec