Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

E1907 Refactor and improve response_controller.rb assignment on expertiza #1383

Closed

Conversation

Anusha-Godavarthi
Copy link

Project name : E1907 Refactor and improve response_controller.rb assignment on expertiza

  • Introduced temporary explaining variables in the redirect method
  • @Anusha-Godavarthi suggested and made the change. @mblewallenncsu and @akhilpabba reviewed it.

@expertiza-bot
Copy link
Contributor

expertiza-bot commented Mar 23, 2019

1 Message
💬 Thanks for the pull request, and welcome! 🎉 The Expertiza team is excited to review your changes, and you should hear from us soon.

This repository is being automatically checked for code quality issues using Code Climate.
You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a pull request is considered ready to review.

If you have any questions, please send email to expertiza-support@lists.ncsu.edu.

✅ Yay.
There are code changes, but no corresponding tests.
Please include tests if this PR introduces any modifications in behavior.

Generated by expertiza-bot

@coveralls
Copy link

Coverage Status

Coverage decreased (-17.5%) to 33.509% when pulling 74acc85 on Anusha-Godavarthi:anusha96commits into d361591 on expertiza:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-17.5%) to 33.509% when pulling 74acc85 on Anusha-Godavarthi:anusha96commits into d361591 on expertiza:master.

@coveralls
Copy link

coveralls commented Mar 23, 2019

Coverage Status

Coverage decreased (-0.6%) to 50.394% when pulling e8599db on Anusha-Godavarthi:anusha96commits into d361591 on expertiza:master.

@expertiza-travisci-bot
Copy link

expertiza-travisci-bot commented Mar 23, 2019

Hey @Anusha-Godavarthi,
Your changes look good to me! 🎉

@codeclimate
Copy link

codeclimate bot commented Mar 23, 2019

Code Climate has analyzed commit e8599db and detected 8 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 8

The test coverage on the diff in this pull request is 28.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 28.9%.

View more on Code Climate.

@@ -135,7 +136,40 @@ def model_name
super
end

# This method should be moved to survey_deployment_contoller.rb
def pending_surveys
Copy link

Choose a reason for hiding this comment

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

Method pending_surveys has a Cognitive Complexity of 22 (exceeds 5 allowed). Consider refactoring.

@@ -135,7 +136,40 @@ def model_name
super
end

# This method should be moved to survey_deployment_contoller.rb
def pending_surveys
Copy link

Choose a reason for hiding this comment

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

Method pending_surveys has 27 lines of code (exceeds 25 allowed). Consider refactoring.

end
end
end

Copy link

Choose a reason for hiding this comment

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

Trailing whitespace detected.

@@ -56,6 +56,7 @@ def param_test
params.require(:survey_deployment).permit(:questionnaire_id, :start_date, :end_date, :parent_id)
end


Copy link

Choose a reason for hiding this comment

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

Extra blank line detected.

next unless survey_deployment && Time.now > survey_deployment.start_date && Time.now < survey_deployment.end_date
@surveys <<
[
'survey' => Questionnaire.find(survey_deployment.questionnaire_id),
Copy link

Choose a reason for hiding this comment

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

Use 2 spaces for indentation in an array, relative to the start of the line where the left square bracket is.

@@ -135,7 +136,40 @@ def model_name
super
end

# This method should be moved to survey_deployment_contoller.rb
def pending_surveys
Copy link

Choose a reason for hiding this comment

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

Cyclomatic complexity for pending_surveys is too high. [8/6]

@@ -135,7 +136,40 @@ def model_name
super
end

# This method should be moved to survey_deployment_contoller.rb
def pending_surveys
Copy link

Choose a reason for hiding this comment

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

Assignment Branch Condition size for pending_surveys is too high. [29.14/15]

@@ -135,7 +136,40 @@ def model_name
super
end

# This method should be moved to survey_deployment_contoller.rb
def pending_surveys
Copy link

Choose a reason for hiding this comment

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

Perceived complexity for pending_surveys is too high. [8/7]

survey_deployments = survey_deployment_type.where(parent_id: p.parent_id)
next unless survey_deployments
survey_deployments.each do |survey_deployment|
next unless survey_deployment && Time.now > survey_deployment.start_date && Time.now < survey_deployment.end_date
Copy link

Choose a reason for hiding this comment

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

Do not use Time.now without zone. Use one of Time.zone.now, Time.current, Time.now.in_time_zone, Time.now.utc, Time.now.getlocal, Time.now.iso8601, Time.now.jisx0301, Time.now.rfc3339, Time.now.to_i, Time.now.to_f instead.

survey_deployments = survey_deployment_type.where(parent_id: p.parent_id)
next unless survey_deployments
survey_deployments.each do |survey_deployment|
next unless survey_deployment && Time.now > survey_deployment.start_date && Time.now < survey_deployment.end_date
Copy link

Choose a reason for hiding this comment

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

Do not use Time.now without zone. Use one of Time.zone.now, Time.current, Time.now.in_time_zone, Time.now.utc, Time.now.getlocal, Time.now.iso8601, Time.now.jisx0301, Time.now.rfc3339, Time.now.to_i, Time.now.to_f instead.

@@ -370,4 +337,4 @@ def init_answers(questions)
Answer.create(response_id: @response.id, question_id: q.id, answer: nil, comments: '') if a.nil?
end
end
end
end
Copy link

Choose a reason for hiding this comment

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

Final newline missing.

@@ -135,7 +134,39 @@ def model_name
super
end

# This method should be moved to survey_deployment_contoller.rb
def pending_surveys
Copy link

Choose a reason for hiding this comment

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

Method pending_surveys has 26 lines of code (exceeds 25 allowed). Consider refactoring.

'parent_id' => p.parent_id,
'participant_id' => p.id,
'global_survey_id' => survey_deployment.global_survey_id
]
Copy link

Choose a reason for hiding this comment

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

Closing array brace must be on the same line as the last array element when opening brace is on the same line as the first array element.

survey_deployments.each do |survey_deployment|
next unless survey_deployment && Time.current > survey_deployment.start_date && Time.current < survey_deployment.end_date
@surveys <<
[ 'survey' => Questionnaire.find(survey_deployment.questionnaire_id),
Copy link

Choose a reason for hiding this comment

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

Do not use space inside array brackets.

@@ -135,7 +134,38 @@ def model_name
super
end

# This method should be moved to survey_deployment_contoller.rb
def pending_surveys
Copy link

Choose a reason for hiding this comment

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

Assignment Branch Condition size for pending_surveys is too high. [31.06/15]

@@ -371,3 +338,4 @@ def init_answers(questions)
end
end
end

Copy link

Choose a reason for hiding this comment

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

1 trailing blank lines detected.

def edit_allowed?(map, user_id)
assignment = map.reviewer.assignment
# if it is a review response map, all the members of reviewee team should be able to view the reponse (can be done from heat map)
def view_allowed?(map, user_id)
Copy link

Choose a reason for hiding this comment

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

Method view_allowed? has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.

def edit_allowed?(map, user_id)
assignment = map.reviewer.assignment
# if it is a review response map, all the members of reviewee team should be able to view the reponse (can be done from heat map)
def view_allowed?(map, user_id)
Copy link

Choose a reason for hiding this comment

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

Cyclomatic complexity for view_allowed? is too high. [8/6]

def edit_allowed?(map, user_id)
assignment = map.reviewer.assignment
# if it is a review response map, all the members of reviewee team should be able to view the reponse (can be done from heat map)
def view_allowed?(map, user_id)
Copy link

Choose a reason for hiding this comment

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

Assignment Branch Condition size for view_allowed? is too high. [30.87/15]

assignment = map.reviewer.assignment
# if it is a review response map, all the members of reviewee team should be able to view the reponse (can be done from heat map)
def view_allowed?(map, user_id)
assignment = map.reviewer.assignment # if it is a review response map, all the members of reviewee team should be able to view the reponse (can be done from heat map)
Copy link

Choose a reason for hiding this comment

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

Line is too long. [170/160]

if map.is_a? ReviewResponseMap
reviewee_team = AssignmentTeam.find(map.reviewee_id)
return current_user_id?(user_id) || reviewee_team.user?(current_user) || current_user.role.name == 'Administrator' ||
(current_user.role.name == 'Instructor' and assignment.instructor_id == current_user.id) ||
(current_user.role.name == 'Teaching Assistant' and TaMapping.exists?(ta_id: current_user.id, course_id: assignment.course.id))
(current_user.role.name == 'Instructor' and assignment.instructor_id == current_user.id) || (current_user.role.name == 'Teaching Assistant' and TaMapping.exists?(ta_id: current_user.id, course_id: assignment.course.id))
Copy link

Choose a reason for hiding this comment

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

Line is too long. [227/160]

def edit_allowed?(map, user_id)
assignment = map.reviewer.assignment
# if it is a review response map, all the members of reviewee team should be able to view the reponse (can be done from heat map)
def view_allowed?(map, user_id)
Copy link

Choose a reason for hiding this comment

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

Perceived complexity for view_allowed? is too high. [9/7]

@Winbobob Winbobob force-pushed the master branch 2 times, most recently from 4580757 to 6c27394 Compare April 21, 2019 20:24
@Winbobob Winbobob changed the title Introduced temporary explaining variables in redirect method E1907 Refactor and improve response_controller.rb assignment on expertiza Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants