-
Notifications
You must be signed in to change notification settings - Fork 101
Add a fix language function #452
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
base: main
Are you sure you want to change the base?
Changes from all commits
ab7e6e6
9b979f4
3a67f0b
d4a413f
ac85ed8
b5da309
955ad1f
e9201e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,8 +45,10 @@ def index | |
|
|
||
| # Process results to get sorted languages and editors | ||
| language_counts = results | ||
| .map { |r| [ r.language&.downcase, r.language_count ] } | ||
| .map { |r| [ r.language&.categorize_language, r.language_count ] } # fix the bug where langs can have both upper and lower case like JAVA and java found here (https://github.com/hackclub/hackatime/issues/402) | ||
| .reject { |lang, _| lang.nil? || lang.empty? } | ||
| .group_by { |lang, _| lang } | ||
| .transform_values { |pairs| pairs.sum { |_, count| count } } | ||
| .uniq | ||
| .sort_by { |_, count| -count } | ||
|
|
||
|
|
@@ -271,12 +273,10 @@ def dashboard | |
| .reverse.map(&:first) | ||
| .compact_blank | ||
| .map { |k| | ||
| if filter == :editor | ||
| ApplicationController.helpers.display_editor_name(k) | ||
| elsif filter == :operating_system | ||
| ApplicationController.helpers.display_os_name(k) | ||
| elsif filter == :language | ||
| ApplicationController.helpers.display_language_name(k) | ||
| if filter == :language | ||
| k.categorize_language | ||
| elsif %i[operating_system editor].include?(filter) | ||
| k.capitalize | ||
| else | ||
| k | ||
|
Comment on lines
+277
to
281
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change replaces the 🤖 Prompt for AI Agent Did we get this right? 👍 / 👎 to inform future reviews. |
||
| end | ||
|
|
@@ -289,6 +289,18 @@ def dashboard | |
| # search for both lowercase and capitalized versions | ||
| normalized_arr = filter_arr.flat_map { |v| [ v.downcase, v.capitalize ] }.uniq | ||
| filtered_heartbeats = filtered_heartbeats.where(filter => normalized_arr) | ||
| elsif filter == :language | ||
| # find the real name, not the pretty one cause some edditors are stupid and return stuff like JAVASCRIPT and javascript and i need to add stuff to make them both fit the lookup | ||
| raw_language_values = [] | ||
| current_user.heartbeats.distinct.pluck(filter).compact_blank.each do |raw_lang| | ||
| categorized = raw_lang.categorize_language | ||
| if filter_arr.include?(categorized) | ||
| raw_language_values << raw_lang | ||
| end | ||
| end | ||
| Rails.logger.debug "lang filter: selected=#{filter_arr}, raw_language_values=#{raw_language_values}" # Debug line | ||
|
|
||
|
Comment on lines
291
to
+302
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The language filtering logic (lines 291-302) is inefficient because it retrieves all raw language values from the database in-memory and then filters them in Ruby. This approach doesn't scale well and performs unnecessary database queries. Consider building a database query that filters on the categorized language values instead, or using a scope/helper method. The current implementation also lacks error handling if 🤖 Prompt for AI Agent Did we get this right? 👍 / 👎 to inform future reviews. |
||
| filtered_heartbeats = filtered_heartbeats.where(filter => raw_language_values) if raw_language_values.any? | ||
| else | ||
| filtered_heartbeats = filtered_heartbeats.where(filter => filter_arr) | ||
| end | ||
|
|
@@ -335,7 +347,12 @@ def dashboard | |
| .duration_seconds | ||
| .each_with_object({}) do |(raw_key, duration), agg| | ||
| key = raw_key.to_s.presence || "Unknown" | ||
| key = key.downcase if %i[editor operating_system].include?(filter) | ||
| # fix the bug where langs can have both upper and lower case like JAVA and java found here (https://github.com/hackclub/hackatime/issues/402) | ||
| if filter == :language | ||
| key = key.categorize_language unless key == "Unknown" | ||
| elsif %i[editor operating_system].include?(filter) | ||
| key = key.downcase | ||
| end | ||
| agg[key] = (agg[key] || 0) + duration | ||
|
Comment on lines
+354
to
356
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line 354 uses 🤖 Prompt for AI Agent Did we get this right? 👍 / 👎 to inform future reviews. |
||
| end | ||
|
|
||
|
Comment on lines
+351
to
358
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the pie chart data section, when normalizing language keys, the code uses 🤖 Prompt for AI Agent Did we get this right? 👍 / 👎 to inform future reviews. |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Calling
Hash#uniqonlanguage_countswill raise aNoMethodError, crashing the application.Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The
language_countscalculation attempts to call.uniqon a Hash object aftergroup_byandtransform_valueshave been applied. TheHash#uniqmethod does not exist in Ruby, leading to aNoMethodError. This error will occur every time a logged-in user with heartbeats visits the public-facing homepage, causing the application to crash.💡 Suggested Fix
Remove the
.uniqcall from line 52. Thegroup_byandtransform_valuesoperations already produce a deduplicated Hash with unique language keys and aggregated counts, making.uniqredundant and incorrect.🤖 Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID:
6043025