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

E1829. OSS project Duke Blue: Fix import glitches [use new issue list] #1267

Closed
wants to merge 3 commits into from

Conversation

ppvasude
Copy link

We have added a new option in the dropdown in app/views/import_file/start.html.erb for renaming exisiting team name if new team has same name as existing team. We have made changes team.rb to rename existing team using the generate name function already present. We then save these updated changes of the existing team.

@expertiza-bot
Copy link
Contributor

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.

1 Warning
⚠️ There are code changes, but no corresponding tests.
Please include tests if this PR introduces any modifications in behavior.
1 Error
💥 You are including debug code in your pull request, please remove it.

Generated by expertiza-bot

@TravisBuddy
Copy link

TravisBuddy commented Oct 30, 2018

Hey @ppvasude,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 77f607a0-dc5a-11e8-a919-2b33139089d4

@coveralls
Copy link

coveralls commented Oct 30, 2018

Pull Request Test Coverage Report for Build 5968

  • 5 of 13 (38.46%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 47.635%

Changes Missing Coverage Covered Lines Changed/Added Lines %
app/models/team.rb 5 13 38.46%
Totals Coverage Status
Change from base Build 5965: -0.01%
Covered Lines: 6355
Relevant Lines: 13341

💛 - Coveralls

team.update(name:self.generate_team_name(Assignment.find(id).name)) # update the old team name
end
#team.update(name:self.generate_team_name(Assignment.find(id).name))
return name # send the new team name
Copy link

Choose a reason for hiding this comment

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

Avoid too many return statements within this method.

if teamtype.is_a?(CourseTeam)
team.update(name:self.generate_team_name(Course.find(id).name)) # update the old team name
elsif teamtype.is_a?(AssignmentTeam)
team.update(name:self.generate_team_name(Assignment.find(id).name)) # update the old team name
Copy link

Choose a reason for hiding this comment

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

Space missing after colon.

puts name.to_s #team name



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.





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.

@@ -203,6 +204,13 @@ def self.import(row_hash, id, options, teamtype)

# Handle existence of the duplicate team
def self.handle_duplicate(team, name, id, handle_dups, teamtype)
puts "in duplicate"
puts team.to_s # team object
puts name.to_s #team name
Copy link

Choose a reason for hiding this comment

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

Missing space after #.

@@ -152,7 +152,7 @@ def self.generate_team_name(team_name_prefix)
counter = 1
loop do
team_name = team_name_prefix + "_Team#{counter}"
return team_name unless Team.find_by(name: team_name)
return team_name unless !Team.find_by(name: team_name).nil?
Copy link

Choose a reason for hiding this comment

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

Favor if over unless for negative conditions.

@@ -178,6 +178,7 @@ def import_team_members(starting_index = 0, row_hash)
def self.import(row_hash, id, options, teamtype)

raise ArgumentError, "Not enough fields on this line." if row_hash.empty? || (row_hash[:teammembers].length < 2 && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last")) || (row_hash[:teammembers].empty? && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"))
puts "hi, im in import"
Copy link

Choose a reason for hiding this comment

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

Do not write to stdout. Use Rails's logger if you want to log.

@@ -203,6 +204,13 @@ def self.import(row_hash, id, options, teamtype)

# Handle existence of the duplicate team
def self.handle_duplicate(team, name, id, handle_dups, teamtype)
puts "in duplicate"
Copy link

Choose a reason for hiding this comment

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

Do not write to stdout. Use Rails's logger if you want to log.

@@ -203,6 +204,13 @@ def self.import(row_hash, id, options, teamtype)

# Handle existence of the duplicate team
def self.handle_duplicate(team, name, id, handle_dups, teamtype)
puts "in duplicate"
puts team.to_s # team object
Copy link

Choose a reason for hiding this comment

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

Do not write to stdout. Use Rails's logger if you want to log.

@@ -203,6 +204,13 @@ def self.import(row_hash, id, options, teamtype)

# Handle existence of the duplicate team
def self.handle_duplicate(team, name, id, handle_dups, teamtype)
puts "in duplicate"
puts team.to_s # team object
puts name.to_s #team name
Copy link

Choose a reason for hiding this comment

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

Do not write to stdout. Use Rails's logger if you want to log.

@codeclimate
Copy link

codeclimate bot commented Oct 30, 2018

Code Climate has analyzed commit c4468b2 and detected 16 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3
Style 9
Bug Risk 4

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

This pull request will bring the total coverage in the repository to 47.6% (0.0% change).

View more on Code Climate.

@Winbobob Winbobob changed the title issue 329 changes - team.rb and start.html.erb E1829. OSS project Duke Blue: Fix import glitches [use new issue list] Nov 6, 2018
@Winbobob Winbobob added this to the CSC517 2018 Fall OSS Projects milestone Nov 6, 2018
@Winbobob Winbobob closed this Dec 13, 2018
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