Skip to content

Conversation

shivabhusal
Copy link
Contributor

@shivabhusal shivabhusal commented Sep 25, 2025

Motivation / Background

While browsing the Active Record codebase, I came across the Aggregations module. I realized that, despite its usefulness, it isn’t currently documented in the Rails Guides. After confirming that no guide exists for this feature, I decided to add one so that developers can better understand and utilize Active Record Aggregations.

Details

This pull request introduces a new guide for Active Record Aggregations, explaining how to use composed_of with value objects through examples and caveats.
Additionally, it updates the Rails Guides index and navigation menu to include this new document for easier discovery.

Suggestions, improvements, and feedback are very welcome.

Add Doc(Latest) Preview: https://shivabhusal-active-record-ag.rails-docs-preview.pages.dev/guides/active_record_aggregations

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.

@rails-bot rails-bot bot added the docs label Sep 25, 2025
@shivabhusal shivabhusal force-pushed the active-record-aggregations branch from 7df6fe9 to 85eca80 Compare September 25, 2025 10:30
composed_of :price,
class_name: "Money",
mapping: [%w(price_cents amount), %w(price_currency currency)],
# mapping: {price_cents: :amount, price_currency: :currency}, # alternative syntax
Copy link
Contributor

Choose a reason for hiding this comment

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

I know the docs often use the array form, but I think the hash syntax is much nicer and clearer. Could we use that as the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

table columns.

```ruby
expensive_products = Product.where("price_cents > ?", 1000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we be using the Money object here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes we should. done!

Comment on lines 282 to 283
Since you cannot use aggregated attribute(like `price`) directly in database queries,
you need to use the underlying table columns(like `price_cents`) in scopes.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the first paragraph you say

You can also use the aggregated attributes in scopes and validations

And here you say that we have to use the underlying table columns. I'm confused what is the aggregation doing here. It seems to only work on validations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made corrections

composed_of :balance, class_name: "Money", mapping: { balance: :amount }
composed_of :address, mapping: { address_street: :street, address_city: :city }
composed_of :address, mapping: [%w(address_street street), %w(address_city city)]
composed_of :gps_location
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need mappings other options for this?

Copy link
Contributor Author

@shivabhusal shivabhusal Sep 26, 2025

Choose a reason for hiding this comment

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

def composed_of(part_id, options = {})
options.assert_valid_keys(:class_name, :mapping, :allow_nil, :constructor, :converter)
unless self < Aggregations
include Aggregations
end
name = part_id.id2name
class_name = options[:class_name] || name.camelize
mapping = options[:mapping] || [ name, name ]
mapping = [ mapping ] unless mapping.first.is_a?(Array)
allow_nil = options[:allow_nil] || false

Yes, it is also possible. Mapping is derived from part_id, i.e. :gps_location and other options are optional.


Other Option Examples
---------------------

Copy link
Contributor

Choose a reason for hiding this comment

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

I think need a sentence before the code example.

Suggested change
Here are some examples of value objects you might build using Active Record Aggregations in your app:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

accepted your suggestion

Caveats
-------

### Query with values in different units
Copy link
Contributor

@MatheusRich MatheusRich Sep 25, 2025

Choose a reason for hiding this comment

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

This is not exclusive to units. Value objects are identified by their values, so if you're using a different value, it's a different object.

The same would happen with a Distance object with kilometers vs miles.

A possible solution would be keep the db unit constant (dollars, meters, etc) and perform conversions/compasissons at the class level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added new descriptive example

product.changed? # => true
product.changes_to_save # => {"price_cents"=>[1000, 2000], "price_currency"=>["USD", "EUR"]}

product.price_changed? # => NoMethodError (no direct price_changed?)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this PR, but at first sight this seems doable. We could delegate methods back to the original model (if we have access to that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will have a look later if we can make such changes

Copy link
Contributor

@MatheusRich MatheusRich left a comment

Choose a reason for hiding this comment

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

Love this addition!

@shivabhusal shivabhusal force-pushed the active-record-aggregations branch from 85eca80 to 43fe86e Compare September 26, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants