Skip to content

Sorted tags alphabetically #57

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.

Sign up for GitHub

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
+22 −8
Open

Sorted tags alphabetically #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7b0ad6f
changed the app port
shrestha-bishal Aug 1, 2025
a429cfe
sorted tags alphabetically
shrestha-bishal Aug 1, 2025
35d3c04
sorted tags naturally and reverted the dev port back to 3000
shrestha-bishal Aug 1, 2025
13f543b
Linter noise
jjmata Aug 10, 2025
1cd4e60
added safeguard against case sensitive and nullability
shrestha-bishal Aug 11, 2025
3bef7fc
Improve Tag.sorted_naturally method for robust natural sorting:
shrestha-bishal Aug 11, 2025
32db336
Merge branch 'sort-tags-alphabetically' of https://github.com/shresth…
shrestha-bishal Aug 11, 2025
f34e770
Improved Tag natural sorting with database methods
shrestha-bishal Aug 11, 2025
4784e80
Fix trailing whitespace and add missing final newline in Tag model
shrestha-bishal Aug 11, 2025
8fae3e9
Sorted rule categories and value options alphabetically in dropdowns
shrestha-bishal Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions app/controllers/tags_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ class TagsController < ApplicationController
before_action :set_tag, only: %i[edit update destroy]

def index
@tags = Current.family.tags.alphabetically

@tags = Current.family.tags.sorted_naturally
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use the scope to keep it relational (no eager loading)
Switch to the DB scope so this remains chainable and avoids loading into Ruby.

-    @tags = Current.family.tags.sorted_naturally
+    @tags = Current.family.tags.sorted_naturally_db
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@tags = Current.family.tags.sorted_naturally
@tags = Current.family.tags.sorted_naturally_db
🤖 Prompt for AI Agents 
In app/controllers/tags_controller.rb around line 5, the current call loads Tag
records into Ruby (non-relational) via a method that eagerly materializes
results; replace it with a chainable ActiveRecord scope so the result stays an
ActiveRecord::Relation. Use the DB scope version of the natural sort by either
merging the Tag.sorted_naturally scope into Current.family.tags
(Current.family.tags.merge(Tag.sorted_naturally)) or by applying
Tag.sorted_naturally.where(family: Current.family), and ensure you do not call
any loading methods like to_a, load, or similar.

render layout: "settings"
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/rule/action_executor/set_transaction_category.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ def type
end

def options
family.categories.pluck(:name, :id)
family.categories.order(:name).pluck(:name, :id)
end

def execute(transaction_scope, value: nil, ignore_attribute_locks: false)
Expand Down
2 changes: 1 addition & 1 deletion app/models/rule/action_executor/set_transaction_tags.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ def type
end

def options
family.tags.pluck(:name, :id)
family.tags.order(:name).pluck(:name, :id)
end

def execute(transaction_scope, value: nil, ignore_attribute_locks: false)
Expand Down
4 changes: 2 additions & 2 deletions app/models/rule/registry/transaction_resource.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def condition_filters
Rule::ConditionFilter::TransactionName.new(rule),
Rule::ConditionFilter::TransactionAmount.new(rule),
Rule::ConditionFilter::TransactionMerchant.new(rule)
]
].sort_by { |filter| filter.label.downcase }
end

def action_executors
Expand All @@ -24,7 +24,7 @@ def action_executors
enabled_executors << Rule::ActionExecutor::AutoDetectMerchants.new(rule)
end

enabled_executors
enabled_executors.sort_by { |executor| executor.label.downcase }
end

private
Expand Down
15 changes: 15 additions & 0 deletions app/models/tag.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ class Tag < ApplicationRecord

scope :alphabetically, -> { order(:name) }

scope :sorted_naturally_db, -> {
order(
Arel.sql(
"REGEXP_REPLACE(name, '\\d+$', '') ASC, " \
"CAST(REGEXP_REPLACE(name, '\\D+', '') AS INTEGER) ASC"
)
)
}

COLORS = %w[#e99537 #4da568 #6471eb #db5a54 #df4e92 #c44fe9 #eb5429 #61c9ea #805dee #6ad28a]

UNCATEGORIZED_COLOR = "#737373"
Expand All @@ -23,4 +32,10 @@ def replace_and_destroy!(replacement)
destroy!
end
end

def self.sorted_naturally
all.to_a.sort_by do |tag|
tag.name.to_s.scan(/\d+|\D+/).map { |s| s =~ /\d+/ ? s.to_i : s.downcase }
end
end
Comment on lines +36 to +40
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Make natural sort chainable and consistent (avoid loading entire table and ambiguity between scope vs class method)

self.sorted_naturally returns an Array (loads all records) and is a class method, not a scope. Calling Current.family.tags.sorted_naturally relies on Rails’ method_missing scoping semantics and returns a Ruby array, which:

  • Breaks chainability (can’t paginate, further filter, etc.).
  • Loads all matching rows into memory.
  • Diverges from the DB-level variant.

Prefer a scope for use in relations and keep the Ruby method only as an explicit fallback.

Two options:

Option A (recommended): consolidate on DB scope and remove/rename the Ruby method.

- def self.sorted_naturally
-   all.to_a.sort_by do |tag|
-     tag.name.to_s.scan(/\d+|\D+/).map { |s| s =~ /\d+/ ? s.to_i : s.downcase }
-   end
- end
+ # Optional: keep a clearly named Ruby-only helper if ever needed explicitly
+ def self.sorted_naturally_ruby(collection = all)
+   collection.to_a.sort_by do |tag|
+     tag.name.to_s.scan(/\d+|\D+/).map { |s| s =~ /\d+/ ? s.to_i : s.downcase }
+   end
+ end

Option B: keep both, but add a scope alias for consistent call sites:

+ scope :sorted_naturally, -> { sorted_naturally_db }

If you’d like, I can follow through with a repo-wide rename to use sorted_naturally_db (or the alias) wherever relations are expected.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def self.sorted_naturally
all.to_a.sort_by do |tag|
tag.name.to_s.scan(/\d+|\D+/).map { |s| s =~ /\d+/ ? s.to_i : s.downcase }
end
end
# Optional: keep a clearly named Ruby-only helper if ever needed explicitly
def self.sorted_naturally_ruby(collection = all)
collection.to_a.sort_by do |tag|
tag.name.to_s.scan(/\d+|\D+/).map { |s| s =~ /\d+/ ? s.to_i : s.downcase }
end
end

end
2 changes: 1 addition & 1 deletion app/views/transactions/_form.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<%= render DS::Disclosure.new(title: t(".details")) do %>
<%= f.fields_for :entryable do |ef| %>
<%= ef.select :tag_ids,
Current.family.tags.alphabetically.pluck(:name, :id),
Current.family.tags.sorted_naturally.pluck(:name, :id),
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Prefer DB-level sort for options to avoid loading all records and to keep consistent ordering

Using the scope keeps pluck on the database and avoids Ruby-level sorting overhead.

-                    Current.family.tags.sorted_naturally.pluck(:name, :id),
+                    Current.family.tags.sorted_naturally_db.pluck(:name, :id),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Current.family.tags.sorted_naturally.pluck(:name, :id),
Current.family.tags.sorted_naturally_db.pluck(:name, :id),

{
include_blank: t(".none"),
multiple: true,
Expand Down
2 changes: 1 addition & 1 deletion app/views/transactions/show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
"data-auto-submit-form-target": "auto" %>

<%= ef.select :tag_ids,
Current.family.tags.alphabetically.pluck(:name, :id),
Current.family.tags.sorted_naturally.pluck(:name, :id),
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Align with DB scope for consistent, efficient natural ordering

Same reasoning as in _form: keep sorting in SQL and let pluck run on the DB.

-                    Current.family.tags.sorted_naturally.pluck(:name, :id),
+                    Current.family.tags.sorted_naturally_db.pluck(:name, :id),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Current.family.tags.sorted_naturally.pluck(:name, :id),
Current.family.tags.sorted_naturally_db.pluck(:name, :id),
🤖 Prompt for AI Agents 
app/views/transactions/show.html.erb around line 80: the view currently calls
sorted_naturally in Ruby before pluck, causing sorting to happen in memory;
change this to use an ActiveRecord/SQL ordering so the DB performs the natural
sort and then pluck runs on the DB. Replace the in-memory sort with the model
scope or an order(...) that implements natural ordering (or ensure
sorted_naturally is an ActiveRecord scope) so you call
.order(...)/.sorted_naturally_scope.pluck(:name, :id) instead of sorting the
array after loading.

{
include_blank: t(".none"),
multiple: true,
Expand Down