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

Are STIs supported? #123

Closed
notalex opened this issue Jan 27, 2018 · 5 comments
Closed

Are STIs supported? #123

notalex opened this issue Jan 27, 2018 · 5 comments

Comments

@notalex
Copy link

notalex commented Jan 27, 2018

Crystal's inherited hook is called even when inheriting child is inherited. So when we have an STI model like so, the inherited hook is called twice.

class Animal < Granite::ORM::Base
  macro inherited
    puts "inherited macro called"
  end
end

class Dog < Animal
end

class Cat < Animal
end

In Granite, the validators.cr uses the inherited hook to define @@validators.

  macro inherited
    @@validators = Array({field: Symbol, message: String, block: Proc(self, Bool)}).new
  end

However when we define a new child model, granite tries to create @@validators again and fails because Crystal cannot reinitialize a class var.

Is there a workaround for this?
Thanks for your effort.

@drujensen
Copy link
Member

drujensen commented Jan 27, 2018

@notalex yes. @faustinoaq fixed this in his last PR. 🥇 We removed the class var. I will release Granite soon or you can try the master branch.

@notalex notalex closed this as completed Jan 27, 2018
@faustinoaq faustinoaq added this to Done in Framework 2018 May 5, 2018
@johansenja
Copy link

Are there any updates on this? I have seem to be having the same problems - I'm on version 0.15.2, which as far as I am aware appears to be the latest

@faustinoaq faustinoaq reopened this Apr 7, 2019
Framework 2018 automation moved this from Done to To Do Apr 7, 2019
@Blacksmoke16
Copy link
Contributor

@johansenja This will be fixed in #346.

abstract class Animal < Granite::Base
  column id : Int64, primary: true
  column color : String
  column breed : String
end

class Dog < Animal
  table dogs
  column dog_column : String
end

class Cat < Animal
  table cats
  column cat_column : String
end

Dog.new
#<Dog:0x7f49e4a59f00
 @_current_callback=nil,
 @breed=nil,
 @color=nil,
 @destroyed=false,
 @dog_column=nil,
 @errors=[],
 @id=nil,
 @new_record=true>

Cat.new
#<Cat:0x7f49e4a59e60
 @_current_callback=nil,
 @breed=nil,
 @cat_column=nil,
 @color=nil,
 @destroyed=false,
 @errors=[],
 @id=nil,
 @new_record=true>

It will at least not error on you. However I wouldn't call this STI in that there would have to be some extra work to support automatically settings a discriminator.

@Blacksmoke16
Copy link
Contributor

Actually after looking into the issue more it's a bit more involved than I thought. My fix then causes some other issues when actually using the validations.

I'm going to save this for another PR at some point if I think of another option.

@crimson-knight
Copy link
Member

Closing this out, but keeping the other RFC issue about changing how validations work since that's the core issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Framework 2018
  
To Do
Development

No branches or pull requests

6 participants