tingobol / gii-template-collection

Automatically exported from code.google.com/p/gii-template-collection
0 stars 0 forks source link

FullCrudCode uses table schema to determine related class to load, causes exception when table name is not the same as model name #5

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Assume models Country has many Region. Their corresponding database tables are 
prefix__country and prefix__region.

FullModel works fine with this. But when you try to run FullCrud on Region 
(that BELONGS_TO Country) and click preview, the method 
FullCrudCode::generateValueField() causes an exception at line 156 (as of r63). 
The exception log is at the bottom of this post.

The problem seems to be this, at the beginning of the function:
- The table schema for the current model (Region) is loaded.
- The schema's foreign key for the column that we are generating output for, is 
looked up (country_id).
- The foreign key's is inspected in order to resolve the foreign table 
(prefix__country).
- We then try to instantiate the related model using the foreign table name.
- This fails because there is no model class named like that (prefix__country, 
should be Country).

After investigating the current model it looks like we could instead use its 
relations to fetch the name of the related class. I'll try this and if it works 
I'll commit.

Question: Could this bug exist somewhere else too or is it just in this place 
that we are doing this kind of lookup?

Original issue reported on code.google.com by l...@finalresort.org on 4 Aug 2010 at 11:50

GoogleCodeExporter commented 9 years ago
Ok, fail.. In the views calling FullCrudCode::generateValueField() we don't use 
the relations of the model, but loop and handle things using the table schema 
and its columns.

This means that the incoming column namn that I thought I could use in 
generateValueField() is 'country_id' instead of the relation name 'countries'.

Question: Why is it the table schema that is used to generate output/fields 
instead of the model's attributes and relations? The latter seem much more 
appropriate, both naturally and especially considering that what the user has 
defined as attributes and relations in the model is what they want to have 
fields for in their crud forms -- not necessarily all columns.

So, as I understand it right now (without having gone through ALL the code) 
there are two options:
1) Either keep the table *schema* as the base for all the work in view 
generation, and in FullCrudCode::generateValueField() loop through the 
relations in order to find the relation that matches the given column, and use 
that relation to get the class name of the related model.
2) Or, redo the view stuff so that we don't base things on the table schema but 
rather the model's attributes and relations.

Original comment by l...@finalresort.org on 5 Aug 2010 at 12:00

GoogleCodeExporter commented 9 years ago
Ok, I worked around this for now, because I really have to get going with my 
project. I did it by doing the following to get the related model inside 
FullCrudCode::generateValueField():

            foreach($model->relations() as $relation) {
                if($relation[2] == $column->name) {
                    $fmodel = CActiveRecord::model($relation[1]);
                }
            }

However, it's not pretty, and I still think we should discuss the fact that 
everything is centered around the table schema instead of attributes and 
relations.

I also noticed that in the generated views we get things like: 
CHtml::listData(Prefix__country::model()->findAll(), 'id', 
'creation_timestamp') instead of the right class name in there, and this too is 
of course because everything uses table schema..

Maybe my specific problem could've been solved using table prefixes, I haven't 
tried that. I should.

Original comment by l...@finalresort.org on 5 Aug 2010 at 12:28

GoogleCodeExporter commented 9 years ago
These issues should be fixed with r113

Original comment by schm...@usrbin.de on 16 Sep 2010 at 10:18

GoogleCodeExporter commented 9 years ago

Original comment by thyseus on 17 Sep 2010 at 8:29