cid field in comments table
By Hugh Messenger on Tuesday, 03 January 2017
Posted in Technical Issues
Replies 5
Likes 0
Views 500
Votes 0
Hi,

I'm lead dev for Fabrik (an app builder extension). I'm writing a Komento integration.

The problem I've run into is that the assumption underlying integrations is that the app has a single "article" table like #__categries, or vm_products, or k2_items, etc. So the 'cid' field in the main Komento #__komento_comments table is an INT, essentially expecting it to be the primary key on the app's single "article" table.

In Fabrik, we don't have a single "article" table. Our equivalent of categories / articles are "lists" (arbitrary database tables) and "row ids" (PKs in the tables). And any given Fabrik app can have dozens of different lists. And of course the rowids of those lists (tables) are not unique - they are all auto-inc PKs starting at 1.

So for Fabrik to work with Komento, the 'cid' needs to hold two ids - the list and the rowid.

As far as I can tell, the only change that needs to be made in Komento for this to work is the 'cid' field in #__komento_comments table needs to be a VARCHAR, not a LARGEINT. That way, I can set the 'cid' in my item to listid:rowid (like 17:147), and my various KomentoComfabrik class methods like load() can just split out the two ids I need from that.

I've tested locally, changing 'cid' to VARCHAR(64), and it doesn't seem to adversely affect Komento at all, and my integration then works perfectly.

I know changing field types is not something we like doing as extension devs ... but I'm hoping you might put it on your list for things to consider.

-- hugh
Hello Hugh,

Correct me if I'm wrong, your issue now is the type of the 'cid' column. To be honest, changing a column type is not advisable but I will take this matter to be discussed with the team and see how it goes.
Nik Faris
·
Tuesday, 03 January 2017 18:41
·
0 Likes
·
0 Votes
·
0 Comments
·
Yes, that is the issue.

And yes, I understand that changing field types is not something to be done lightly. But I've been through the Komento code, and there is nowhere it needs to be treated specifically as an integer, although there's a couple of places it is assumed to be, like some $input->get('cid', '', 'int'), although in other places it's fetched as 'cmd' not 'int'. So there would be a handful of code changes needed. But it isn't used in any join statements, so no risk of collation issues in join queries, etc. It's just a unique identifier provided by the integration plugin, that the Komento core stores and hands back to the integration. Komento is totally agnostic as to what type it is - it is just an arbitrary identifier.

And for all integrations up till now, having it as an integer works, because they have a single "articles" table (be it Virtumart products, K2 items, blog posts, etc), and the cid is the numeric primary key for rows in that single table. Fabrik just doesn't fit into that "single article table" model, and needs what amounts to a kind of pseudo composite key to uniquely identify an article.

Also note that it is safe to change an integer type to a varchar in MySQL, with no data loss.

I think I can work round this, albeit in a kind of kludgy way with a lot of overhead (maintaining a mapping table for Komento cids to my listid/rowid pairs), but I'd like to get this on your radar for the next release.

-- hugh
Hugh Messenger
·
Wednesday, 04 January 2017 02:59
·
0 Likes
·
0 Votes
·
0 Comments
·
Hello Hugh,

Thanks for the brief explanation. This would make our discussion easier. I'll let the other developers know about this. Will update you as soon as we made a decision.
Nik Faris
·
Wednesday, 04 January 2017 10:04
·
0 Likes
·
0 Votes
·
0 Comments
·
Didn't mean to sound like I was lecturing you on your own code. That was me thinking out loud.

BTW, I made the handful of little changes on my copy locally to the handful of places where $this->input->get() assumes 'int', and have been testing my integration, the J! content one, and the Virtumart one, so far no problems.
Hugh Messenger
·
Thursday, 05 January 2017 03:35
·
0 Likes
·
0 Votes
·
0 Comments
·
Hello Hugh,

Thanks! I will log this down.
Nik Faris
·
Thursday, 05 January 2017 09:57
·
0 Likes
·
0 Votes
·
0 Comments
·
View Full Post