|
|
Line 520: |
Line 520: |
| </pre> | | </pre> |
| } | | } |
|
| |
| ==Action Allowed==
| |
| Action allowed is called before every method call to confirm whether or not the currently logged in user has the required privileges to complete the requested action.
| |
| To test the functionality of this method one has to attempt to complete an action that they have privileges to complete, and confirm that they're allowed to complete it. One also has to attempt to complete an action that they don't have the privileges to complete, and confirm that they're not allowed to complete it.
| |
|
| |
| ===Allowed===
| |
| Log into the application with the user having a student's role (<b>User Id:</b> user5072, <b>Password:</b> password)
| |
|
| |
| # Click on Assignments, and you should see the following:
| |
|
| |
| [[File:actionallowedPass.png]]
| |
|
| |
| ===Not Allowed===
| |
| Log into the application with the user having an instructor's role (<b>User Id:</b> user6, <b>Password:</b> password)
| |
|
| |
| # Click on Assignments, and you should see the following:
| |
|
| |
| [[File:actionallowedFail.png]]
| |
|
| |
|
| == get_scores == | | == get_scores == |
Revision as of 03:22, 30 October 2015
E1570. Refactoring TreeDisplayController
This page provides a description of the Expertiza based OSS project. This project aimed at refactoring the TreeDisplayController.
Introduction to Expertiza
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). The Expertiza project is supported by the National Science Foundation.<ref>Expertiza</ref><ref>Wiki</ref>
Problem Statement
Classes involved:
tree_display_controller.rb
What they do:
This controller deals with displaying “trees” of objects to the instructor. When an instructor logs in, (s)he is greeted with a homepage that lists questionnaires (rubrics, quizzes, surveys), courses, and assignments. These objects are displayed in a “tree,” allowing the user to click on the top-level object and see the objects beneath it.
What's wrong with it:
- Many Duplicate Methods.
- Very Long Methods combining many Functionalities.
- Redundant methods not being used anywhere.
What needs to be done:
- Split get_children_node_ng and get_children_node_2_ng into smaller methods and give reasonable names to them and make sure implement common code in a single method.
Changes Made
TreeDisplayController
Method Name
|
Changes Made
|
Reason For Change
|
Put ur txt here
|
put ur text here
|
get_children_node_ng
|
Moved part of the code from these methods to newly created methods populate_rows,display_row,page_render methods.
|
These Methods had many functionalities being implemented which made the method look lengthy and moreover these methods were performing almost similar functions which could be grouped together and reduce the overall code length.
|
get_children_node_2_ng
|
-------
|
Put ur txt here
|
put ur text here
|
Re-factored Code
Refactoring get_children_node_ng method
Before Changes
|
After Changes
|
def get_children_node_ng
childNodes = {}
if params[:reactParams][:child_nodes].is_a? String
childNodes = JSON.parse(params[:reactParams][:child_nodes])
else
childNodes = params[:reactParams][:child_nodes]
end
tmpRes = {}
res = {}
for node in childNodes
fnode = eval(params[:reactParams][:nodeType]).new
for a in node
fnode[a[0]] = a[1]
end
# fnode is the parent node
# ch_nodes are childrens
ch_nodes = fnode.get_children(nil, nil, session[:user].id, nil, nil)
tmpRes[fnode.get_name] = ch_nodes
# cnode = fnode.get_children("created_at", "desc", 2, nil, nil)
end
for nodeType in tmpRes.keys
res[nodeType] = Array.new
for node in tmpRes[nodeType]
tmpObject = {}
tmpObject["nodeinfo"] = node
tmpObject["name"] = node.get_name
tmpObject["type"] = node.type
if nodeType == 'Courses' || nodeType == "Assignments"
tmpObject["directory"] = node.get_directory
tmpObject["creation_date"] = node.get_creation_date
tmpObject["updated_date"] = node.get_modified_date
tmpObject["private"] = node.get_private
instructor_id = node.get_instructor_id
tmpObject["instructor_id"] = instructor_id
unless (instructor_id.nil?)
tmpObject["instructor"] = User.find(instructor_id).name
else
tmpObject["instructor"] = nil
end
tmpObject["is_available"] = is_available(session[:user], instructor_id) || (session[:user].role_id == 6 &&
Ta.get_my_instructors(session[:user].id).include?(instructor_id) && ta_for_current_course?(node))
if nodeType == "Assignments"
tmpObject["course_id"] = node.get_course_id
tmpObject["max_team_size"] = node.get_max_team_size
tmpObject["is_intelligent"] = node.get_is_intelligent
tmpObject["require_quiz"] = node.get_require_quiz
tmpObject["allow_suggestions"] = node.get_allow_suggestions
tmpObject["has_topic"] = SignUpTopic.where(['assignment_id = ?', node.node_object_id]).first ? true : false
end
end
res[nodeType] << tmpObject
end
end
respond_to do |format|
format.html {render json: res}
end
end
|
def get_children_node_ng
childNodes = {}
if params[:reactParams][:child_nodes].is_a? String
childNodes = JSON.parse(params[:reactParams][:child_nodes])
else
childNodes = params[:reactParams][:child_nodes]
end
ch_nodes = {}
for node in childNodes
# Declaring Foldernode Object as New
fnode = eval(params[:reactParams][:nodeType]).new
for a in node
fnode[a[0]] = a[1]
end
# fnode is the parent node
# ch_nodes are childrens
ch_nodes[fnode.get_name] = fnode.get_children(nil, nil, session[:user].id, nil, nil)
# cnode = fnode.get_children("created_at", "desc", 2, nil, nil)
end
call_function ="get_children_node_ng"
#Render JSON of the child nodes
populate_rows(ch_nodes,call_function)
end
|
Refactoring get_children_node_2_ng method
Before Changes
|
After Changes
|
def get_children_node_2_ng
childNodes = {}
if params[:reactParams2][:child_nodes].is_a? String
childNodes = JSON.parse(params[:reactParams2][:child_nodes])
else
childNodes = params[:reactParams2][:child_nodes]
end
tmpRes = {}
res = []
fnode = eval(params[:reactParams2][:nodeType]).new
childNodes.each do |key, value|
fnode[key] = value
end
ch_nodes = fnode.get_children(nil, nil, session[:user].id, nil, nil)
tmpRes = ch_nodes
if tmpRes
for child in tmpRes
nodeType = child.type
res2 = {}
res2["nodeinfo"] = child
res2["name"] = child.get_name
res2["key"] = params[:reactParams2][:key]
res2["type"] = nodeType
res2["private"] = child.get_private
res2["creation_date"] = child.get_creation_date
res2["updated_date"] = child.get_modified_date
if nodeType == 'CourseNode' || nodeType == "AssignmentNode"
res2["directory"] = child.get_directory
instructor_id = child.get_instructor_id
res2["instructor_id"] = instructor_id
unless (instructor_id.nil?)
res2["instructor"] = User.find(instructor_id).name
else
res2["instructor"] = nil
end
res2["is_available"] = is_available(session[:user], instructor_id) || (session[:user].role_id == 6 &&
Ta.get_my_instructors(session[:user].id).include?(instructor_id) && ta_for_current_course?(child))
if nodeType == "AssignmentNode"
res2["course_id"] = child.get_course_id
res2["max_team_size"] = child.get_max_team_size
res2["is_intelligent"] = child.get_is_intelligent
res2["require_quiz"] = child.get_require_quiz
res2["allow_suggestions"] = child.get_allow_suggestions
res2["has_topic"] = SignUpTopic.where(['assignment_id = ?', child.node_object_id]).first ? true : false
end
end
res << res2
end
end
respond_to do |format|
format.html {render json: res}
end
end
|
def get_children_node_2_ng
# Second Level of Tree Display with Children Nodes
childNodes = {}
if params[:reactParams2][:child_nodes].is_a? String
childNodes = JSON.parse(params[:reactParams2][:child_nodes])
else
childNodes = params[:reactParams2][:child_nodes]
end
res = []
fnode = eval(params[:reactParams2][:nodeType]).new
childNodes.each do |key, value|
fnode[key] = value
end
ch_nodes = fnode.get_children(nil, nil, session[:user].id, nil, nil)
call_function = "get_children_node_2_ng"
populate_rows(ch_nodes, call_function)
end
|
Common Methods Added to get_children_node_ng and get_children_node_2_ng controllers
populate_rows
This is a new Common Method for both get_children_node_ng and get_children_node_2_ng that does the functionality of rendering a page with data from all nodes by calling another method populate_1_row(). There is another Method call that happens in this method which as a whole renders the List View.
populate_rows()
|
def populate_rows(list,call_function) #render page with data for all nodes in list
if call_function == "get_children_node_ng"
tmpRes ={}
tmpRes = list
res = {}
for nodeType in tmpRes.keys
# declaring a new array
res[nodeType] = Array.new
for node in tmpRes[nodeType]
res[nodeType] << populate_1_row(node)
end
end
else
tmpRes = list
res = []
if tmpRes
for child in tmpRes
res2 = {}
res2 = populate_1_row(child)
res2["key"] = params[:reactParams2][:key]
res << res2
end
end
end
page_render(res)
end
}
populate_1_row
This is a new Method that is called from Method populate_row() and fetches all the child node names from respective Controller actions and then populates the tmpObject.
populate_1_row()
|
def populate_1_row(node) #return JSON for 1 Node
tmpObject = {}
tmpObject["nodeinfo"] = node
# all the child nodes names got and put in tmpObject from respective controller actions
tmpObject["name"] = node.get_name
tmpObject["type"] = node.type
if node.type == 'CourseNode' || node.type == "AssignmentNode"
tmpObject["directory"] = node.get_directory
tmpObject["creation_date"] = node.get_creation_date
tmpObject["updated_date"] = node.get_modified_date
tmpObject["private"] = node.get_private
instructor_id = node.get_instructor_id
tmpObject["instructor_id"] = instructor_id
unless (instructor_id.nil?)
tmpObject["instructor"] = User.find(instructor_id).name
else
tmpObject["instructor"] = nil
end
tmpObject["is_available"] = is_available(session[:user], instructor_id) || (session[:user].role_id == 6 && Ta.get_my_instructors(session[:user].id).include?(instructor_id) && ta_for_current_course?(node))
if node.type == "AssignmentNode"
tmpObject["course_id"] = node.get_course_id
tmpObject["max_team_size"] = node.get_max_team_size
tmpObject["is_intelligent"] = node.get_is_intelligent
tmpObject["require_quiz"] = node.get_require_quiz
tmpObject["allow_suggestions"] = node.get_allow_suggestions
tmpObject["has_topic"] = SignUpTopic.where(['assignment_id = ?', node.node_object_id]).first ? true : false
end
end
tmpObject
end
}
page_render
This is also a common Method which integrates the rendering functionality of get_children_node_ng and get_children_node_2_ng making sure both levels are properly populated in the view.
page_render()
|
def page_render(list) # Common Render Functionality for get_children_node_ng and get_children_node_2_ng methods
respond_to do |format|
format.html {render json: list}
end
end
}
Method Removal
There was a Method which dint have any Routes defined in the Routes.rb file and was not called in any of the Controller . After Careful analysis we descided to remove the method from the tree Display Controller.
filter
filter
|
def filter
search = params[:filter_string]
filter_node = params[:filternode]
qid = 'filter+'
if filter_node == 'QAN'
assignment = Assignment.find_by_name(search)
if assignment
assignment_questionnaires = AssignmentQuestionnaire.where(assignment_id: assignment.id)
if assignment_questionnaires
assignment_questionnaires.each { |q| qid << "#{q.questionnaire_id.to_s}+" }
session[:root] = 1
end
end
elsif filter_node == 'ACN'
session[:root] = 2
qid << search
end
return qid
end
}
Removing duplicate methods
Anush For u to write here...
Before Changes
|
After Changes
|
def new_feedback
review = Response.find(params[:id])
if review
reviewer = AssignmentParticipant.where(user_id: session[:user].id, parent_id: review.map.assignment.id).first
map = FeedbackResponseMap.where(reviewed_object_id: review.id, reviewer_id: reviewer.id).first
if map.nil?
map = FeedbackResponseMap.create(:reviewed_object_id => review.id, :reviewer_id => reviewer.id,
:reviewee_id => review.map.reviewer.id)
end
redirect_to :action => 'new', :id => map.map_id, :return => "feedback"
else
redirect_to :back
end
end
def new_feedback
review = Response.find(params[:id])
if review
reviewer = AssignmentParticipant.where(user_id: session[:user].id, parent_id: review.map.assignment.id).first
map = FeedbackResponseMap.where(reviewed_object_id: review.id, reviewer_id: reviewer.id).first
if map.nil?
#if no feedback exists by dat user den only create for dat particular response/review
map = FeedbackResponseMap.create(:reviewed_object_id => review.id, :reviewer_id => reviewer.id,
:reviewee_id => review.map.reviewer.id)
end
redirect_to :action => 'new', :id => map.id, :return => "feedback"
else
redirect_to :back
end
end
|
def new_feedback
review = Response.find(params[:id])
if review
reviewer = AssignmentParticipant.where(user_id: session[:user].id, parent_id: review.map.assignment.id).first
map = FeedbackResponseMap.where(reviewed_object_id: review.id, reviewer_id: reviewer.id).first
if map.nil?
#if no feedback exists by dat user den only create for dat particular response/review
map = FeedbackResponseMap.create(:reviewed_object_id => review.id, :reviewer_id => reviewer.id,
:reviewee_id => review.map.reviewer.id)
end
redirect_to :action => 'new', :id => map.id, :return => "feedback"
else
redirect_to :back
end
end
|
def view
@response = Response.find(params[:id])
return if redirect_when_disallowed(@response)
@map = @response.map
get_content
@review_scores = Array.new
@question_type = Array.new
@questions.each do |question|
@review_scores << Score.where(response_id: @map.response_id, question_id: question.id).first
@question_type << QuestionType.find_by_question_id(question.id)
end
end
def view
@response = Response.find(params[:id])
return if action_allowed?(@response)
@map = @response.map
get_content
get_scores(@response, @questions)
end
|
def view
@response = Response.find(params[:id])
return if action_allowed?(@response)
@map = @response.map
get_content
get_scores(@response, @questions)
end
|
Rspec Test
We have written Rspec tests for the tree_display_controller and have run the same with success.
page_render()
|
require 'rails_helper'
describe TreeDisplayController do
describe "#get_children_node_ng" do
end
describe "#list" do
it "should not redirect to student_task controller if current user is an instructor" do
allow(session[:user]).to receive("student?").and_return(false)
post "list"
response.should_not redirect_to(list_student_task_index_path)
end
it "should redirect to student_task controller if current user is a student" do
allow(session[:user]).to receive("student?").and_return(true)
post "list"
response.should redirect_to(list_student_task_index_path)
end
end
describe "#ta_for_current_mappings?" do
it "should return true if current user is a TA for current course" do
end
end
describe "#populate_rows" do
let(:dbl) { double }
before { expect(dbl).to receive(:populate_rows).with(Hash, String)}
it "passes when the arguments match" do
dbl.populate_rows({},"")
end
end
describe "#populate_1_row" do
let(:dbl) { double }
before { expect(dbl).to receive(:populate_1_row).with(Node) }
it "passes when the arguments match" do
dbl.populate_1_row(Node.new)
end
end
end
}
get_scores
Get scores is called anytime the scores for reviews are shown. Given that, the way to test its correct functionality is to navigate to a completed review and make sure the scores are shown and are correct.
Log into the application with the user having a student's role (User Id: user5072, Password: password)
- Click on Assignments
- Click on Writing assignment 1b, Spring 2013
- Click on Others' Work
- Click on Review done at --2013-03-01 23:57:55 UTC
Successful loading of this page confirms the get_scores method.
rereview
Rereview manages the versioning of multiple reviews for the same assignment. To test the correct functionality of review, one should attempt to update a review (i.e. 'rereview').
Log into the application with the user having a student's role (User Id: user5072, Password: password)
- Click on Assignments
- Click on Writing assignment 1b, Spring 2013
- Click on Others' Work
- Click on Update (beside Metaprogramming in dynamically typed languages)
Successful loading of this page confirms the rereview method.
References
<references></references>
|
|
|
|
|