-
Notifications
You must be signed in to change notification settings - Fork 361
Singletons: Default collection_name
to singleton_name
#418
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
Open
seanpdoyle
wants to merge
1
commit into
rails:main
Choose a base branch
from
seanpdoyle:singleton-custom-methods
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seanpdoyle
commented
Mar 7, 2025
Comment on lines
+134
to
+136
def custom_method_element_url(method_name, options = {}) | ||
"#{self.class.prefix(prefix_options)}#{self.class.collection_name}/#{method_name}#{self.class.format_extension}#{self.class.__send__(:query_string, options)}" | ||
end |
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.
This implementation is copied directly from ActiveResource::CustomMethods
, then altered to omit the id
portion of the path:
activeresource/lib/active_resource/custom_methods.rb
Lines 121 to 123 in 9c8a2ee
def custom_method_element_url(method_name, options = {}) | |
"#{self.class.prefix(prefix_options)}#{self.class.collection_name}/#{URI.encode_www_form_component(id.to_s)}/#{method_name}#{self.class.format_extension}#{self.class.__send__(:query_string, options)}" | |
end |
def custom_method_element_url(method_name, options = {})
- "#{self.class.prefix(prefix_options)}#{self.class.collection_name}/#{URI.encode_www_form_component(id.to_s)}/#{method_name}#{self.class.format_extension}#{self.class.__send__(:query_string, options)}"
+ "#{self.class.prefix(prefix_options)}#{self.class.collection_name}/#{method_name}#{self.class.format_extension}#{self.class.__send__(:query_string, options)}"
end
The problem --- Active Resource's custom methods are operations outside of the conventional collection of CRUD actions. For typical collections of resources, custom methods operate on the collection itself. For example, consider a custom `refresh` action for a `Product` resource triggered by `POST` requests: `POST /products/:product_id/refresh`. The route utilizes a `:product_id` dynamic segment to identify the `Product` in question, and the path includes `/refresh` to signify the custom method to perform. The concept of a "singleton" resource is that there is only one singular resource, and operations should consistently modify the same resource. Since the resource is singular in nature, paths *do not* identify that resource with an ID. For example, consider a singleton `Inventory` resource that belongs to a `Product`. Its singleton path would be `/products/:product_id/inventory`. Prior to this commit, custom methods invoked by both *instances* and *classes* of singleton resources ignored the "singleton" nature of route, and use pluralize nouns instead of singular ones. For example, consider calls to custom "report" and "reset" methods for an instance of a singleton `Inventory` resource: ```ruby inventory = Inventory.find(params: { product_id: 1 }) # => GET /products/1/inventory.json # BEFORE inventory.get(:report, product_id: 1) # => GET /products/1/inventories/report.json inventory.delete(:reset, product_id: 1) # => DELETE /products/1/inventories/reset.json ``` Note the `/inventories/` portion of the URL prefix. The same occurs for class methods. For example, consider calls to the same custom "report" and "reset" routes for a singleton `Inventory` resource class: ```ruby # BEFORE Inventory.get(:report, product_id: 1) # => GET /products/1/inventories/report.json Inventory.delete(:reset, product_id: 1) # => DELETE /products/1/inventories/reset.json ``` The proposal --- In order to make "singleton" resources behave more consistently with a singular mental model, this commit proposes that Active Resource change instance-level custom methods (through the same `get`, `post`, `put`, `patch`, and `delete` style methods) to use the singular singleton name in their paths. When declaring a resource as a "singleton" (through including the `ActiveResource::Singleton` module), ensure that subsequent calls to class-level custom methods (through the `get`, `post`, `put`, `patch`, and `delete` class and instance methods) use the singleton name by default. ```ruby # AFTER Inventory.get(:report, product_id: 1) # => GET /products/1/inventory/report.json Inventory.delete(:reset, product_id: 1) # => DELETE /products/1/inventory/reset.json ``` When a `collection_name` is explicitly configured, use that value instead of the `singleton_name` default. Apply the same changes to instances of singleton resources: ```ruby inventory = Inventory.find(params: { product_id: 1 }) # => GET /products/1/inventory.json # AFTER inventory.get(:report) # => GET /products/1/inventory/report.json inventory.delete(:reset) # => DELETE /products/1/inventory/reset.json ```
d82b7cd
to
213f735
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem
Active Resource's custom methods are operations outside of the
conventional collection of CRUD actions. For typical collections of
resources, custom methods operate on the collection itself. For example,
consider a custom
refresh
action for aProduct
resource triggered byPOST
requests:POST /products/:product_id/refresh
. The routeutilizes a
:product_id
dynamic segment to identify theProduct
inquestion, and the path includes
/refresh
to signify the custom methodto perform.
The concept of a "singleton" resource is that there is only one singular
resource, and operations should consistently modify the same resource.
Since the resource is singular in nature, paths do not identify that
resource with an ID. For example, consider a singleton
Inventory
resource that belongs to a
Product
. Its singleton path would be/products/:product_id/inventory
.Prior to this commit, custom methods invoked by both instances and
classes of singleton resources ignored the "singleton" nature of
route, and use pluralize nouns instead of singular ones.
For example, consider calls to custom "report" and "reset" methods for
an instance of a singleton
Inventory
resource:Note the
/inventories/
portion of the URL prefix. The same occurs forclass methods. For example, consider calls to the same custom "report"
and "reset" routes for a singleton
Inventory
resource class:The proposal
In order to make "singleton" resources behave more consistently with a
singular mental model, this commit proposes that Active Resource change
instance-level custom methods (through the same
get
,post
,put
,patch
, anddelete
style methods) to use the singular singleton namein their paths.
When declaring a resource as a "singleton" (through including the
ActiveResource::Singleton
module), ensure that subsequent calls toclass-level custom methods (through the
get
,post
,put
,patch
,and
delete
class and instance methods) use the singleton name bydefault.
When a
collection_name
is explicitly configured, use that valueinstead of the
singleton_name
default.Apply the same changes to instances of singleton resources: