Dynamic Import issue and possible fix #42

New Issue

Closed

opened 2017-02-22 18:10:21 +00:00 by Peeripapo · 7 comments

Peeripapo commented

2017-02-22 18:10:21 +00:00

(Migrated from github.com)

I've been trying to use this and found a small bug. When checking for data types it iterates this array in a_mapping.php:72

protected $dataTypes = array(	'CHAR' => 'Text', 'VARCHAR' => 'Text',
				'TEXT' => 'Textarea', 'MEDIUMTEXT' => 'Textarea', 
				'LONGTEXT'  => 'Textarea', 'DATE' => 'Text', 'TIME' => 'Text',
				'DATETIME' => 'Calendar', 'INT' => 'Text', 'TINYINT' => 'Text',
				'BIGINT' => 'Text', 'FLOAT' => 'Text', 'DECIMAL' => 'Text',
				'DOUBLE' => 'Text');

It uses strpos() for searching and since it firstly searches for CHAR Instead of VARCHAR, if a VARCHAR is being searched it will assume it it's a CHAR :) (since "char" string is part of "varchar" string)
Same will go for INT , TIME AND DATE.

We need to change the order in the array to have the longer name length types first so it processes this correctly.

He's a proposed solution:

protected $dataTypes = array(	'VARCHAR' => 'Text', 'CHAR' => 'Text',
				'MEDIUMTEXT' => 'Textarea', 'LONGTEXT'  => 'Textarea',
				'TEXT' => 'Textarea', 'DATETIME' => 'Calendar',
				'DATE' => 'Text', 'TIME' => 'Text', 'TINYINT' => 'Text',
				'BIGINT' => 'Text', 'INT' => 'Text',  'FLOAT' => 'Text',
				'DECIMAL' => 'Text', 'DOUBLE' => 'Text');

Didn't have the time to check the following array $dataSize to see if this fix is necessary too.

Hope I helped :)

I've been trying to use this and found a small bug. When checking for data types it iterates this array in a_mapping.php:72 protected $dataTypes = array( 'CHAR' => 'Text', 'VARCHAR' => 'Text', 'TEXT' => 'Textarea', 'MEDIUMTEXT' => 'Textarea', 'LONGTEXT' => 'Textarea', 'DATE' => 'Text', 'TIME' => 'Text', 'DATETIME' => 'Calendar', 'INT' => 'Text', 'TINYINT' => 'Text', 'BIGINT' => 'Text', 'FLOAT' => 'Text', 'DECIMAL' => 'Text', 'DOUBLE' => 'Text'); It uses strpos() for searching and since it firstly searches for CHAR Instead of VARCHAR, if a VARCHAR is being searched it will assume it it's a CHAR :) (since "char" string is part of "varchar" string) Same will go for INT , TIME AND DATE. We need to change the order in the array to have the longer name length types first so it processes this correctly. He's a proposed solution: protected $dataTypes = array( 'VARCHAR' => 'Text', 'CHAR' => 'Text', 'MEDIUMTEXT' => 'Textarea', 'LONGTEXT' => 'Textarea', 'TEXT' => 'Textarea', 'DATETIME' => 'Calendar', 'DATE' => 'Text', 'TIME' => 'Text', 'TINYINT' => 'Text', 'BIGINT' => 'Text', 'INT' => 'Text', 'FLOAT' => 'Text', 'DECIMAL' => 'Text', 'DOUBLE' => 'Text'); Didn't have the time to check the following array $dataSize to see if this fix is necessary too. Hope I helped :)

Llewellyn commented

2017-02-22 18:56:54 +00:00

So you have been testing this.... I have an idea to add the form_fields.xml to the import with the language files, well actually that you can upload the old component and that it maps the basic back-end from the files found in the component. SO to do away with the sql dump only... and add the drag a drop area instead that via ajax does all the work... hmmm what do you think?

Okay, I see the logic of this, but you do realize at this time both are converted to the same filed type. That is mainly why I did it that way... but to ensure we always get what we expect lets fix this... So you have been testing this.... I have an idea to add the form_fields.xml to the import with the language files, well actually that you can upload the old component and that it maps the basic back-end from the files found in the component. SO to do away with the sql dump only... and add the drag a drop area instead that via ajax does all the work... hmmm what do you think?

Peeripapo commented

2017-02-22 23:20:14 +00:00

(Migrated from github.com)

Still regarding my post please notice that on function getType() there are two assignments made
The fieldType is returned but in the line before
$field['dataType'] = $type;
the dataType is also assigned

Try this code:(in a compoment tab -> dynamic build)


CREATE TABLE IF NOT EXISTS `app_aal3` (
  `id` bigint(20) UNSIGNED NOT NULL AUTO_INCREMENT,
  `name` varchar(256) NOT NULL,
  `id_aal2` bigint(20) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=2604 DEFAULT CHARSET=utf8;

If you use the existing version of JCB the bigint will be imported as INT and the varchar as CHAR and will throw an error because CHAR type can have 255 length max :)

If you change the code to what i proposed it will work just fine :)

That sounds like a nice idea but what I'm actually trying to do is to migrate an app I've done which has no backend yet and use JCB to build the backend in Joomla for an already existing DB. I'm doing this exporting the tables structure from phpmyadmin, adding the #__ to the table names and pasting this on the components Dynamic Build tab (and then I just save, this way I can build the admin views one at a time and test for any errors). Still regarding my post please notice that on function getType() there are two assignments made The fieldType is returned but in the line before `$field['dataType'] = $type;` the dataType is also assigned Try this code:(in a compoment tab -> dynamic build) ``` CREATE TABLE IF NOT EXISTS `app_aal3` ( `id` bigint(20) UNSIGNED NOT NULL AUTO_INCREMENT, `name` varchar(256) NOT NULL, `id_aal2` bigint(20) NOT NULL, PRIMARY KEY (`id`) ) ENGINE=InnoDB AUTO_INCREMENT=2604 DEFAULT CHARSET=utf8; ``` If you use the existing version of JCB the bigint will be imported as INT and the varchar as CHAR and will throw an error because CHAR type can have 255 length max :) If you change the code to what i proposed it will work just fine :)

Llewellyn commented

2017-02-23 00:25:30 +00:00

hmmm I see.. well I already add your fix to the core, and the push should be up in the next few minutes.

I think in the long run, we should keep both options, one to use a dump and the other to upload an old component... the last will still take a few months. But for what you are using it, it will work :)

hmmm I see.. well I already add your fix to the core, and the push should be up in the next few minutes. I think in the long run, we should keep both options, one to use a dump and the other to upload an old component... the last will still take a few months. But for what you are using it, it will work :)

Peeripapo commented

2017-02-23 12:09:36 +00:00

(Migrated from github.com)

Great! Thanks for the quick fix.

Btw is there a forum where we can talk and discuss this kind of issues, suggestions and improvements ? Or should I do it right here ? I have a few of them already but don't really know where to put them.

Great! Thanks for the quick fix. Btw is there a forum where we can talk and discuss this kind of issues, suggestions and improvements ? Or should I do it right here ? I have a few of them already but don't really know where to put them.

Llewellyn commented

2017-02-23 13:13:17 +00:00

Well at this time in the project, github will have to do... I do have a project running that covers these automatic stuff... I will add the issues to the correct place in that project as we go..

So a guess that you can just open another issue if you have more improvement ideas... also if those ideas are related to the core, you can on our forked branch make the needed changes and then you can make a pull request and in that request we can workout the way forward. Since there is place on github to manage all this kind of discourse around the code... ideal for it really.

Well at this time in the project, github will have to do... I do have a [project](https://github.com/vdm-io/Joomla-Component-Builder/projects/2#card-1835907) running that covers these automatic stuff... I will add the issues to the correct place in that project as we go.. So a guess that you can just open another issue if you have more improvement ideas... also if those ideas are related to the core, you can on our forked branch make the needed changes and then you can make a pull request and in that request we can workout the way forward. Since there is place on github to manage all this kind of discourse around the code... ideal for it really.

Llewellyn commented

2017-02-23 13:55:05 +00:00

@Peeripapo I have added you as a collaborator on this project... well first as a member but then though that was little to much :) so I think collaborator will do... Looking forward to see what you have in mind.

Peeripapo commented

2017-02-23 14:57:43 +00:00

(Migrated from github.com)

Thanks :) I'll post my suggestions asap. Not very big changes, but definitively would help, at least me :D