Skip to content

Conversation

@GrimbiXcode
Copy link
Owner

No description provided.

- Add sortable headers for id, name, and label columns.
- Implement pagination for the index action.
- Update table to display primary countries more effectively.
- Fix `has_many` association typo in `Currency` model.
…nation, and delete functionality

- Add sortable headers for columns in the countries table.
- Introduce sorting and pagination logic to the countries index action.
- Add delete button to country and currency forms.
- Update destroy actions in countries and currencies controllers for record deletion.
- Enhance UI layout for better usability.
"countries.#{@sort} #{@direction}"
end

@countries = scope.order(order_clause)

Check failure

Code scanning / CodeQL

SQL query built from user-controlled sources High

This SQL query depends on a
user-provided value
.
This SQL query depends on a
user-provided value
.

Copilot Autofix

AI 3 months ago

To fix the problem, replace the manual string interpolation of column names and directions for the order clause with the hash/symbol approach provided by Rails' ActiveRecord, which internally parameterizes and escapes the query. Specifically:

  • For core table columns (id, name): use order(countries: { @sort => @direction }).
  • For the joined currency column, use order('currencies.name' => @direction), but expressed with a hash if possible: order(currencies: { name: @direction }).
  • Only use the permitted fields (id, name, currency) and directions (asc, desc), and ensure both are symbols to avoid SQL injection.
  • No new imports or gems are required. All necessary methods are provided by ActiveRecord.
  • Only change code in the index method of app/controllers/admin/countries_controller.rb, specifically the code relating to order_clause and its use.

Suggested changeset 1
app/controllers/admin/countries_controller.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/controllers/admin/countries_controller.rb b/app/controllers/admin/countries_controller.rb
--- a/app/controllers/admin/countries_controller.rb
+++ b/app/controllers/admin/countries_controller.rb
@@ -6,13 +6,12 @@
 
     scope = Country.left_joins(:currency)
 
-    order_clause = if @sort == 'currency'
-                     "currencies.name #{@direction}"
-                   else
-                     "countries.#{@sort} #{@direction}"
-                   end
+    if @sort == 'currency'
+      @countries = scope.order(currencies: { name: @direction.to_sym })
+    else
+      @countries = scope.order(countries: { @sort.to_sym => @direction.to_sym })
+    end
 
-    @countries = scope.order(order_clause)
     @countries = @countries.page(params[:page]) if @countries.respond_to?(:page)
   end
 
EOF
@@ -6,13 +6,12 @@

scope = Country.left_joins(:currency)

order_clause = if @sort == 'currency'
"currencies.name #{@direction}"
else
"countries.#{@sort} #{@direction}"
end
if @sort == 'currency'
@countries = scope.order(currencies: { name: @direction.to_sym })
else
@countries = scope.order(countries: { @sort.to_sym => @direction.to_sym })
end

@countries = scope.order(order_clause)
@countries = @countries.page(params[:page]) if @countries.respond_to?(:page)
end

Copilot is powered by AI and may make mistakes. Always verify output.

scope = Currency.left_joins(:countries).distinct

@currencies = scope.order("currencies.#{@sort} #{@direction}")

Check failure

Code scanning / CodeQL

SQL query built from user-controlled sources High

This SQL query depends on a
user-provided value
.
This SQL query depends on a
user-provided value
.

Copilot Autofix

AI 3 months ago

To fix the problem, we should avoid directly interpolating user-influenced strings into the order() clause, even when using a whitelist, and instead use ActiveRecord's hash-style syntax, which automatically parameterizes column names and directions. Since ActiveRecord does not natively accept the hash syntax for column directions as variables, you can build the order clause as a hash mapping the column symbol to a symbol for direction, e.g. { id: :asc } or { name: :desc }, depending on the validated input.

We'll map the string values from @sort and @direction into symbols, and then supply the hash to .order():

@currencies = scope.order(@sort.to_sym => @direction.to_sym)

This approach uses only validated values, avoids SQL fragments, and is safe against injection. Edits are limited to the region in index where the query is built (line 9), and no new imports or methods are needed.


Suggested changeset 1
app/controllers/admin/currencies_controller.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/controllers/admin/currencies_controller.rb b/app/controllers/admin/currencies_controller.rb
--- a/app/controllers/admin/currencies_controller.rb
+++ b/app/controllers/admin/currencies_controller.rb
@@ -6,7 +6,7 @@
 
     scope = Currency.left_joins(:countries).distinct
 
-    @currencies = scope.order("currencies.#{@sort} #{@direction}")
+    @currencies = scope.order(@sort.to_sym => @direction.to_sym)
 
     @currencies = @currencies.page(params[:page]) if @currencies.respond_to?(:page)
   end
EOF
@@ -6,7 +6,7 @@

scope = Currency.left_joins(:countries).distinct

@currencies = scope.order("currencies.#{@sort} #{@direction}")
@currencies = scope.order(@sort.to_sym => @direction.to_sym)

@currencies = @currencies.page(params[:page]) if @currencies.respond_to?(:page)
end
Copilot is powered by AI and may make mistakes. Always verify output.
GrimbiXcode and others added 2 commits October 21, 2025 22:51
…n permissions

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
- Add missing currency references in countries fixtures
- Remove duplicate description field in products fixtures
- Fix selling_data fixture association mapping with explicit class mapping
- Add missing timestamps to selling_data fixtures
- Fix controller test URL helpers (home_url, admin_url)

All 30 tests now pass with 57 assertions, 0 failures, 0 errors
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.

2 participants