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

E2300. Refactor E1858. Github metrics integration #2524

Open
wants to merge 66 commits into
base: main
Choose a base branch
from

Conversation

sukruthimodem
Copy link

To Do: Add wiki link

sukruthimodem and others added 15 commits March 27, 2023 13:39
Instead of controller: 'assignments', we can use the list_submissions_assignment_path helper method to generate the correct path for redirect_to.
The unless statement in require_instructor_privileges can be simplified to a one-liner.
I removed the only: [:action_allowed?] from require_instructor_privileges, since there is no action_allowed? method defined in the controller.
I removed the comments that were not providing any additional value.
Extracted some code to private methods to improve readability and organization.
Removed unnecessary instance variables and local variables.
Simplified the retrieve_github_data and create_github_metrics methods.
Removed unnecessary @total_* instance variables.
Removed the @check_statuses instance variable, which isn't used in this method.
The retrieve_github_data method has been refactored to use partition instead of select, simplifying the code.

In query_all_pull_requests, we've combined two lines of code to simplify the creation of the head_refs hash. We've also passed hyperlink_data to parse_pull_request_data instead of github_data to match the method's expected argument.

Finally, we've refactored extract_hyperlink_data to use slice and zip to create the hash, which is shorter and easier to read.
In pull_request_data, I replaced the while loop with a loop do construct and used break to exit the loop instead of modifying a flag variable. I also used the safe navigation operator (&.) to avoid NoMethodError exceptions when accessing nested hashes that may be nil.

In parse_pull_request_data, I used the safe navigation operator to avoid NoMethodError exceptions when accessing nested hashes that may be nil. I also added some nil checks to skip over commits or commit data that is missing or invalid. Finally, I simplified the date slicing logic by chaining the to_s and slice methods.

retrieve_repository_data now calls parse_hyperlink to extract the owner and repository names from the GitHub repository URL. It also uses a loop instead of a while to simplify the code and avoid unnecessary variables.
query_and_parse_repository_data now returns the page info hash instead of modifying it in place, to make it clearer and less error-prone.
parse_repository_data now uses dig to access nested hashes safely, avoiding errors if any of the expected keys are missing. It also simplifies the date string extraction by using a slice instead of a regex.

Used the dig method to access nested keys in the hash, which returns nil if any intermediate key is nil instead of raising an exception.
Removed unnecessary comments and made the existing ones more concise.
Used ternary operator to simplify the if statement.

Used dig instead of nested hash indexing with square brackets. This makes the code more concise and avoids NoMethodError exceptions when a hash key is missing.
Used return to exit early from count_github_authors_and_dates if the author is a collaborator. This avoids unnecessary code execution and improves performance.
Used Hash.new(0) to initialize the hash for each author's commits. This simplifies the code and avoids the need for if...else logic.
Used default to set the default value for each author's commits to 0. This avoids the need for if...else logic when incrementing the commit count.
Used to_h to convert the sorted array of commits back to a hash. This is more concise than building a new hash with Hash[].
Used dig to access nested hash keys in team_statistics. This avoids NoMethodError exceptions and makes the code more concise.
Used ternary operator to simplify
Used the find_by method instead of where and first for simplicity.
Used the safe navigation operator (&.) to avoid errors when trying to call a method on a nil object.
Used present? instead of nil? to check if a record exists.
Used update instead of assigning values and calling save separately for readability.
Used string interpolation to concatenate the @ncsu.edu suffix to the email prefix.
Removed unnecessary comments and code.
E2300. Refactor E1858. Github metrics integration
unless Rails.env.development?
match '*path' => 'content_pages#view', :via => %i[get post]
end
get ':controller(/:action(/:id))(.:format)'

Check failure

Code scanning / Brakeman

All public methods in controllers are available as actions in routes.rb. Error

All public methods in controllers are available as actions in routes.rb.
Gemfile.lock Fixed Show fixed Hide fixed
<% p team.submitted_files %>
<% if participant and !team.submitted_files.empty? %>
<% files = team.submitted_files %>
<%= display_directory_tree(participant, files, true).html_safe if files and files.length > 0 %>

Check notice

Code scanning / Brakeman

Unescaped model attribute. Note

Unescaped model attribute.
<% participant = participants.compact.first %>
<% if participant and !team.hyperlinks.empty? %>
<% team.hyperlinks.each do |link| %>
<a href="<%= link %>" target="_blank">- <%= link %></a><br/>

Check warning

Code scanning / CodeQL

Potentially unsafe external link Medium

External links without noopener/noreferrer are a potential security risk.
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

4 participants