-
Notifications
You must be signed in to change notification settings - Fork 42
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
Scimitar::ActiveRecordBackedResourcesController doesn't use mapped id
from scim_attributes_map
#35
Comments
While I'm adding specific support so that it does not rely on the ActiveRecord features I'm about to describe, I would note that your issues make it sound like your ActiveRecord model subclass is not properly declaring the name of its primary key column per this guide via this API: self.primary_key = :foo When you do this, ActiveRecord creates an alias so that your model instances will respond to The alias means that User.find(1) ...yields: User Load (0.6ms) SELECT "users".* FROM "users" WHERE "users"."foo" = $1 LIMIT $2 [["foo", nil], ["LIMIT", 1]] On the issue of url_for({controller: :users, action: :show, id: 1})
# => "http://www.example.com/users/1"
url_for({controller: :users, action: :show, foo: 1})
# No route matches {:action=>"show", :controller=>"users", :foo=>1} (ActionController::UrlGenerationError)
url_for({controller: :users, action: :show, foo: 22, id: 1})
# => "http://www.example.com/users/1?foo=22" ...so the Personally, I've never been a huge fan of the alias approach so I'm looking at a change set for Scimitar that will only query on the named column, though it's ended up a larger footprint than I expected. It should at least allow people to avoid using the ActiveRecord primary key declaration if they for some reason decided they didn't want it present. |
I do note that a recent change to |
#42 is now in review. I note that the test suite passes whether or not |
I have need to use a different column as we have an |
@acceptto-nhumble We've been a bit overloaded but I'm trying to escalate the review of #42 so we can get that new version pushed out. |
No worries, I just wanted to clarify my use-case. |
@acceptto-nhumble Yeah, that was useful, thanks. I just pushed v2.3.0 to RubyGems, so that's available. I'll back-port to V1 when I get a moment. |
...and that's now done, via #44 yielding version 1.4.0. https://rubygems.org/gems/scimitar/versions If this is working for you (or not!), please let me know so I can update the issue status. Thanks! |
...given no response, I'm going to make the assumption that this is resolved to the OP's satisfaction and close the issue. |
If I map
id
to another column inscim_attributes_map
it is ignored by the default ActiveRecord backed controller. This means I have to override not onlyfind_record
, but alsourl_for
so that the generated URLs have the correctid
in them.The text was updated successfully, but these errors were encountered: