Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-12-06T13:27:53Z

Vrcholay: /* Testing from UI */

__TOC__

== Topic Overview ==
===Background===
Expertiza uses Sidekiq gem for asynchronous processing of email tasks. It has a queue system to hold and then process jobs. Sidekiq’s queue replaces DelayedMailer’s queue. The previous team that worked on this also created a method perform() to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

===Prior Work===
The prior work which is the basis for this topic is linked here [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task E2144]. This work was done in Fall of 2021, and achieved much of the desired functionality for this task. This prior work will be referenced several times within this documentation, as it has a more comprehensive explanation of the underlying design principles behind the views, controller, model, algorithm, and RSpec tests implemented. These details will only be briefly summarized here.

This prior implementation was rejected due to lack of method comments and a failing test, and so correcting the test and adding method comments--and explaining how those corrections were done--will be the major focus of this project documentation.

The work in [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task E2253] was rejected because it included code changes in files that were deemed by Dr. Gehringer to be unnecessary and unassociated with the core functionality of this feature. Additionally, some of the comments are shallow and not insightful enough according to Dr. Gehringer.

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]

==Problem Statement==
<li> This project used the Sidekiq gem to asynchronously handle email tasks, which is sort of similar to E2135. Sidekiq was partially merged before, but this project changes the queue setup slightly. The refactor of methods in assignment_form.rb is significant, and several methods were moved to due_date.rb. It also adds reminder functionality to mail_worker, which also conflicts with E2135. Significant spec additions have been made from what exists in beta. Overall, code looks appropriate and well-written. Sidekiq needs to be started in the background while Expertiza is running, but since Sidekiq was already partially merged, I think this should be expected. ''After viewing the peer reviews and newest Travis run, this appears almost ready to merge. Peer reviewers believe it is a quality project, but note that several methods lack method comments. The travis build has a single small error in models (assignment_form_spec.rb)''.
 

Files involved (some changed should not have been changed):
<li> app/controllers/assignments_controller.rb
<li> app/mailers/mail_worker.rb
<li> app/models/assignment_form.rb
<li> config/sidekiq.yml
<li> spec/models/assignment_form_spec.rb
<li> spec/sidekiq_mail_worker_spec.rb
<li> spec/spec_helper.rb

 
The code for the prior PR is linked here [https://github.com/expertiza/expertiza/pull/2089 is linked here].

==Explanation of Feature==
Mailer/mailing is referring to the sending of emails to email address associated with expertiza account. There are two types of mailing: '''synchronous and asynchronous'''. Synchronous mailing occurs after an action is taken e.g., an email that notifies when a change is made to a document. Asynchronous mailing occurs during a set time e.g., a notification email to warn students when a deadline is approaching.

=== Summary of Work Needed ===

After discussing with our project mentor and Dr. Gehringer, we discovered that what needed to be done is refining the reason why the previous PR was not deemed production-ready and resolve those issues in order to merge to main:
<li> Add insightful method comments
<li> Limit changes to delayed_mailer.rb and scheduled_task.rb (this means removing any changes to assignments_controller.rb and assignment_form.rb
<li> Increase test coverage for above files
<li> Fix failing test in assignment_form_spec.rb

 
In terms of functionality, Dr. Gehringer mentioned that asynchronous mailing is having issues e.g., not sending out an assignment deadline notification a day before it is due. If there is time, the team may debug and work to fix this. If the team is unable to resolve, this may be passed on to future teams. Dr. Gehringer did not specify if it is within scope of this project.

== Problems and planned changes ==

We are planning to accomplish the following tasks in this project:

=== Problem 1: Add insightful comments to the methods in mailer.rb===
* '''Proposed Solution''': Add insightful comments to each method.
* '''Solution Implemented''': Added meaningful comments to every major line in the mailer.rb file to reflect the functionality and purpose of each method.

=== Problem 2: Fix Bugs in sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Fix the failing existing tests.
* '''Solution Implemented''': Changed the email from expertiza.development@gmail.com to expertiza.debugging@gmail.com. Generated an email with the appropriate body, instead of a nil object.

<pre>
it "should have sent welcome email after user was created" do
Sidekiq::Testing.inline!
email = Mailer.sync_message(
to: 'tluo@ncsu.edu',
subject: 'Your Expertiza account and password has been created',
body: {
obj_name: 'assignment',
type: 'submission',
location: '1',
first_name: 'User',
partial_name: 'update'
}
).deliver_now
email = Mailer.deliveries.first
expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
expect(email.to[0]).to eq("expertiza.debugging@gmail.com")
expect(email.subject).to eq("Your Expertiza account and password has been created")
end
</pre>

<pre>
it 'should send reminder email to required email address with proper content' do
Sidekiq::Testing.inline!
Mailer.deliveries.clear
worker = MailWorker.new
worker.perform("1", "metareview", "2018-12-31 00:00:01")
expect(Mailer.deliveries.size).to eq(1)
email = Mailer.deliveries.first
expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
expect(email.bcc[0]).to eq("psingh22@ncsu.edu")
expect(email.subject).to eq("Message regarding teammate review for assignment no assignment")
expect(email.body).to eq("This is a reminder to complete teammate review for assignment no assignment.\nPlease follow the link: http://expertiza.ncsu.edu/student_task/view?id=1\nDeadline is 2018-12-31 00:00:01. If you have already done the teammate review, then please ignore this mail.")
end
</pre>

=== Problem 3: Increase the test coverage for sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Add more tests to increase test coverage.
* '''Solution Implemented''': Added the following tests:
1. Test to check an email is not sent when there is deadline with outstanding reviews.

<pre>
it "should not return email if deadline is drop_outstanding_reviews" do
Sidekiq::Testing.inline!
Mailer.deliveries.clear
worker = MailWorker.new
worker.perform("1", "drop_outstanding_reviews", "2018-12-31 00:00:01")
expect(Mailer.deliveries.size).to eq(0)
end
</pre>
2. Test to check if the size of the queue is increased when Mailworker is used.
<pre>
it "should increase the size of queue by 1 when MailWorker is used" do
Sidekiq::Testing.inline!
Mailer.deliveries.clear
worker = MailWorker.new
worker.perform(1, 'review', '2022-12-05 00:00:01')
expect(Mailer.deliveries.size).to eq(1)
end
</pre>

==Test Plan==

===Automated Testing using RSPEC===
To run the tests, run the following commands from the expertiza directory
<pre>
rspec ./spec/workers/sidekiq_mail_worker_spec.rb
</pre>
[[File:E2273_mailer_spec.png|800px]]

The current version of expertiza included tests for the assignments_form and the mail_worker, but they were far from extensive. Thus we added a few tests of our own:

<pre>
it "should have sent welcome email after user was created" do
Sidekiq::Testing.inline!
email = Mailer.sync_message(
to: 'tluo@ncsu.edu',
subject: 'Your Expertiza account and password has been created',
body: {
obj_name: 'assignment',
type: 'submission',
location: '1',
first_name: 'User',
partial_name: 'update'
}
).deliver_now
email = Mailer.deliveries.first
expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
expect(email.to[0]).to eq("expertiza.debugging@gmail.com")
expect(email.subject).to eq("Your Expertiza account and password has been created")
end
</pre>

===Testing from UI===
No changes were made to the functionality of the project, however you can access it through the setup below.

*Visit the VCL hosted site: http://152.7.99.69:8080/
**Login: Instructor6
**Password: Password

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, we needed to run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

NB: To create a generic sidekiq worker to test with, you could use
<pre>rails g sidekiq:worker TestWorker</pre>
to create a shell worker class.

===== Sidekiq UI =====
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Ensure the Late Policy is set to <code>--None--</code> under the Due Dates tab. If there is a late policy set, per pre-existing code, the enqueued SimicheckWorker job will be deleted and will not appear in the Sidekiq UI (unless it is dequeued quickly, before the code can delete the jobs in the queue).
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

[[File:simicheck-job-enqueued.png|950px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
# Log into Expertiza as an instructor.
# Navigate to '''Manage'''>'''Assignments''' in the navigation.
# Click the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings.
# Go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Select a late policy next to '''Apply penalty policy:'''. If a late policy has not been created, press '''New Late Policy''' and follow the steps for creating a late policy.
# Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page.
# Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. The assignment due date must be changed to a time at least an hour ahead of the current time for the job to be queued.
# To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq.
# Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, you should see something like the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

'''Functional Tests'''

Functional testing is done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected. A previously implemented test for Simicheck was commented out as advised by Dr. Gehringer, because it has a helper function dependency called PlagiarismCheckerHelper. According to Dr. Gehringer, in PlagiarismCheckerHelper, the request "variable is supposed to contain an http request for running the plagiarism checker. The checker has not been used for several years. It will not hurt anything important if it does not work now."

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/MichaelDacanay/expertiza-mailer-refactor/tree/E2273 GitHub Project Repository Fork]
#[https://github.com/expertiza/expertiza/pull/2478 E2273 Pull Request]
#[http://expertiza.ncsu.edu/ The live Expertiza website]

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-12-05T13:39:06Z

Vrcholay: /* Problem 3: Increase the test coverage for sidekiq_mail_worker_spec.rb */

__TOC__

== Topic Overview ==
===Background===
Expertiza uses Sidekiq gem for asynchronous processing of email tasks. It has a queue system to hold and then process jobs. Sidekiq’s queue replaces DelayedMailer’s queue. The previous team that worked on this also created a method perform() to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

===Prior Work===
The prior work which is the basis for this topic is linked here [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task E2144]. This work was done in Fall of 2021, and achieved much of the desired functionality for this task. This prior work will be referenced several times within this documentation, as it has a more comprehensive explanation of the underlying design principles behind the views, controller, model, algorithm, and RSpec tests implemented. These details will only be briefly summarized here.

This prior implementation was rejected due to lack of method comments and a failing test, and so correcting the test and adding method comments--and explaining how those corrections were done--will be the major focus of this project documentation.

The work in [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task E2253] was rejected because it included code changes in files that were deemed by Dr. Gehringer to be unnecessary and unassociated with the core functionality of this feature. Additionally, some of the comments are shallow and not insightful enough according to Dr. Gehringer.

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]

==Problem Statement==
<li> This project used the Sidekiq gem to asynchronously handle email tasks, which is sort of similar to E2135. Sidekiq was partially merged before, but this project changes the queue setup slightly. The refactor of methods in assignment_form.rb is significant, and several methods were moved to due_date.rb. It also adds reminder functionality to mail_worker, which also conflicts with E2135. Significant spec additions have been made from what exists in beta. Overall, code looks appropriate and well-written. Sidekiq needs to be started in the background while Expertiza is running, but since Sidekiq was already partially merged, I think this should be expected. ''After viewing the peer reviews and newest Travis run, this appears almost ready to merge. Peer reviewers believe it is a quality project, but note that several methods lack method comments. The travis build has a single small error in models (assignment_form_spec.rb)''.
 

Files involved (some changed should not have been changed):
<li> app/controllers/assignments_controller.rb
<li> app/mailers/mail_worker.rb
<li> app/models/assignment_form.rb
<li> config/sidekiq.yml
<li> spec/models/assignment_form_spec.rb
<li> spec/sidekiq_mail_worker_spec.rb
<li> spec/spec_helper.rb

 
The code for the prior PR is linked here [https://github.com/expertiza/expertiza/pull/2089 is linked here].

==Explanation of Feature==
Mailer/mailing is referring to the sending of emails to email address associated with expertiza account. There are two types of mailing: '''synchronous and asynchronous'''. Synchronous mailing occurs after an action is taken e.g., an email that notifies when a change is made to a document. Asynchronous mailing occurs during a set time e.g., a notification email to warn students when a deadline is approaching.

=== Summary of Work Needed ===

After discussing with our project mentor and Dr. Gehringer, we discovered that what needed to be done is refining the reason why the previous PR was not deemed production-ready and resolve those issues in order to merge to main:
<li> Add insightful method comments
<li> Limit changes to delayed_mailer.rb and scheduled_task.rb (this means removing any changes to assignments_controller.rb and assignment_form.rb
<li> Increase test coverage for above files
<li> Fix failing test in assignment_form_spec.rb

 
In terms of functionality, Dr. Gehringer mentioned that asynchronous mailing is having issues e.g., not sending out an assignment deadline notification a day before it is due. If there is time, the team may debug and work to fix this. If the team is unable to resolve, this may be passed on to future teams. Dr. Gehringer did not specify if it is within scope of this project.

== Problems and planned changes ==

We are planning to accomplish the following tasks in this project:

=== Problem 1: Add insightful comments to the methods in mailer.rb===
* '''Proposed Solution''': Add insightful comments to each method.
* '''Solution Implemented''': Added meaningful comments to every major line in the mailer.rb file to reflect the functionality and purpose of each method.

=== Problem 2: Fix Bugs in sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Fix the failing existing tests.
* '''Solution Implemented''': Changed the email from expertiza.development@gmail.com to expertiza.debugging@gmail.com. Generated an email with the appropriate body, instead of a nil object.

<pre>
it "should have sent welcome email after user was created" do
Sidekiq::Testing.inline!
email = Mailer.sync_message(
to: 'tluo@ncsu.edu',
subject: 'Your Expertiza account and password has been created',
body: {
obj_name: 'assignment',
type: 'submission',
location: '1',
first_name: 'User',
partial_name: 'update'
}
).deliver_now
email = Mailer.deliveries.first
expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
expect(email.to[0]).to eq("expertiza.debugging@gmail.com")
expect(email.subject).to eq("Your Expertiza account and password has been created")
end
</pre>

<pre>
it 'should send reminder email to required email address with proper content' do
Sidekiq::Testing.inline!
Mailer.deliveries.clear
worker = MailWorker.new
worker.perform("1", "metareview", "2018-12-31 00:00:01")
expect(Mailer.deliveries.size).to eq(1)
email = Mailer.deliveries.first
expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
expect(email.bcc[0]).to eq("psingh22@ncsu.edu")
expect(email.subject).to eq("Message regarding teammate review for assignment no assignment")
expect(email.body).to eq("This is a reminder to complete teammate review for assignment no assignment.\nPlease follow the link: http://expertiza.ncsu.edu/student_task/view?id=1\nDeadline is 2018-12-31 00:00:01. If you have already done the teammate review, then please ignore this mail.")
end
</pre>

=== Problem 3: Increase the test coverage for sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Add more tests to increase test coverage.
* '''Solution Implemented''': Added the following tests:
1. Test to check an email is not sent when there is deadline with outstanding reviews.

<pre>
it "should not return email if deadline is drop_outstanding_reviews" do
Sidekiq::Testing.inline!
Mailer.deliveries.clear
worker = MailWorker.new
worker.perform("1", "drop_outstanding_reviews", "2018-12-31 00:00:01")
expect(Mailer.deliveries.size).to eq(0)
end
</pre>
2. Test to check if the size of the queue is increased when Mailworker is used.
<pre>
it "should increase the size of queue by 1 when MailWorker is used" do
Sidekiq::Testing.inline!
Mailer.deliveries.clear
worker = MailWorker.new
worker.perform(1, 'review', '2022-12-05 00:00:01')
expect(Mailer.deliveries.size).to eq(1)
end
</pre>

==Test Plan==

===Automated Testing using RSPEC===
To run the tests, run the following commands from the expertiza directory
<pre>
rspec ./spec/workers/sidekiq_mail_worker_spec.rb
</pre>
[[File:E2273_mailer_spec.png|800px]]

The current version of expertiza included tests for the assignments_form and the mail_worker, but they were far from extensive. Thus we added a few tests of our own:

<pre>
it "should have sent welcome email after user was created" do
Sidekiq::Testing.inline!
email = Mailer.sync_message(
to: 'tluo@ncsu.edu',
subject: 'Your Expertiza account and password has been created',
body: {
obj_name: 'assignment',
type: 'submission',
location: '1',
first_name: 'User',
partial_name: 'update'
}
).deliver_now
email = Mailer.deliveries.first
expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
expect(email.to[0]).to eq("expertiza.debugging@gmail.com")
expect(email.subject).to eq("Your Expertiza account and password has been created")
end
</pre>

===Testing from UI===
No changes were made to the functionality of the project, however you can access it through the setup below.

*Visit the VCL hosted site: http://152.7.99.69:8080/
**Login: Instructor6
**Password: Password

'''To Test via UI'''

1) Login with the login above

2) Click on Manage...

3) Click on Assignments

4) On the right side click the +

5) Once you've added a name, scroll to the bottom and click create

6) Repeat steps 2 and 3, to view the assignment added to the list

'''Functional Tests'''

Functional testing is done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected. A previously implemented test for Simicheck was commented out as advised by Dr. Gehringer, because it has a helper function dependency called PlagiarismCheckerHelper. According to Dr. Gehringer, in PlagiarismCheckerHelper, the request "variable is supposed to contain an http request for running the plagiarism checker. The checker has not been used for several years. It will not hurt anything important if it does not work now."

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/MichaelDacanay/expertiza-mailer-refactor/tree/E2273 GitHub Project Repository Fork]
#[https://github.com/expertiza/expertiza/pull/2478 E2273 Pull Request]
#[http://expertiza.ncsu.edu/ The live Expertiza website]

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-12-05T12:48:53Z

Vrcholay: /* Problem 1: Add insightful comments to the methods in mail_worker.rb */

__TOC__

== Topic Overview ==
===Background===
Expertiza uses Sidekiq gem for asynchronous processing of email tasks. It has a queue system to hold and then process jobs. Sidekiq’s queue replaces DelayedMailer’s queue. The previous team that worked on this also created a method perform() to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

===Prior Work===
The prior work which is the basis for this topic is linked here [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task E2144]. This work was done in Fall of 2021, and achieved much of the desired functionality for this task. This prior work will be referenced several times within this documentation, as it has a more comprehensive explanation of the underlying design principles behind the views, controller, model, algorithm, and RSpec tests implemented. These details will only be briefly summarized here.

This prior implementation was rejected due to lack of method comments and a failing test, and so correcting the test and adding method comments--and explaining how those corrections were done--will be the major focus of this project documentation.

The work in [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task E2253] was rejected because it included code changes in files that were deemed by Dr. Gehringer to be unnecessary and unassociated with the core functionality of this feature. Additionally, some of the comments are shallow and not insightful enough according to Dr. Gehringer.

=== History: Previous projects ===

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]

==Problem Statement==
<li> This project used the Sidekiq gem to asynchronously handle email tasks, which is sort of similar to E2135. Sidekiq was partially merged before, but this project changes the queue setup slightly. The refactor of methods in assignment_form.rb is significant, and several methods were moved to due_date.rb. It also adds reminder functionality to mail_worker, which also conflicts with E2135. Significant spec additions have been made from what exists in beta. Overall, code looks appropriate and well-written. Sidekiq needs to be started in the background while Expertiza is running, but since Sidekiq was already partially merged, I think this should be expected. ''After viewing the peer reviews and newest Travis run, this appears almost ready to merge. Peer reviewers believe it is a quality project, but note that several methods lack method comments. The travis build has a single small error in models (assignment_form_spec.rb)''.
 

Files involved (some changed should not have been changed):
<li> app/controllers/assignments_controller.rb
<li> app/mailers/mail_worker.rb
<li> app/models/assignment_form.rb
<li> config/sidekiq.yml
<li> spec/models/assignment_form_spec.rb
<li> spec/sidekiq_mail_worker_spec.rb
<li> spec/spec_helper.rb

 
The code for the prior PR is linked here [https://github.com/expertiza/expertiza/pull/2089 is linked here].

==Explanation of Feature==
Mailer/mailing is referring to the sending of emails to email address associated with expertiza account. There are two types of mailing: '''synchronous and asynchronous'''. Synchronous mailing occurs after an action is taken e.g., an email that notifies when a change is made to a document. Asynchronous mailing occurs during a set time e.g., a notification email to warn students when a deadline is approaching.

=== Summary of Work Needed ===

After discussing with our project mentor and Dr. Gehringer, we discovered that what needed to be done is refining the reason why the previous PR was not deemed production-ready and resolve those issues in order to merge to main:
<li> Add insightful method comments
<li> Limit changes to delayed_mailer.rb and scheduled_task.rb (this means removing any changes to assignments_controller.rb and assignment_form.rb
<li> Increase test coverage for above files
<li> Fix failing test in assignment_form_spec.rb

 
In terms of functionality, Dr. Gehringer mentioned that asynchronous mailing is having issues e.g., not sending out an assignment deadline notification a day before it is due. If there is time, the team may debug and work to fix this. If the team is unable to resolve, this may be passed on to future teams. Dr. Gehringer did not specify if it is within scope of this project.

== Problems and planned changes ==

We are planning to accomplish the following tasks in this project:

=== Problem 1: Add insightful comments to the methods in mailer.rb===
* '''Proposed Solution''': Add insightful comments to each method.
* '''Solution Implemented''': Added meaningful comments to every major line in the mailer.rb file to reflect the functionality and purpose of each method.

=== Problem 2: Fix Bugs in sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Fix the failing existing tests.
* '''Solution Implemented''': Changed the email from expertiza.development@gmail.com to expertiza.debugging@gmail.com. Generated an email with the appropriate body, instead of a nil object.

<pre>
it "should have sent welcome email after user was created" do
Sidekiq::Testing.inline!
email = Mailer.sync_message(
to: 'tluo@ncsu.edu',
subject: 'Your Expertiza account and password has been created',
body: {
obj_name: 'assignment',
type: 'submission',
location: '1',
first_name: 'User',
partial_name: 'update'
}
).deliver_now
email = Mailer.deliveries.first
expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
expect(email.to[0]).to eq("expertiza.debugging@gmail.com")
expect(email.subject).to eq("Your Expertiza account and password has been created")
end
</pre>

<pre>
it 'should send reminder email to required email address with proper content' do
Sidekiq::Testing.inline!
Mailer.deliveries.clear
worker = MailWorker.new
worker.perform("1", "metareview", "2018-12-31 00:00:01")
expect(Mailer.deliveries.size).to eq(1)
email = Mailer.deliveries.first
expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
expect(email.bcc[0]).to eq("psingh22@ncsu.edu")
expect(email.subject).to eq("Message regarding teammate review for assignment no assignment")
expect(email.body).to eq("This is a reminder to complete teammate review for assignment no assignment.\nPlease follow the link: http://expertiza.ncsu.edu/student_task/view?id=1\nDeadline is 2018-12-31 00:00:01. If you have already done the teammate review, then please ignore this mail.")
end
</pre>

=== Problem 3: Increase the test coverage for sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Add more tests to increase test coverage.
* '''Solution Implemented''': Added the following tests:
1. Test to check an email is not sent when there is deadline with outstanding reviews.

<pre>
it "should not return email if deadline is drop_outstanding_reviews" do
Sidekiq::Testing.inline!
Mailer.deliveries.clear
worker = MailWorker.new
worker.perform("1", "drop_outstanding_reviews", "2018-12-31 00:00:01")
expect(Mailer.deliveries.size).to eq(0)
end
</pre>

==Test Plan==

===Automated Testing using RSPEC===
To run the tests, run the following commands from the expertiza directory
<pre>
rspec ./spec/workers/sidekiq_mail_worker_spec.rb
</pre>
[[File:E2273_mailer_spec.png|800px]]

The current version of expertiza included tests for the assignments_form and the mail_worker, but they were far from extensive. Thus we added a few tests of our own:

<pre>
it "should have sent welcome email after user was created" do
Sidekiq::Testing.inline!
email = Mailer.sync_message(
to: 'tluo@ncsu.edu',
subject: 'Your Expertiza account and password has been created',
body: {
obj_name: 'assignment',
type: 'submission',
location: '1',
first_name: 'User',
partial_name: 'update'
}
).deliver_now
email = Mailer.deliveries.first
expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
expect(email.to[0]).to eq("expertiza.debugging@gmail.com")
expect(email.subject).to eq("Your Expertiza account and password has been created")
end
</pre>

===Testing from UI===
No changes were made to the functionality of the project, however you can access it through the setup below.

*Visit the VCL hosted site: http://152.7.99.69:8080/
**Login: Instructor6
**Password: Password

'''To Test via UI'''

1) Login with the login above

2) Click on Manage...

3) Click on Assignments

4) On the right side click the +

5) Once you've added a name, scroll to the bottom and click create

6) Repeat steps 2 and 3, to view the assignment added to the list

'''Functional Tests'''

Functional testing is done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected. A previously implemented test for Simicheck was commented out as advised by Dr. Gehringer, because it has a helper function dependency called PlagiarismCheckerHelper. According to Dr. Gehringer, in PlagiarismCheckerHelper, the request "variable is supposed to contain an http request for running the plagiarism checker. The checker has not been used for several years. It will not hurt anything important if it does not work now."

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/MichaelDacanay/expertiza-mailer-refactor/tree/E2273 GitHub Project Repository Fork]
#[https://github.com/expertiza/expertiza/pull/2478 E2273 Pull Request]
#[http://expertiza.ncsu.edu/ The live Expertiza website]

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T23:29:33Z

Vrcholay: /* Automated Testing using RSPEC */

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T23:28:58Z

Vrcholay: /* Problem 1: Add insightful comments to the methods in mail_worker.rb */

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T23:28:24Z

Vrcholay: /* Problem 1: Add insightful comments to the methods in mail_worker.rb */

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T23:26:51Z

Vrcholay: /* Problem 2: Fix Bugs in sidekiq_mail_worker_spec.rb */

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T23:23:23Z

Vrcholay: /* Problem 3: Increase the test coverage for sidekiq_mail_worker_spec.rb */

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T23:23:03Z

Vrcholay:

CSC/ECE 517 Fall 2022

2022-11-16T21:17:04Z

Vrcholay: /* Final Projects */

== OSS Projects ==

* [[CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb]]
* [[CSC/ECE 517 Fall 2022 - E2252. Refactor auth controller.rb & password retrieval controller.rb]]
* [[CSC/ECE 517 Fall 2022 - E2253.Refactor delayed mailer.rb and scheduled task.rb]]
* [[CSC/ECE 517 Fall 2022 - E2254: Refactor teams_controller.rb]]
* [[CSC/ECE 517 Fall 2022 - E2255. Refactor student_quizzes_controller.rb]]
* [[CSC/ECE 517 Fall 2022 - E2256: Refactor late_policies_controller.rb]]
* [[CSC/ECE 517 Fall 2022 - E2257: Refactor questionnaires_controller.rb]]
* [[CSC/ECE 517 Fall 2022 - E2258. Refactor submitted_content_controller]]
* [[CSC/ECE 517 Fall 2022 - E2259. Refactor signup_sheet_controller]]
* [[CSC/ECE 517 Fall 2022 - E2260. Refactor review_mapping_controller]]
* [[CSC/ECE 517 Fall 2022 - E2255. Refactor review_mapping_helper.rb]]
* [[CSC/ECE 517 Fall 2022 - E2262: Refactor review_mapping_helper.rb]]
* [[CSC/ECE 517 Fall 2022 - E2263.Refactor student teams functionality]]

== Final Projects ==
* [[CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb]]
* [[CSC/ECE 517 Fall 2022 - E2275. Further refactoring and improvement of student_quizzes controller]]
* [[CSC/ECE 517 Fall 2022 - E2276. Fix "Back" link on “New Late Policy” page]]
* [[CSC/ECE 517 Fall 2022 - E2277. Further Refactoring questionnaires_controller.rb]]
* [[CSC/ECE 517 Fall 2022 - E2278. Improve assessment360 controller]]
* [[CSC/ECE 517 Fall 2022 - E2279. Further refactoring and improvement of signup_sheet controller]]
* [[CSC/ECE 517 Fall 2022 - E2281. Reimplement Waitlists]]
* [[CSC/ECE 517 Fall 2022 - E2283. Refactor student_teams functionality]]
* [[CSC/ECE 517 Fall 2022 - E2284. Calibration submissions should be copied along with calibration assignments]]
* [[CSC/ECE 517 Fall 2022 - E2285. Grading audit trail]]

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T15:05:54Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T12:14:04Z

Vrcholay: /* References */

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T11:56:27Z

Vrcholay: /* Problems and planned changes */

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T11:51:15Z

Vrcholay: /* Problems and planned changes */

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-16T11:50:26Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-15T14:31:16Z

Vrcholay:

__TOC__

== Background ==

Expertiza uses Sidekiq gem for asynchronous processing of email tasks. It has a queue system to hold and then process jobs. Sidekiq’s queue replaces DelayedMailer’s queue. The previous team that worked on this also created a method perform() to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

== History: Previous projects ==

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]

==About Assignment Controller==
This class is responsible for the creation and implementation of assignment forms. This includes due dates, questionnaires and assigning reviews.

'''Previous Information''' can be found here [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task E2144]

Files Involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

== Problems and planned changes ==

We are planning to accomplish the following tasks in this project:

=== Problem 1: Refactor the create method in assignment_controller.rb file ===
* '''Proposed Solution''': Split the whole function into multiple methods performing specific tasks. Change the name of the variables to meaningful names in the method.
* '''Solution Implemented''': Refactored the create method to split tasks into different methods and renamed a variable to correctly reflect its purpose.
<pre>
# creates a new assignment via the assignment form
def create
@assignment_form = AssignmentForm.new(assignment_form_params)
if params[:button]
# first check if assignment name exists, if not then create new, otherwise don't create new assignment
find_existing_assignment = Assignment.find_by(name: @assignment_form.assignment.name, course_id: @assignment_form.assignment.course_id)
dir_path = assignment_form_params[:assignment][:directory_path]
find_existing_directory = Assignment.find_by(directory_path: dir_path, course_id: @assignment_form.assignment.course_id)
if !find_existing_assignment && !find_existing_directory && @assignment_form.save # No existing names/directories
assignment_creation_success
# return
else
assignemnt_creation_failure(find_existing_assignment, dir_path, find_existing_directory)
end
else
render 'new'
undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ")
end
end
# create a new assignment via the assigment form if the new assignment name and directory doesn't exist
def assignment_creation_success
@assignment_form.create_assignment_node
exist_assignment = Assignment.find(@assignment_form.assignment.id)
assignment_form_params[:assignment][:id] = exist_assignment.id.to_s
if assignment_form_params[:assignment][:directory_path].blank? # No existing assignment for assignment form
assignment_form_params[:assignment][:directory_path] = "assignment_#{assignment_form_params[:assignment][:id]}"
end
assignment_form_params[:assignment_questionnaire] = assign_questionnaire_array(exist_assignment)
assignment_form_params[:due_date] = assign_due_date_array(exist_assignment)
@assignment_form.update(assignment_form_params, current_user)
assignment_id = Assignment.find(@assignment_form.assignment.id).id
ExpertizaLogger.info "Assignment created: #{@assignment_form.as_json}"
redirect_to edit_assignment_path assignment_id
undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ")
end

# raises a flash error messgae when it fails to create an assignment and redirect to assignement/new page
def assignemnt_creation_failure(existing_assignment, dir_path, existing_directory)
flash[:error] = 'Failed to create assignment.'
if existing_assignment
flash[:error] << ' ' + @assignment_form.assignment.name + ' already exists as an assignment name'
end
if existing_directory
flash[:error] << ' ' + dir_path + ' already exists as a submission directory name'
end
redirect_to '/assignments/new?private=1'
end

</pre>

=== Problem 2: Refactor the edit_params_setting method in assignment_controller.rb file ===
* '''Proposed Solution''': Split the whole function into multiple methods performing specific tasks.
*'''Solution Implemented''': Refactored edit_params_setting by extracting functionality on setting submissions and review rounds.
<pre>
def set_submissions_and_reviews_rounds
# The submission round i.e. Round 1 (before 1st deadline), Round 2 (after first peer review, before 2nd deadline)
@num_submissions_round = @assignment.find_due_dates('submission').nil? ? 0 : @assignment.find_due_dates('submission').count
@num_reviews_round = @assignment.find_due_dates('review').nil? ? 0 : @assignment.find_due_dates('review').count
end

# populates values and settings of the assignment for editing
def edit_params_setting
@assignment = Assignment.find(params[:id])
@topics = SignUpTopic.where(assignment_id: params[:id])
@assignment_form = AssignmentForm.create_form_object(params[:id])
@user = current_user

set_submissions_and_reviews_rounds

@assignment_questionnaires = AssignmentQuestionnaire.where(assignment_id: params[:id])

# Set due date
@due_date_all = AssignmentDueDate.where(parent_id: params[:id])
@due_date_nameurl_not_empty = false
@due_date_nameurl_not_empty_checkbox = false

# Set metareview (self-review)
@metareview_allowed = false
@metareview_allowed_checkbox = false

# Set up signup and drop topic
@signup_allowed = false
@signup_allowed_checkbox = false
@drop_topic_allowed = false
@drop_topic_allowed_checkbox = false

# Set up team formation
@team_formation_allowed = false
@team_formation_allowed_checkbox = false

# Set up participants and teams counts
@participants_count = @assignment_form.assignment.participants.size
@teams_count = @assignment_form.assignment.teams.size
end
</pre>

=== Problem 3: Fix Bugs in sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Comment out or Fix the failing existing tests.

=== Problem 4: Increase the test coverage for sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Add more tests to increase test coverage.

==Automated Testing using RSPEC==
The current version of expertiza included tests for the assignments_form and the mail_worker, but they were far from extensive. Thus we added a few tests of our own.

To run the tests, run the following commands from the expertiza directory
<pre>
rspec ./spec/models/assignment_form_spec.rb

rspec ./spec/workers/sidekiq_mail_worker_spec.rb
</pre>

==Testing from UI==
No changes were made to the functionality of the project, however you can access it through the setup below.

*Visit the VCL hosted site: http://152.7.99.69:8080/
**Login: Instructor6
**Password: Password

'''To Test via UI'''

1) Login with the login above

2) Click on Manage...

3) Click on Assignments

4) On the right side click the +

5) Once you've added a name, scroll to the bottom and click create

6) Repeat steps 2 and 3, to view the assignment added to the list

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/MichaelDacanay/expertiza-mailer-refactor/tree/team-E2273 GitHub Project Repository Fork]
#[https://github.com/expertiza/expertiza/pull/2477 E2273 Pull Request]
#[http://expertiza.ncsu.edu/ The live Expertiza website]

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-15T14:02:47Z

Vrcholay:

__TOC__

== Background ==

Expertiza uses Sidekiq gem for asynchronous processing of email tasks. It has a queue system to hold and then process jobs. Sidekiq’s queue replaces DelayedMailer’s queue. The previous team that worked on this also created a method perform() to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

== History: Previous projects ==

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]

==About Assignment Controller==
This class is responsible for the creation and implementation of assignment forms. This includes due dates, questionnaires and assigning reviews.

'''Previous Information''' can be found here [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task E2144]

Files Involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

== Problems and planned changes ==

We are planning to accomplish the following tasks in this project:

=== Problem 1: Refactor the create method in assignment_controller.rb file ===
* '''Proposed Solution''': Split the whole function into multiple methods performing specific tasks. Change the name of the variables to meaningful names in the method.
* '''Solution Implemented''': Refactored the create method to split tasks into different methods and renamed a variable to correctly reflect its purpose.
<pre>
# creates a new assignment via the assignment form
def create
@assignment_form = AssignmentForm.new(assignment_form_params)
if params[:button]
# first check if assignment name exists, if not then create new, otherwise don't create new assignment
find_existing_assignment = Assignment.find_by(name: @assignment_form.assignment.name, course_id: @assignment_form.assignment.course_id)
dir_path = assignment_form_params[:assignment][:directory_path]
find_existing_directory = Assignment.find_by(directory_path: dir_path, course_id: @assignment_form.assignment.course_id)
if !find_existing_assignment && !find_existing_directory && @assignment_form.save # No existing names/directories
assignment_creation_success
# return
else
assignemnt_creation_failure(find_existing_assignment, dir_path, find_existing_directory)
end
else
render 'new'
undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ")
end
end
# create a new assignment via the assigment form if the new assignment name and directory doesn't exist
def assignment_creation_success
@assignment_form.create_assignment_node
exist_assignment = Assignment.find(@assignment_form.assignment.id)
assignment_form_params[:assignment][:id] = exist_assignment.id.to_s
if assignment_form_params[:assignment][:directory_path].blank? # No existing assignment for assignment form
assignment_form_params[:assignment][:directory_path] = "assignment_#{assignment_form_params[:assignment][:id]}"
end
assignment_form_params[:assignment_questionnaire] = assign_questionnaire_array(exist_assignment)
assignment_form_params[:due_date] = assign_due_date_array(exist_assignment)
@assignment_form.update(assignment_form_params, current_user)
assignment_id = Assignment.find(@assignment_form.assignment.id).id
ExpertizaLogger.info "Assignment created: #{@assignment_form.as_json}"
redirect_to edit_assignment_path assignment_id
undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ")
end

# raises a flash error messgae when it fails to create an assignment and redirect to assignement/new page
def assignemnt_creation_failure(existing_assignment, dir_path, existing_directory)
flash[:error] = 'Failed to create assignment.'
if existing_assignment
flash[:error] << ' ' + @assignment_form.assignment.name + ' already exists as an assignment name'
end
if existing_directory
flash[:error] << ' ' + dir_path + ' already exists as a submission directory name'
end
redirect_to '/assignments/new?private=1'
end

</pre>

=== Problem 2: Refactor the edit_params_setting method in assignment_controller.rb file ===
* '''Proposed Solution''': Split the whole function into multiple methods performing specific tasks.
*'''Solution Implemented''': Refactored edit_params_setting by extracting functionality on setting submissions and review rounds.
<pre>
def set_submissions_and_reviews_rounds
# The submission round i.e. Round 1 (before 1st deadline), Round 2 (after first peer review, before 2nd deadline)
@num_submissions_round = @assignment.find_due_dates('submission').nil? ? 0 : @assignment.find_due_dates('submission').count
@num_reviews_round = @assignment.find_due_dates('review').nil? ? 0 : @assignment.find_due_dates('review').count
end

# populates values and settings of the assignment for editing
def edit_params_setting
@assignment = Assignment.find(params[:id])
@topics = SignUpTopic.where(assignment_id: params[:id])
@assignment_form = AssignmentForm.create_form_object(params[:id])
@user = current_user

set_submissions_and_reviews_rounds

@assignment_questionnaires = AssignmentQuestionnaire.where(assignment_id: params[:id])

# Set due date
@due_date_all = AssignmentDueDate.where(parent_id: params[:id])
@due_date_nameurl_not_empty = false
@due_date_nameurl_not_empty_checkbox = false

# Set metareview (self-review)
@metareview_allowed = false
@metareview_allowed_checkbox = false

# Set up signup and drop topic
@signup_allowed = false
@signup_allowed_checkbox = false
@drop_topic_allowed = false
@drop_topic_allowed_checkbox = false

# Set up team formation
@team_formation_allowed = false
@team_formation_allowed_checkbox = false

# Set up participants and teams counts
@participants_count = @assignment_form.assignment.participants.size
@teams_count = @assignment_form.assignment.teams.size
end
</pre>
=== Problem 3: Increase the test coverage for sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Add more tests to increase test coverage.

==Automated Testing using RSPEC==
The current version of expertiza included tests for the assignments_form and the mail_worker, but they were far from extensive. Thus we added a few tests of our own.

To run the tests, run the following commands from the expertiza directory
<pre>
rspec ./spec/models/assignment_form_spec.rb

rspec ./spec/workers/sidekiq_mail_worker_spec.rb
</pre>

==Testing from UI==
No changes were made to the functionality of the project, however you can access it through the setup below.

*Visit the VCL hosted site: http://152.7.99.69:8080/
**Login: Instructor6
**Password: Password

'''To Test via UI'''

1) Login with the login above

2) Click on Manage...

3) Click on Assignments

4) On the right side click the +

5) Once you've added a name, scroll to the bottom and click create

6) Repeat steps 2 and 3, to view the assignment added to the list

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/MichaelDacanay/expertiza-mailer-refactor/tree/team-E2273 GitHub Project Repository Fork]
#[https://github.com/expertiza/expertiza/pull/2477 E2273 Pull Request]
#[http://expertiza.ncsu.edu/ The live Expertiza website]

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-13T00:18:06Z

Vrcholay:

__TOC__

== Background ==

Expertiza uses Sidekiq gem for asynchronous processing of email tasks. It has a queue system to hold and then process jobs. Sidekiq’s queue replaces DelayedMailer’s queue. The previous team that worked on this also created a method perform() to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

== History: Previous projects ==

* E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
* E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]

==About Assignment Controller==
This class is responsible for the creation and implementation of assignment forms. This includes due dates, questionnaires and assigning reviews.

'''Previous Information''' can be found here [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task E2144]

Files Involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

== Problems and planned changes ==

We are planning to accomplish the following tasks in this project:

=== Problem 1: Refactor the create method in assignment_controller.rb file ===
* '''Proposed Solution''': Split the whole function into multiple methods performing specific tasks. Change the name of the variables to meaningful names in the method.
* '''Solution Implemented''': Refactored the create method to split tasks into different methods and renamed a variable to correctly reflect its purpose.
<pre>
# creates a new assignment via the assignment form
def create
@assignment_form = AssignmentForm.new(assignment_form_params)
if params[:button]
# first check if assignment name exists, if not then create new, otherwise don't create new assignment
find_existing_assignment = Assignment.find_by(name: @assignment_form.assignment.name, course_id: @assignment_form.assignment.course_id)
dir_path = assignment_form_params[:assignment][:directory_path]
find_existing_directory = Assignment.find_by(directory_path: dir_path, course_id: @assignment_form.assignment.course_id)
if !find_existing_assignment && !find_existing_directory && @assignment_form.save # No existing names/directories
assignment_creation_success
# return
else
assignemnt_creation_failure(find_existing_assignment, dir_path, find_existing_directory)
end
else
render 'new'
undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ")
end
end
# create a new assignment via the assigment form if the new assignment name and directory doesn't exist
def assignment_creation_success
@assignment_form.create_assignment_node
exist_assignment = Assignment.find(@assignment_form.assignment.id)
assignment_form_params[:assignment][:id] = exist_assignment.id.to_s
if assignment_form_params[:assignment][:directory_path].blank? # No existing assignment for assignment form
assignment_form_params[:assignment][:directory_path] = "assignment_#{assignment_form_params[:assignment][:id]}"
end
assignment_form_params[:assignment_questionnaire] = assign_questionnaire_array(exist_assignment)
assignment_form_params[:due_date] = assign_due_date_array(exist_assignment)
@assignment_form.update(assignment_form_params, current_user)
assignment_id = Assignment.find(@assignment_form.assignment.id).id
ExpertizaLogger.info "Assignment created: #{@assignment_form.as_json}"
redirect_to edit_assignment_path assignment_id
undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ")
end

# raises a flash error messgae when it fails to create an assignment and redirect to assignement/new page
def assignemnt_creation_failure(existing_assignment, dir_path, existing_directory)
flash[:error] = 'Failed to create assignment.'
if existing_assignment
flash[:error] << ' ' + @assignment_form.assignment.name + ' already exists as an assignment name'
end
if existing_directory
flash[:error] << ' ' + dir_path + ' already exists as a submission directory name'
end
redirect_to '/assignments/new?private=1'
end

</pre>

=== Problem 2: Refactor the edit_params_setting method in assignment_controller.rb file ===
* '''Proposed Solution''': Split the whole function into multiple methods performing specific tasks.

=== Problem 3: Increase the test coverage for sidekiq_mail_worker_spec.rb ===

* '''Proposed Solution''': Add more tests to increase test coverage.

==Automated Testing using RSPEC==
The current version of expertiza included tests for the assignments_form and the mail_worker, but they were far from extensive. Thus we added a few tests of our own.

To run the tests, run the following commands from the expertiza directory
<pre>
rspec ./spec/models/assignment_form_spec.rb

rspec ./spec/workers/sidekiq_mail_worker_spec.rb
</pre>

==Testing from UI==
No changes were made to the functionality of the project, however you can access it through the setup below.

*Visit the VCL hosted site: http://152.7.99.69:8080/
**Login: Instructor6
**Password: Password

'''To Test via UI'''

1) Login with the login above

2) Click on Manage...

3) Click on Assignments

4) On the right side click the +

5) Once you've added a name, scroll to the bottom and click create

6) Repeat steps 2 and 3, to view the assignment added to the list

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/MichaelDacanay/expertiza-mailer-refactor/tree/team-E2273 GitHub Project Repository Fork]
#[https://github.com/expertiza/expertiza/pull/2477 E2273 Pull Request]
#[http://expertiza.ncsu.edu/ The live Expertiza website]

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-13T00:16:06Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-13T00:09:38Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-11T19:01:28Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-10T19:55:25Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-09T19:28:09Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-09T19:27:39Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-09T17:31:07Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-09T17:23:49Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-08T21:24:14Z

Vrcholay:

__TOC__

===Problem Statement===
We are planning to accomplish the following tasks in this project:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-08T21:23:25Z

Vrcholay:

===Problem Statement===
We are planning to accomplish the following tasks in this project:

CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb

2022-11-08T18:08:05Z

Vrcholay: Created page with "===Problem Statement=== The following tasks were accomplished in this project:"

===Problem Statement===
The following tasks were accomplished in this project:

CSC/ECE 517 Fall 2022

2022-11-02T12:52:35Z

Vrcholay:

CSC/ECE 517 Fall 2022

2022-11-02T12:51:24Z

Vrcholay:

CSC/ECE 517 Fall 2022

2022-10-26T01:06:01Z

Vrcholay:

== OSS Projects ==

* [[CSC/ECE 517 Fall 2022 - E2253.Refactor delayed mailer.rb and scheduled task.rb]]
* [[CSC/ECE 517 Fall 2022 - E2256: Refactor late_policies_controller.rb]]
* [[CSC/ECE 517 Fall 2022 - E2258. Refactor submitted_content_controller]]
* [[CSC/ECE 517 Fall 2022 - E2259. Refactor signup_sheet_controller]]
* [[CSC/ECE 517 Fall 2022 - E2260. Refactor review_mapping_controller]]
* [[CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb]]

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-26T01:03:34Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
* Increased the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

* '''Problem 5''': In views/suggestion/show.html.erb and views/suggestion/student_view.html.erb, there is a DRY violation which needs to be fixed.

* '''Solution''': Merge views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
<pre>
<% if current_user_role?.name.eql?("Student") or session[:flip_user] %>

.....
else
.....
<%end%>
</pre>

* '''Problem 6''': Increase the test coverage of the controller and functionality by adding more tests.

* '''Solution''': Increase the test coverage of the suggestion controller by adjusting current test for above problems and adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

<pre>
it 'adds a participant is not successfull' do
allow_any_instance_of(SuggestionComment).to receive(:save).and_return(false)
request_params = { id: 1, suggestion_comment: { vote: 'Y', comments: 'comments' } }
user_session = { user: instructor }
get :add_comment, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'There was an error in adding your comment.'
end
</pre>

<pre>
it 'rejecting a suggestion is not successfull' do
allow_any_instance_of(Suggestion).to receive(:update_attribute).and_return(false)
request_params = { id: 1, reject_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when rejecting the suggestion.'
end
</pre>

<pre>
it 'accept a suggestion is not successfull' do
allow(SignUpTopic).to receive(:new_topic_from_suggestion).and_return('failed')
allow_any_instance_of(SuggestionController).to receive(:notification).and_return(true)
request_params = { id: 1, approve_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when approving the suggestion.'
end
</pre>

===Automated Testing using RSPEC===
Go to the expertiza source code root path and use following command to run test:
<pre>
$ rspec spec/contollers/suggestion_controller_spec.rb
</pre>
Here is the operation video link:

https://youtu.be/qzkdNmTnAIc

====Test Plan====
[[File:TestFlow.png|900px]]

====Travis CI====

Also , our project passed the Travis CI build test.

[[File:TravisCISuggestionController.png]]

====Coverage====
Our coverage increased 8.04%.

* '''Before Changes'''
[[File:CoverageBeforeChanges.png|900px]]

* '''After Changes'''

[[File:CoverageAfterChanges.png|900px]]

===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.119:8080/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

File:TravisCISuggestionController.png

2022-10-26T01:01:29Z

Vrcholay: Vrcholay uploaded a new version of File:TravisCISuggestionController.png

File:TravisCISuggestionController.png

2022-10-26T00:59:17Z

Vrcholay: Vrcholay uploaded a new version of File:TravisCISuggestionController.png

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-26T00:58:39Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
* Increased the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

* '''Problem 5''': In views/suggestion/show.html.erb and views/suggestion/student_view.html.erb, there is a DRY violation which needs to be fixed.

* '''Solution''': Merge views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
<pre>
<% if current_user_role?.name.eql?("Student") or session[:flip_user] %>

.....
else
.....
<%end%>
</pre>

* '''Problem 6''': Increase the test coverage of the controller and functionality by adding more tests.

* '''Solution''': Increase the test coverage of the suggestion controller by adjusting current test for above problems and adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

<pre>
it 'adds a participant is not successfull' do
allow_any_instance_of(SuggestionComment).to receive(:save).and_return(false)
request_params = { id: 1, suggestion_comment: { vote: 'Y', comments: 'comments' } }
user_session = { user: instructor }
get :add_comment, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'There was an error in adding your comment.'
end
</pre>

<pre>
it 'rejecting a suggestion is not successfull' do
allow_any_instance_of(Suggestion).to receive(:update_attribute).and_return(false)
request_params = { id: 1, reject_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when rejecting the suggestion.'
end
</pre>

<pre>
it 'accept a suggestion is not successfull' do
allow(SignUpTopic).to receive(:new_topic_from_suggestion).and_return('failed')
allow_any_instance_of(SuggestionController).to receive(:notification).and_return(true)
request_params = { id: 1, approve_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when approving the suggestion.'
end
</pre>

===Automated Testing using RSPEC===
Go to the expertiza source code root path and use following command to run test:
<pre>
$ rspec spec/contollers/suggestion_controller_spec.rb
</pre>
Here is the operation video link:

https://youtu.be/qzkdNmTnAIc

====Test Plan====
[[File:TravisCISuggestionController.png|900px]]

====Travis CI====

Also , our project passed the Travis CI build test.

[[File:travisCL.png|900px]]

====Coverage====
Our coverage increased 8.04%.

* '''Before Changes'''
[[File:CoverageBeforeChanges.png|900px]]

* '''After Changes'''

[[File:CoverageAfterChanges.png|900px]]

===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.119:8080/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

File:TravisCISuggestionController.png

2022-10-26T00:58:10Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-26T00:56:47Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
* Increased the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

* '''Problem 5''': In views/suggestion/show.html.erb and views/suggestion/student_view.html.erb, there is a DRY violation which needs to be fixed.

* '''Solution''': Merge views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
<pre>
<% if current_user_role?.name.eql?("Student") or session[:flip_user] %>

.....
else
.....
<%end%>
</pre>

* '''Problem 6''': Increase the test coverage of the controller and functionality by adding more tests.

* '''Solution''': Increase the test coverage of the suggestion controller by adjusting current test for above problems and adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

<pre>
it 'adds a participant is not successfull' do
allow_any_instance_of(SuggestionComment).to receive(:save).and_return(false)
request_params = { id: 1, suggestion_comment: { vote: 'Y', comments: 'comments' } }
user_session = { user: instructor }
get :add_comment, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'There was an error in adding your comment.'
end
</pre>

<pre>
it 'rejecting a suggestion is not successfull' do
allow_any_instance_of(Suggestion).to receive(:update_attribute).and_return(false)
request_params = { id: 1, reject_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when rejecting the suggestion.'
end
</pre>

<pre>
it 'accept a suggestion is not successfull' do
allow(SignUpTopic).to receive(:new_topic_from_suggestion).and_return('failed')
allow_any_instance_of(SuggestionController).to receive(:notification).and_return(true)
request_params = { id: 1, approve_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when approving the suggestion.'
end
</pre>

===Automated Testing using RSPEC===
Go to the expertiza source code root path and use following command to run test:
<pre>
$ rspec spec/contollers/suggestion_controller_spec.rb
</pre>
Here is the operation video link:

https://youtu.be/qzkdNmTnAIc

====Test Plan====
[[File:TestFlow.png|900px]]

====Travis CI====

Also , our project passed the Travis CI build test.

[[File:travisCL.png|900px]]

====Coverage====
Our coverage increased 8.04%.

* '''Before Changes'''
[[File:CoverageBeforeChanges.png|900px]]

* '''After Changes'''

[[File:CoverageAfterChanges.png|900px]]

===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.119:8080/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-25T11:26:04Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
* Increased the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

* '''Problem 5''': In views/suggestion/show.html.erb and views/suggestion/student_view.html.erb, there is a DRY violation which needs to be fixed.

* '''Solution''': Merge views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.

* '''Problem 6''': Increase the test coverage of the controller and functionality by adding more tests.

* '''Solution''': Increase the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

<pre>
it 'adds a participant is not successfull' do
allow_any_instance_of(SuggestionComment).to receive(:save).and_return(false)
request_params = { id: 1, suggestion_comment: { vote: 'Y', comments: 'comments' } }
user_session = { user: instructor }
get :add_comment, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'There was an error in adding your comment.'
end
</pre>

<pre>
it 'rejecting a suggestion is not successfull' do
allow_any_instance_of(Suggestion).to receive(:update_attribute).and_return(false)
request_params = { id: 1, reject_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when rejecting the suggestion.'
end
</pre>

<pre>
it 'accept a suggestion is not successfull' do
allow(SignUpTopic).to receive(:new_topic_from_suggestion).and_return('failed')
allow_any_instance_of(SuggestionController).to receive(:notification).and_return(true)
request_params = { id: 1, approve_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when approving the suggestion.'
end
</pre>

===Automated Testing using RSPEC===
Go to the expertiza source code root path and use following command to run test:
<pre>
$ rspec spec/contollers/suggestion_controller_spec.rb
</pre>
Here is the operation video link:

https://youtu.be/qzkdNmTnAIc

====Test Plan====
[[File:TestFlow.png|900px]]

====Coverage====
Our coverage increased 8.04%.

* '''Before Changes'''
[[File:CoverageBeforeChanges.png|900px]]

* '''After Changes'''

[[File:CoverageAfterChanges.png|900px]]

===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.119:8080/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

File:CoverageAfterChanges.png

2022-10-25T11:21:27Z

Vrcholay:

File:CoverageBeforeChanges.png

2022-10-25T11:20:16Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-25T11:13:22Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
* Increased the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

* '''Problem 5''': In views/suggestion/show.html.erb and views/suggestion/student_view.html.erb, there is a DRY violation which needs to be fixed.

* '''Solution''': Merge views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.

* '''Problem 6''': Increase the test coverage of the controller and functionality by adding more tests.

* '''Solution''': Increase the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

<pre>
it 'adds a participant is not successfull' do
allow_any_instance_of(SuggestionComment).to receive(:save).and_return(false)
request_params = { id: 1, suggestion_comment: { vote: 'Y', comments: 'comments' } }
user_session = { user: instructor }
get :add_comment, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'There was an error in adding your comment.'
end
</pre>

<pre>
it 'rejecting a suggestion is not successfull' do
allow_any_instance_of(Suggestion).to receive(:update_attribute).and_return(false)
request_params = { id: 1, reject_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when rejecting the suggestion.'
end
</pre>

<pre>
it 'accept a suggestion is not successfull' do
allow(SignUpTopic).to receive(:new_topic_from_suggestion).and_return('failed')
allow_any_instance_of(SuggestionController).to receive(:notification).and_return(true)
request_params = { id: 1, approve_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when approving the suggestion.'
end
</pre>

===Automated Testing using RSPEC===
Go to the expertiza source code root path and use following command to run test:
<pre>
$ rspec spec/contollers/suggestion_controller_spec.rb
</pre>
Here is the operation video link:

https://youtu.be/qzkdNmTnAIc

====Test Plan====
[[File:TestFlow.png|900px]]

====Travis CI====

Also , our project passed the Travis CI build test.

[[File:travisCL.png|900px]]

====Coverage====
Our coverage increased 4.77%.

[[File:CoverageSuggestion.png|900px]]

===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.119:8080/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-25T11:09:10Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
* Increased the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

* '''Problem 5''': In views/suggestion/show.html.erb and views/suggestion/student_view.html.erb, there is a DRY violation which needs to be fixed.

* '''Solution''': Merge views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.

* '''Problem 6''': Increase the test coverage of the controller and functionality by adding more tests.

* '''Solution''': Increase the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

<pre>
it 'adds a participant is not successfull' do
allow_any_instance_of(SuggestionComment).to receive(:save).and_return(false)
request_params = { id: 1, suggestion_comment: { vote: 'Y', comments: 'comments' } }
user_session = { user: instructor }
get :add_comment, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'There was an error in adding your comment.'
end
</pre>

<pre>
it 'rejecting a suggestion is not successfull' do
allow_any_instance_of(Suggestion).to receive(:update_attribute).and_return(false)
request_params = { id: 1, reject_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when rejecting the suggestion.'
end
</pre>

<pre>
it 'accept a suggestion is not successfull' do
allow(SignUpTopic).to receive(:new_topic_from_suggestion).and_return('failed')
allow_any_instance_of(SuggestionController).to receive(:notification).and_return(true)
request_params = { id: 1, approve_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when approving the suggestion.'
end
</pre>

===Automated Testing using RSPEC===
Go to the expertiza source code root path and use following command to run test:
<pre>
$ rspec spec/contollers/suggestion_controller_spec.rb
</pre>
Here is the operation video link:

https://youtu.be/_HsUSm39iEQ

====Test Plan====
[[File:TestFlow.png|900px]]

====Travis CI====

Also , our project passed the Travis CI build test.

[[File:travisCL.png|900px]]

====Coverage====
Our coverage increased 4.77%.

[[File:CoverageSuggestion.png|900px]]

===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.119:8080/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-25T11:01:54Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
* Increased the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

* '''Problem 5''': In views/suggestion/show.html.erb and views/suggestion/student_view.html.erb, there is a DRY violation which needs to be fixed.

* '''Solution''': Merge views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.

* '''Problem 6''': Increase the test coverage of the controller and functionality by adding more tests.

* '''Solution''': Increase the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

<pre>
it 'adds a participant is not successfull' do
allow_any_instance_of(SuggestionComment).to receive(:save).and_return(false)
request_params = { id: 1, suggestion_comment: { vote: 'Y', comments: 'comments' } }
user_session = { user: instructor }
get :add_comment, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'There was an error in adding your comment.'
end
</pre>

<pre>
it 'rejecting a suggestion is not successfull' do
allow_any_instance_of(Suggestion).to receive(:update_attribute).and_return(false)
request_params = { id: 1, reject_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when rejecting the suggestion.'
end
</pre>

<pre>
it 'accept a suggestion is not successfull' do
allow(SignUpTopic).to receive(:new_topic_from_suggestion).and_return('failed')
allow_any_instance_of(SuggestionController).to receive(:notification).and_return(true)
request_params = { id: 1, approve_suggestion: true }
user_session = { user: instructor }
get :submit, params: request_params, session: user_session, xhr: true
expect(flash[:error]).to eq 'An error occurred when approving the suggestion.'
end
</pre>
===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.119:8080/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-25T10:56:44Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.
* Increased the test coverage of the suggestion controller by adding 3 more tests: 'adds a participant is not successfull', 'rejecting a suggestion is not successfull', 'accept a suggestion is not successfull'

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.119:8080/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-24T22:03:37Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.119:8080/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-24T22:03:21Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

===Testing from UI===
Here is the VCL link:

<pre>
http://152.7.176.62:8081/
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-24T22:02:32Z

Vrcholay:

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Added more comments to the controller, briefly explain the functionality of custom methods.
* Refactored the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
* Renamed methods approve and approve_suggestion to correctly reflect their functionalities.
* Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Renamed the method to notify_suggestion_approval.
* Merged views/suggestion/show.html.erb and views/suggestion/student_view.html.erb into one file to fix the DRY problem.

===Problems and Solutions===
* '''Problem 1''': Add more comments to the controller, briefly explain the functionality of custom methods.
::For example for notification method
<pre>
def notification
"""
method content
"""
end
</pre>
* '''Solution''': The approach we have taken is to add comment lines to all methods present in the suggestion_controller.rb file.
<pre>
#will provie notification/email based on the suggestion being approved or not
#will create and assign team if user is not in any team
def notification
"""
method content
"""
end
</pre>
* '''Problem 2''': Refactor the notification method to make it simpler by simplifying the control logic and reducing the function's complexity.
<pre>
def notification
if @suggestion.signup_preference == 'Y'
if @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
else
if @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
</pre>

* '''Solution''': Remove the nested if-else structure and fit all the functionalities into one if-elsif-else-end structure
<pre>
def notification
if @suggestion.signup_preference == 'Y' and @team_id.nil?
new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
new_team.create_new_team(@user_id, @signuptopic)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and @topic_id.nil?
# clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
elsif @suggestion.signup_preference == 'Y' and !@team_id.nil? and !@topic_id.nil?
@signuptopic.private_to = @user_id
@signuptopic.save
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
else
# if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
Mailer.notify_suggestion_approval(@used_id, @team_id, @suggestion.title)
end
end
</pre>

* '''Problem 3''': Rename methods approve and approve_suggestion to correctly reflect their functionalities.
<pre>
def approve
"""
method content
"""
end

def approve_suggestion
"""
method content
"""
end
</pre>
* '''Solution''': Renamed approve method to approve_suggestion and approve_suggestion to approve_suggestion_and_notify
<pre>
def approve_suggestion
"""
method content
"""
end

def approve_suggestion_and_notify
"""
method content
"""
end
</pre>

* '''Problem 4''': Move the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file and change its name.
<pre>
def send_email
proposer = User.find_by(id: @user_id)
if proposer
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>
* '''Solution''': Moved the send_email method from suggestion_controller to Mailer class in app/Mailer/mailer.rb file. Also changed the name of the method to notify_suggestion_approval.
<pre>
def notify_suggestion_approval(used_id, team_id, suggestion_title)
proposer = User.find_by(id: user_id)
if proposer
teams_users = TeamsUser.where(team_id: team_id)
cc_mail_list = []
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
suggested_topic_approved_message(
to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{suggestion_title}' has been approved",
body: {
approved_topic_name: suggestion_title,
proposer: proposer.name
}
).deliver_now!
end
end
</pre>

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password (Use this account for Testing)
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

1. After logging in as '''Instructor''', click "Manage instructor content" -> "Assignment":
[[File:E2121_S1.png|700px]]

2. Select the "Assignment" tab, in this example we search "textbook" as figure shows.
[[File:E2121_S2.png|700px]]

3. Choose a item and click "view suggestion" icon. In this example we use "Wiki textbook 2":
[[File:E2121_S3.png|700px]]

4. Click the "view" link in which tuple the Status is "Initiated":
[[File:E2121_S4.png|700px]]

5. Now we can see the Suggestion page with "Approve suggestion" and "Reject suggestion" butttons:
[[File:E2121_S5.png|700px]]

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/vyasrc/expertiza/tree/refactor_suggestion_controller GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://152.7.176.119:8080/ VCL link (will expire on Nov 24th 2022)]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-24T19:43:20Z

Vrcholay:

CSC/ECE 517 Fall 2022 - E2250. Refactor suggestion controller.rb

2022-10-24T19:33:17Z

Vrcholay: