-
Notifications
You must be signed in to change notification settings - Fork 0
WIP: general database schema and core endpoints #2
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?
Conversation
…and add pagination
- 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
user-provided value
This SQL query depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
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): useorder(countries: { @sort => @direction }). - For the joined
currencycolumn, useorder('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
indexmethod ofapp/controllers/admin/countries_controller.rb, specifically the code relating toorder_clauseand its use.
-
Copy modified lines R9-R13
| @@ -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 | ||
|
|
|
|
||
| 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
user-provided value
This SQL query depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
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.
-
Copy modified line R9
| @@ -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 |
…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
No description provided.