E1959.Intelligent copying of assignments without topics (Issue #1341) #1524

subhasekhar · 2019-10-17T16:54:07Z

we identified the code to make the changes and identified the pattern used.

expertiza-bot · 2019-10-17T16:56:40Z

	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. Also, please spend some time looking at the instructions at the top of your course project writeup. If you have any questions, please send email to expertiza-support@lists.ncsu.edu.

UUID		4 Warnings
e222	⚠️	Your pull request is more than 500 LoC. Please make sure you did not commit unnecessary changes, such as `schema.rb`, `node_modules`, `change logs`.
846d	⚠️	Your pull request has many duplicated commit messages. Please try to `squash` similar commits. And using meaningful commit messages later.
c51d	⚠️	You are modifying `Gemfile` or `Gemfile.lock`, please double check whether it is necessary. You are suppose to add a new gem only if you have a very good reason. Try to use existing gems instead. Please revert changes to `Gemfile.lock` made by the IDE.
04af	⚠️	One or more of your tests do not have expectations or you commented out some expectations. To avoid `shallow tests` – tests concentrating on irrelevant, unlikely-to-fail conditions – please write at least one expectation for each test and do not comment out expectations.
d699	✅	~~You changed YAML (`.yml`) or example (`.yml.example`) files; please double-check whether this is necessary.~~
ecaa	✅	There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior.
61df	✅	Your pull request is less than 50 LoC. If you are finished refactoring the code, please consider writing corresponding tests.

UUID		✅ Woo!
65de	✅	~~You are including debug code in your pull request, please remove it.~~

Generated by expertiza-bot

coveralls · 2019-10-17T17:04:53Z

Coverage increased (+8.2%) to 49.43% when pulling f6c084e on subhasekhar:master into 6a7042a on expertiza:master.

expertiza-travisci-bot · 2019-10-17T17:05:18Z

Hey @subhasekhar,
Your changes look good to me! 🎉

…he view

codeclimate · 2019-10-22T00:39:10Z

app/models/assignment_form.rb

-        questionnaire_weight: aq.questionnaire_weight,
-        used_in_round: aq.used_in_round,
-        dropdown: aq.dropdown
+          assignment_id: new_assign.id,


Indent the first parameter one step more than the start of the previous line.

codeclimate · 2019-10-22T00:39:11Z

app/models/assignment_form.rb

      )
    end
  end
 end
+


1 trailing blank lines detected.

codeclimate · 2019-10-22T00:39:11Z

app/models/assignment_form.rb

+      if copytopics == 'Y'
+        topics = SignUpTopic.where(assignment_id: old_assign.id)
+        topics.each do |topic|
+          SignUpTopic.create(topic_name: topic.topic_name, assignment_id: new_assign_id, max_choosers: topic.max_choosers, category: topic.category, topic_identifier: topic.topic_identifier, micropayment: topic.micropayment)


Line is too long. [224/160]

codeclimate · 2019-10-22T00:39:11Z

app/controllers/assignments_controller.rb

+    @assignment_id = params[:id]
+  end
+
+  def copy#Issue #1341 - Might require to modify below code


Missing space after #.

codeclimate · 2019-10-22T00:39:11Z

app/controllers/assignments_controller.rb

+    @assignment_id = params[:id]
+  end
+
+  def copy#Issue #1341 - Might require to modify below code


Put a space before an end-of-line comment.

codeclimate · 2019-10-22T00:39:11Z

app/controllers/assignments_controller.rb

@@ -380,4 +385,4 @@ def update_feedback_assignment_form_attributes
  def assignment_form_params
    params.require(:assignment_form).permit!
  end
-end
+end


Final newline missing.

codeclimate · 2019-10-22T00:39:11Z

app/controllers/assignments_controller.rb

+    @assignment_id = params[:id]
+  end
+
+  def copy#Issue #1341 - Might require to modify below code


Do not place comments on the same line as the def keyword.

codeclimate · 2019-10-22T00:39:14Z

Code Climate has analyzed commit f6c084e and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	1
Style	1

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

This pull request will bring the total coverage in the repository to 29.9% (-22.0% change).

View more on Code Climate.

…assignment_form.rb

codeclimate · 2019-11-03T00:22:41Z

app/models/assignment_form.rb

@@ -310,7 +309,7 @@ def enqueue_simicheck_task(due_date, simicheck_delay)
  end

  # Copies the inputted assignment into new one and returns the new assignment id
-  def self.copy(assignment_id, user)
+  def self.copy(assignment_id, copyoption, user)


Method copy has a Cognitive Complexity of 9 (exceeds 5 allowed). Consider refactoring.

codeclimate · 2019-11-03T00:22:41Z

app/models/assignment_form.rb

@@ -310,7 +309,7 @@ def enqueue_simicheck_task(due_date, simicheck_delay)
  end

  # Copies the inputted assignment into new one and returns the new assignment id
