Foreign key problem with namespaced Models about revisionable

[duellsy]

Hmm, a very valid issue, will have a look into it. Thanks.

[mmodler]

Thanks for your reply!
I think adding a "relatedmodel" field/colum to Revision and populate it onSave with the related models name/namespace will do it?

Then you'll also make custom foreign key names working. Atm. related model Foo with foreign key name fooBar_id would resolve to Model "FooBar" in Revisionable...

[mmodler]

Hi duellsy,

i appreciate your work on revisionable. The next days i have to get revisionable working completely in my app, so i would like to ask you about your plans with revisionable? Do you have time to maintain and improve it atm?

Because there are two forks with improvements and i need to decide what i use in my app. I would prefer to use "the original one" if i know the projekt is active. I could try to solve the foreign key issue and make a pull request (would be my first one) if you like?

[duellsy]

Hey @mmodler, yeah the project is definitely active and I've contacted the authors of the other forks and mentioned that they should be submitting pull requests to keep the project in the one direction.

All for you submitting a pull request with a fix, just been flat out myself recently, apologies for that.

[duellsy]

Currently working on a solution, have a working update but want to test it a little more to make sure its backwards compatible. I'll get you to test also before a full release if that's cool

[duellsy]

Hey @mmodler can you test this update with your app to check that all is in order, before I tag and release to production?

Might need to update your composer to use the dev version:

"venturecraft/revisionable": "dev-develop"

[mmodler]

Hi duellsy, i will test it now. Its a little more elegant than my quick and dirty fix which extracted the namespace from revisions_type.

[mmodler]

It seems to work like a charm. I have one issue with a foreign key named foo_bar_id, i will have to rename it to foobar_id so the model coluld resolved.

Why do you prefer throwing exeptions instead of returning "revisionUnknownString" and/or some new customizable strings? I i think of an running app with some old, not resolveable revisions i would prefer a clean sting output like "Model not found" or "Relation not found"?

[duellsy]

Cool.

Basically, because it's catching behaviour that shouldn't happen, therefor exceptions to the norm. The only cases that it should occur is if someone has modified the records in their database manually, or have moved their models to a new namespace. If you were to move a class to a new namespace and not change your code, an Exception would be thrown too, so it's just kind of sticking to that mentality.
As well as providing an area in code to be able to handle differently later, for now it just logs the issue for you to see in your log files.

[mmodler]

Yes you're right - if i mess up the databse or rename my models i'll have to check my revisions table. And without exeptions i would propably never notice that there is something broken...

I will continue testing...

[Antonimo]

I was declaring classnames anyway:

class Biz extends MyRevisionable {
protected $classname = 'Biz';

so you can use that:

'revisionable_type' => (isset($this->classname) ? $this->classname : get_class($this)),

about the case with old database: when changing the app, migrate the database (update the columns to have the new classnames with namespacing)