Table prefix is still not handled

YAFA - April 1, 2009 - 18:20
Project:Movie database
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:critical
Assigned:ultimatedruid
Status:needs work
Description

First of all, this is a great module. We used it for club-reviews of our Sneak-Preview-Night-Club. I'd really love to see, that this module is continued and updated contemporarily. As I don't have the time for contributions at the moment, what I can do, is, to write a bug-report.

I already wrote one for this bug nearly two years ago (http://drupal.org/node/150103). I think the bug is critical, because it prevents the module from working at all, if you have a drupal setup with table prefixes. The code used in conjunction with table names simply is bad.

Within moviedb.module $mdb_job is defined as an object, also for handling the table-names used: $mdb_job->table-name. This variable is set and read on several lines in the code, but the table-prefixes aren't handled in any way. The tables are created correctly - with the prefix - but on writing or reading data to or from the db, they aren't.

You first get in contact with that bug after installing the module and creating the first moviedb-node. You will get several errors, that there is no table 'moviedb_actors', as well as 'moviedb_directors', 'moviedb_producers' and 'moviedb_writers', because their names are, for example 'd_moviedb_actors', where 'd_' is the prefix. That happens in mdb_common.inc on line 267:

<?php
256
function moviedb_movie_person_join($mdb_job, $mid, $pid, $weight, $data) {
257
258  
if (!$mdb_job->table_name) {
259     return FALSE;
260  
261   elseif ($mdb_job->table_name == 'moviedb_misc') {
262     db_query("INSERT INTO {moviedb_misc} (mid, pid, jid, weight, data) VALUES (%d,%d, %d, %d, '%s')", $mid, $pid, $mdb_job->jid, $weight, check_plain($data));
263   }
264   else {
265     $table = $mdb_job->table_name;
266
267     db_query
("INSERT INTO {$table} (mid, pid, weight, data) VALUES (%d, %d, %d, '%s')", $mid, $pid, $weight, check_plain($data));
268   }
269 }
?>

#1

YAFA - April 1, 2009 - 18:26

Further information:

Either $mdb_job->table-name, $table or db_query("INSERT INTO {$table} (mid, pid, weight, data) VALUES (%d, %d, %d, '%s')", $mid, $pid, $weight, check_plain($data)); should be modified to handle the prefix correctly within this function.

#2

YAFA - April 1, 2009 - 19:49

After playing around with the code of the module, I have to say, it's pretty unusable at the moment. Please don't get me wrong. At the moment I am sad about that, but I'm looking forward to when I will use it.

I had a working version running within a drupal 5.1 and thought this one would work like that. But it seems the code is being revisioned in a deeper way.

Great module, stay with it.

#3

ultimatedruid - April 2, 2009 - 01:01

Hi there,
Thanks for your comments. I'd just like to say that the 6.x dev version is still in devlopement and has a long way to go before it is truely functioning correctly. I am using a version of the module on a site and it works ok - a few bugs that don't impact upon functionality, yet impact upon productivity that need to be ironed out. Unfortunately, I don;t have enough time at the moment to sort them out, and realistically won;t until the end of May. I'm sorry to say this, but any patches you can create would be a great help - as I truely believe that this module is needed and an important way to adding cross referencing functionality that may not otherwise esixt in Drupal.
Unfortuately i don;t even have the time a present to reply fully to this issue, but will do over the weekend. Appologies, but I hope you can understand!

#4

YAFA - April 2, 2009 - 18:12

Hi,

thanks for your quick reply on this. I truly understand time issues, as I have them myself. I'd really like to contribute code, but it's been a while since I really coded php. Some hardcodefixes here and there ok, but real project code - phew. I'm into Java at the moment and have several other, more conceptual issues, so there's barely time.

I do indeed see a need for this module, that's why I posted this ;o)

I had an idea to realize the functionality another way and got stopped at a certain point. Maybe this is an alternative for your whole development on functionality like this: CCK. Before you wave your hands, read on:
I created two content types: 'movie' and 'movieperson' and added a field 'actors' to the movie-submission form, that should create a node reference to a movieperson-node. Now the point I was stopped is, that currently there is no ready-to-use functionality to automatically create a node for a reference. The closest module is 'Popups: Add & refer'.

You could combine the following to build a whole module based on CCK, wich is integrated into Drupal-core of d7:
- New nodetypes with presetup fields
- New field-types for persons, infos, pictures and links (amazon, imdb, offsite) in relation to movies
- Autocreate-function for referred nodes, that don't exist.

This would kill your whole module, but maybe it's much easier to reuse, what already exists and implements >80% of your needed functionality!

Anyways, if you need someone for testing and rifle through code, I want to help as well as I can. I also use Eclipse PDT+svn for setting up my Drupal-sites, so it's easy to jump around between branches for testing and documentation.

Greetings,
Felix aka YAFA

#5

ultimatedruid - April 6, 2009 - 17:29
Assigned to:Anonymous» ultimatedruid
Status:active» needs work

Hi Felix,
THanks again for your comments - I have some thoughts about how best to proceed with the database prefix, but am not certain if it'll work or not.

Regarding CCK for this functionality - I would advise anyone that doesn't need the dual relational aspects of this module to merely use CCK - it is an awesome module, however, for one of my sites in particular, it doesn't do all of the things that I want it to without an enormous amount of work - whereas this module does (or atleast tries!!).

I agree that if there is a future for this module (MovieDB) then integration with CCK (or similar) and views is essential. However, once that connection with CCK is made, then there is no longer the need for this module - it can be used as an add on to CCK to auto create new references. This is the main problem though. At present, I am unable to devote the time to even attempt this task - let alone complete it! However, I think that once the bugs in the .dev version get ironed out, then cck type functionality shouldn't be too far away (especially if Views integration is achieved). It won;t be a module to replace CCK, in fact it'll be the opposite - I'm not sure that this type of relation can be done (at present) in CCK, as there are better coders than I trying - and - to the best of my knowledge - failing, so for people who need this functionality, I think this module can be useful!

 
 

Drupal is a registered trademark of Dries Buytaert.