-  def self.copy(assignment_id, user)
+  def self.copy(assignment_id, copyoption, user)


Method copy has 33 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2019-11-03T00:22:41Z

app/models/assignment_form.rb

-        dropdown: aq.dropdown
-      )
+        AssignmentQuestionnaire.create(
+            assignment_id: new_assign.id,


Indent the first parameter one step more than the start of the previous line.

codeclimate · 2019-11-03T00:22:41Z

app/models/assignment_form.rb

-        used_in_round: aq.used_in_round,
-        dropdown: aq.dropdown
-      )
+        AssignmentQuestionnaire.create(


Use 2 (not 4) spaces for indentation.

codeclimate · 2019-11-03T00:22:41Z

app/models/assignment_form.rb

+      if copyoption != 'copyWithoutTopics'
+        topics = SignUpTopic.where(assignment_id: old_assign.id)
+        topics.each do |topic|
+          newSignUpTopic = SignUpTopic.create(topic_name: topic.topic_name, assignment_id: new_assign_id, max_choosers: topic.max_choosers, category: topic.category, topic_identifier: topic.topic_identifier, micropayment: topic.micropayment)


Line is too long. [241/160]

codeclimate · 2019-11-03T00:22:43Z

spec/models/assignment_form_spec.rb

+      expect(AssignmentForm.copy(1, 'copyWithTopicsTeams', build(:instructor))).to eq(2)
+    end
+
+


Tab detected.

codeclimate · 2019-11-03T00:22:43Z

spec/models/assignment_form_spec.rb

+	  allow_any_instance_of(AssignmentForm).to receive(:update_attribute).with(any_args).and_return('OK!')
+      expect(AssignmentForm.copy(1, 'copyWithTopicsTeams', build(:instructor))).to eq(2)
+    end
+


Trailing whitespace detected.

codeclimate · 2019-11-03T00:22:43Z

spec/models/assignment_form_spec.rb

+      expect(AssignmentForm.copy(1, 'copyWithTopicsTeams', build(:instructor))).to eq(2)
+    end
+
+


Trailing whitespace detected.

codeclimate · 2019-11-03T00:22:43Z

spec/controllers/assignments_controller_spec.rb

-        allow(assignment).to receive(:dup).and_return(new_assignment)
-        allow(new_assignment).to receive(:save).and_return(false)
+
+	context 'when new assignment is not able to be copied' do


Inconsistent indentation detected.

codeclimate · 2019-11-03T00:22:44Z

spec/controllers/assignments_controller_spec.rb

+	context 'when new assignment is not able to be copied' do
+      it 'should show an error and redirect to assignments#tree display page' do	  
+		allow(AssignmentForm).to receive(:copy).with("1", nil, instructor).and_return(false)
+        allow(Assignment).to receive(:find).with(2).and_return(false)


Inconsistent indentation detected.

Yashad

Please correct the code climate issues.

Yashad · 2019-11-03T23:57:55Z

Gemfile.lock

@@ -398,7 +398,7 @@ GEM
      rake (>= 0.8.7)
      thor (>= 0.18.1, < 2.0)
    rainbow (3.0.0)
-    rake (12.3.2)
+    rake (13.0.0)


Most of the Gems versions are changed, it might affect the other parts of the system. Could to try keeping the same version?

Yashad · 2019-11-03T23:58:38Z

app/controllers/assignments_controller.rb

-      (assignment.instructor_id == current_user.try(:id)) ||
-      TaMapping.exists?(ta_id: current_user.try(:id), course_id: assignment.course_id) ||
-      (assignment.course_id && Course.find(assignment.course_id).instructor_id == current_user.try(:id))
+          (assignment.instructor_id == current_user.try(:id)) ||


white spaces.

Yashad · 2019-11-04T00:03:40Z

app/views/assignments/checktopicscopy.html.erb

+  <%= label_tag(:copyoption_topics, "copy with topics") %><br>
+  <%= radio_button_tag(:copyoption, "copyWithTopicsTeams") %>
+  <%= label_tag(:copyoption_topics_teams, "copy with topics and teams") %><br>
+  <%= submit_tag("Create") %>


Shouldn't the submit button name be "Copy"

Yashad · 2019-11-04T00:14:15Z

app/assets/javascripts/tree_display.jsx

+            React.createElement(TabSystem),
+            document.getElementById("tree_display")
+        )
+    }

 })


Seems like you did not intend to change this file but introduced many whitespaces and newlines unknowingly. Please correct it. (remove this file from commit if it does not have any changes relevant to your project or correct its code climate )

Yashad · 2019-11-04T00:16:42Z

config/routes.rb

@@ -143,7 +144,7 @@
    end
  end

-resources :institution, except: [:destroy] do
+  resources :institution, except: [:destroy] do


white space

Yashad · 2019-11-04T00:17:15Z

spec/controllers/assignments_controller_spec.rb

-        allow(assignment).to receive(:dup).and_return(new_assignment)
-        allow(new_assignment).to receive(:save).and_return(false)
+
+	context 'when new assignment is not able to be copied' do


white spaces

Yashad · 2019-11-04T00:21:05Z

Do you have any other pull request for expertiza beta? We would like these changes on beta.

codeclimate · 2019-11-05T04:18:46Z

app/models/assignment_form.rb

@@ -310,7 +309,7 @@ def enqueue_simicheck_task(due_date, simicheck_delay)
  end

  # Copies the inputted assignment into new one and returns the new assignment id
-  def self.copy(assignment_id, user)
+  def self.copy(assignment_id, copyoption, user)


Method copy has 40 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2021-03-07T21:12:27Z

app/models/assignment_form.rb

@@ -310,7 +309,7 @@ def enqueue_simicheck_task(due_date, simicheck_delay)
  end

  # Copies the inputted assignment into new one and returns the new assignment id
-  def self.copy(assignment_id, user)
+  def self.copy(assignment_id, copyoption, user)


Method copy has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.

Identified the code requiring modification

868ca44

VishnusRepo and others added 4 commits October 21, 2019 17:19

Additions for confirming copying topics

d6a924d

Fixed errors

8b34233

Topics created only when YES is selected and modified the format of t…

688977f

…he view

Added a flash message after the assignment is successfully copied

dc6ccea

codeclimate bot reviewed Oct 22, 2019

View reviewed changes

subhasekhar and others added 3 commits October 26, 2019 19:14

Modified the test case for assignment_form.rb

1d96b42

Modified the tests for the assignments_controller.rb

c01d689

Removed trailing spaces for rubocop

33f0632

rohanpillai20 force-pushed the master branch from 5d1e9c7 to 33f0632 Compare October 27, 2019 00:53

VishnusRepo and others added 6 commits October 26, 2019 21:05

Implementation of Issues 1 & 2

47a83a4

Minor changes in the checktopicscopy.html

84279f4

Modifications to test cases for assignment controller and model

c2bbf95

Changes made to pass codeclimate tests

9391b58

Modifications to assignment_form_spec and changes to pass codeclimate

fb1a589

Added testcases for controllers/assignments_controller.rb and models/…

431c1cc

…assignment_form.rb

codeclimate bot reviewed Nov 3, 2019

View reviewed changes

Yashad reviewed Nov 4, 2019

View reviewed changes

rohanpillai20 added 5 commits November 4, 2019 21:00

Changes recommended by code climate

a7f56ef

Changes in assignment_form based on code climate recommendation

f9f7648

CodeClimate changes in assignment_form.rb

5d3b52c

CodeClimate changes in assignment_form.rb

3eebcb9

CodeClimate tab changes in assignment_form.rb

cfc54c3

codeclimate bot reviewed Nov 5, 2019

View reviewed changes

Saurabh110 mentioned this pull request Jun 7, 2020

Intelligent copying of assignments with topics #1341

Closed

Saurabh110 mentioned this pull request Jun 24, 2020

There needs to be a way to remove a lot of topics quickly #1409

Closed

Merge branch 'master' into master

f6c084e

codeclimate bot reviewed Mar 7, 2021

View reviewed changes

Saurabh110 merged commit f6c084e into expertiza:master Mar 7, 2021

Winbobob added this to the CSC517 2019 Fall OSS Projects milestone Jun 18, 2021

		expect(AssignmentForm.copy(1, 'copyWithTopicsTeams', build(:instructor))).to eq(2)
		end

E1959.Intelligent copying of assignments without topics (Issue #1341) #1524

E1959.Intelligent copying of assignments without topics (Issue #1341) #1524

Conversation

subhasekhar commented Oct 17, 2019

expertiza-bot commented Oct 17, 2019 • edited

coveralls commented Oct 17, 2019 • edited

expertiza-travisci-bot commented Oct 17, 2019 • edited

codeclimate bot Oct 22, 2019

Choose a reason for hiding this comment

codeclimate bot Oct 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot Oct 22, 2019

Choose a reason for hiding this comment

codeclimate bot Oct 22, 2019

Choose a reason for hiding this comment

codeclimate bot Oct 22, 2019

Choose a reason for hiding this comment

codeclimate bot Oct 22, 2019

Choose a reason for hiding this comment

codeclimate bot Oct 22, 2019

Choose a reason for hiding this comment

codeclimate bot commented Oct 22, 2019 • edited

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot Nov 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yashad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yashad commented Nov 4, 2019

codeclimate bot Nov 5, 2019

Choose a reason for hiding this comment

codeclimate bot Mar 7, 2021

Choose a reason for hiding this comment

expertiza-bot commented Oct 17, 2019 •

edited

coveralls commented Oct 17, 2019 •

edited

expertiza-travisci-bot commented Oct 17, 2019 •

edited

codeclimate bot commented Oct 22, 2019 •

edited