-
Notifications
You must be signed in to change notification settings - Fork 138
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
non-conventional primary keys and attributes #409
Comments
Hey Jason 👋 Great to hear from you! A legacy non-rails DB was the original use case for Graphiti, actually. But we used I agree this should be a Resource responsibility. But
filter :id do
eq do |scope, value|
scope.where(thingid: value)
end
end This fixes your immediate issue. But for attribute :id do
@object.thingid
end Also remember it's not just filter :id do
match do |scope, value|
scope.where("thingid LIKE %?%", value)
end
end Plus a whole thing when we get to writes. So basically - there are fairly easy overrides built for this purpose, but it gets very boilerplatey when you want a comprehensive override for all behavior. So, you could do that, stick it in a module, you now have your very own Resource-based That's one route, the other is to just build this into graphiti itself, lots of code changes to I assume I you'd also prefer not to use a database view? |
Just want to be understand, do you suggest a mixin? Including the module your are talking about in the resource class? I have the same problem, I look at the code and I way going in the direction of creating a new adapter that inherits from the ActiveRecord adapter and override the method that return the Arel column. But this solution would only work if all the model have the same column name as primary id. I prefer what you suggest @richmolj |
Should thoses changes only be on the adapter for ActiveRecord? |
@richmolj I am available if you need help for this issue. 🙋♂️ |
@masterT thanks! I think updating Graphiti itself is the best route, but the module maybe the quickest/easiest. If you go the Graphiti route I think it would be good to pass in the "correct" attribute to the adapters, so no change or logic needed in the adapters themselves. |
All right, I will continue my digging. 🔎 |
Finally coming back to this... I kinda waffle on two different opinions. On the other hand, I understand that aliasing attributes in the general case is a common activity. IMO, I feel that hiding a database's "dirty" column names is precisely the responsibility of an ActiveRecord model, which is doing the job of the data layer. Exposing only "clean" attribute names from the model allows the api layer (resources) to be blissfully ignorant of any crufty db names. So I rather think that thoughts? |
@jasonkarns well written, I think I agree. To do this would require touching query-building, serialization, and writes so it's a little work. But I agree with the principle. |
Hi 👋. Love that graphiti exists!
Scenario: layering in a graphiti-powered api over a legacy (non rails-conventional) database.
Our thinking is that the ActiveRecord objects should remain as close to the legacy db as possible (ie, we probably don't want to define too many
alias_attribute
s, for instance). This way the AR models are honest about what the db schema looks like. On the flip side, the REST api should be designed to be more "ideal" in how the domain objects are represented.To that end, it's presumably the job of the Resource then, to map the "ideal api-level" names/properties to the legacy forms on the AR models. Does this feel like the right approach without setting up for too much pain?
More particular question, what to do about non-conventional primary keys.
Assuming:
Our feeling is that we would like
ThingResource
to still exposeid
as the primary key at the api layer, since that is "ideal". (And presumably, exposing non-conventional primary keys in the api layer would cause a lot of pain to the api clients and their ability to use jsonapi.org libraries.)When we do
ThingResource.find(id: 42)
, we get the incorrect SQL query:It seems graphiti is not respecting
Thing
's custom primary key. (Which also implies the active record adapter backend is not using ActiveRecord'sThing.find
which would respect the customized primary key.So, what is graphiti's opinion on the best place to define these customizations that reconciles our desire to expose the more "ideal/conventional" domain api while at the same time not setting ourselves up for a world of hurt with graphiti's resource associations and the frontend/mobile api clients (read: jsonapi spec libs). (Keeping in mind this is a legacy database so non-conventional PKeys, FKeys, and whatnot will be the rule, not the exception.)
The text was updated successfully, but these errors were encountered: