Skip to content
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

Legacy AR database -- polymorphic association #73

Closed
blelump opened this issue Mar 6, 2017 · 16 comments
Closed

Legacy AR database -- polymorphic association #73

blelump opened this issue Mar 6, 2017 · 16 comments

Comments

@blelump
Copy link

@blelump blelump commented Mar 6, 2017

Hello!

Since ROM API (and docs!) is going better and better with each release, we'd like to taste it for real. However, carrying a legacy AR DB needs some solution for 'polymorphic association' antipattern I haven't found. I've seen this and this, but none of them offer complete answer, at least for me. Let's continue with the code snippet given here and say that I have Snapshot and want to get the Questions, based on the parent_type attribute. In particular:

        snapshots.by_id(id)
          .combine(one: polymorphic_association_name...?)
          .one!

So the result is an aggregate of snapshot and whatever is given within the polymorphic association. In AR world, the aggregate's child is resolved based upon the parent_type column, but how to specify relation name based on that column value?

I'm not expecting any one-line solution, I'd be happy with any 😄 .

@solnic

This comment has been minimized.

Copy link
Member

@solnic solnic commented Mar 6, 2017

@blelump

This comment has been minimized.

Copy link
Author

@blelump blelump commented Mar 7, 2017

Hi Piotr!

Thanks for the gist ! How would you query from tag_repo for whatever is given for particular record? For instance, having insterted book_tag, how to get the aggregate of tag (the aggregate root) and book? In short, the opposite direction from your example (so you get: ROM::Struct[Tag] id=1 ... book=[#<ROM::Struct[Book] id=1 ...). Sorry if I haven't been more concrete previously.

@solnic

This comment has been minimized.

Copy link
Member

@solnic solnic commented Mar 7, 2017

@blelump updated the gist with tags => {song,book} associations too

@blelump

This comment has been minimized.

Copy link
Author

@blelump blelump commented Mar 7, 2017

@solnic , thanks for update ! However, I'll dig deeper ;-) . What if you dont know where particular tag is bound to? Say you want you want exactly ROM::Struct[Tag] id=1 ... book=[#<ROM::Struct[Book] id=1 ... , but you don't know the tag is associated to book. In essence, you don't know what's the value of taggable_type. I imagine:

puts tag_repo.aggregate(:taggable).inspect
=> `ROM::Struct[Tag] id=1 ... taggable=[#<ROM::Struct[Book] id=1 ...`

That may be difficult and I imagine you'd achieve the above entity with two separated queries and then merge the result with Transproc features. Is there any other solution (by just utilizing ROM)?

@solnic

This comment has been minimized.

Copy link
Member

@solnic solnic commented Mar 7, 2017

@blelump yes it's possible but requires custom relation composition, as associations don't support overridding query logic (you can only extend them via :view option, but it's not enough in this case). I'll prep another gist later today or tomorrow.

@solnic solnic added this to the v2.0.0 milestone Mar 7, 2017
@solnic

This comment has been minimized.

Copy link
Member

@solnic solnic commented Mar 7, 2017

I marked it as a feature for v2.0.0 milestone. We don't want this to be a first class feature, but it can be a plugin that people will be able to turn on when they need to connect to a legacy, post-AR database schema.

I imagine something like this could be done:

class Songs < ROM::Relation[:sql]
  schema(infer: true) do
    use :ar_polymorphic_associations

    associations do
      has_many :taggings, foreign_key: :taggable_id
      has_many :tags, as: :taggable, through: :taggings
    end
  end
end

class Tags < ROM::Relation[:sql]
  schema(infer: true) do
    use :ar_polymorphic_associations

    associations do
      belongs_to :taggable, polymorphic: true
    end
  end
end
@flash-gordon

This comment has been minimized.

Copy link
Member

@flash-gordon flash-gordon commented Mar 7, 2017

schema plugins? I started to think they'll be useful for auto-indexing and other type-manipulation things

@blelump

This comment has been minimized.

Copy link
Author

@blelump blelump commented Mar 7, 2017

@solnic , thanks in advance for another gist ! I completetely understand you don't want it as a basic feature. IMO it doesn't have to be a plugin either if the overall complexity of the acceptable solution is a few extra LoC. The API should be clear and straightforward, but on the other hand there're other places for sure that need one/your's attention (& improvements).

@solnic

This comment has been minimized.

Copy link
Member

@solnic solnic commented Mar 7, 2017

@blelump ayee, hence "help-wanted" label 🤣

@flash-gordon yeah I think we need schema plugins, as schemas are separate, standalone objects, so extending them through relation plugins would be awkward :)

@flash-gordon

This comment has been minimized.

Copy link
Member

@flash-gordon flash-gordon commented Mar 7, 2017

@solnic exactly!

@blelump

This comment has been minimized.

Copy link
Author

@blelump blelump commented Mar 8, 2017

Hello @solnic , sorry for the remainder, any progress on the taggable ? 🙄

@solnic

This comment has been minimized.

Copy link
Member

@solnic solnic commented Mar 8, 2017

@blelump

This comment has been minimized.

Copy link
Author

@blelump blelump commented Mar 11, 2017

Hi @solnic ,

I've tried to investigate it further and as I understand, at least from the public API, I need to provide another relation in order to 'combine' the current relation with something. However, I imagine, to make it working, I need to 'edit' current relation somehow (without another relation since it's not known yet -- I mean whether it's a book or a song) and append something to it, based on taggable_type. Here's the crux.

@jnylen

This comment has been minimized.

Copy link

@jnylen jnylen commented Jun 16, 2017

Hey @solnic,
Did you get anywhere on this?

I have some models that use polymorphism in mongoid that I want to move over.

The way I do it is:
belongs_to :media, polymorphic: true, index: true

where media can be any type of model.

What is the best way to do this? I don't want 10 different belongs_to.

@solnic

This comment has been minimized.

Copy link
Member

@solnic solnic commented Jun 16, 2017

@jnylen there's a chance it'll be supported in rom 4, if not then 4.1. It's low priority at the moment, so it needs to wait.

@solnic

This comment has been minimized.

Copy link
Member

@solnic solnic commented Jul 2, 2017

Closing this. We'll keep this in mind and eventually make this possible.

@solnic solnic closed this Jul 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.