[drupal-devel] [bug] Can't delete subjects with '&' in title

Tue Sep 13 22:21:25 UTC 2005

Issue status update for 
http://drupal.org/node/27140
Post a follow up: 
http://drupal.org/project/comments/add/27140

 Project:      Drupal
 Version:      cvs
 Component:    contact.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  Souvent22
 Reported by:  m3avrck
 Updated by:   Cvbge
-Status:       patch (code needs work)
+Status:       patch (ready to be committed)

About the update path: since this module is in cvs only, not in 4.6,
then we probably should not worry about data being already in the
table.
(Although drupal.org is already running on cvs with contact.module :))

Cvbge

Previous comments:
------------------------------------------------------------------------

Wed, 20 Jul 2005 14:57:03 +0000 : m3avrck

Using the site-wide contact forms, any subjects with an '&' cannot be
deleted. Clicking delete for these subjects brings up the expected
action flow, however, confirming the delete does nothing. Subject still
exists.

------------------------------------------------------------------------

Mon, 25 Jul 2005 13:45:38 +0000 : m3avrck

It appears this issue might be linked with this one:
http://drupal.org/node/23685

A problem with mod_rewrite and not the actual contact module.

------------------------------------------------------------------------

Wed, 31 Aug 2005 16:51:56 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/contact.module_6.patch (4.7 KB)

Redid how contacts work. It now references contacts by ID instead of
their category as the unique identifier. It also allows for contacts to
have weights, so that one may control where the contact will appear in
the listing.

------------------------------------------------------------------------

Wed, 31 Aug 2005 16:53:01 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/updates.inc (27.23 KB)

This is the update.inc for the database cahnges that goes along with
this patch.

------------------------------------------------------------------------

Wed, 31 Aug 2005 18:31:37 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/contact.module_7.patch (4.5 KB)

Updated patch.

------------------------------------------------------------------------

Wed, 31 Aug 2005 18:31:55 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/updates.inc_4.patch (1.29 KB)

Updated database patch.

------------------------------------------------------------------------

Sat, 03 Sep 2005 16:17:21 +0000 : Cvbge

Hi, 

0. You should keep all changes in one file, i.e. pake one patch, not
patch for every file
1. You should include changes for database/database.{mysql,pgsql}
2. IIRC auto_increment was depreciated, you should use db_next_id()
(but I see auto_increment in cvs...)
3. You use "WHERE cid = '%s'", but cid is integer so this should
probably be %d (although this probably would not break anything..)

------------------------------------------------------------------------

Wed, 07 Sep 2005 17:31:21 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/contact_full.patch (6.38 KB)

Ok, I've taken your suggestions in, and i think I have it this time. Let
me know if this is a correct format for a patch. This only fixes the bug
of not being able to delete special characters.

------------------------------------------------------------------------

Wed, 07 Sep 2005 17:37:30 +0000 : m3avrck

Check your database updates, the UNIQUE fields don't look right
syntactically.

------------------------------------------------------------------------

Wed, 07 Sep 2005 18:23:07 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/contact_full_0.patch (6.25 KB)

Ok, i apologize for the last upload. Pleasle ignore. I send the wrong
file. :-[. Anyway, here's a tested, tried, and correct patch.

------------------------------------------------------------------------

Wed, 07 Sep 2005 18:26:33 +0000 : m3avrck

+1 patch works great! Latest HEAD version applied with update.php works
great, modifies database correctly for MySQL (and PostgreSQL looks
correct as well) and contacts can now be deleted if they have '&' in
the subject. Also, dumped entire database and created from modified
database.mysql and everything works great. I'd say this patch is ready
to be committed!

------------------------------------------------------------------------

Wed, 07 Sep 2005 21:44:47 +0000 : Cvbge

Hi, patch does not applies to currect cvs anymore.

For the database.pgsql you need to remove (11) from int(11) and also
first "category" from UNIQUE line, so it looks like this:
CREATE TABLE contact (
  cid int NOT NULL,
  category varchar(255) NOT NULL default '',
  recipients text NOT NULL default '',
  reply text NOT NULL default '',
  UNIQUE (category),
  PRIMARY KEY (cid)
);

The updates are also not compatibile with postgres, but postgres
upgrade path is completly broken in cvs and I'll fix it later, so do
not pay attention to it.

------------------------------------------------------------------------

Thu, 08 Sep 2005 13:05:54 +0000 : Souvent22

Hello. Thanks for the feedback. I'm in the middle of fixing it, but i
have a question. You'll have to excuse my ignorance with PostGres, I'm
not as familiar with it as I am MySQL. With that said, I have a
question.....
I am trying to make my update where one does not lose data. However, in
PostGres, I have to make a seperate "temp" table before hand and then
copy the date over the the new table (cause i don't know the exact name
of the constraint to drop e.g. the primary key).  

Basically I'm asking if there is a better suggestion in which I doint
have to use a temp table, and i could just manipulate the original
table? Thanks.

------------------------------------------------------------------------

Thu, 08 Sep 2005 13:12:35 +0000 : Souvent22

Also,

I created a temp name for my temp table. However, for prefix purposes,
should I enclose it? Example:

$temp_name = rand();
$res = db_query("SELECT * FROM $temp_name");
or
$temp_name = rand();
$res = db_query("SELECT * FROM {$temp_name}");

Thanks.

------------------------------------------------------------------------

Thu, 08 Sep 2005 13:55:40 +0000 : Souvent22

Clarification:

$temp_name = rand();
db_query("CREATE TABLE $temp_name AS SELECT * FROM {contact}");
$res = db_query("SELECT * FROM $temp_name");
or
$temp_name = rand();
db_query("CREATE TABLE {$temp_name} AS SELECT * FROM {contact}");
$res = db_query("SELECT * FROM {$temp_name}");

------------------------------------------------------------------------

Thu, 08 Sep 2005 17:52:06 +0000 : Cvbge

"cause i don't know the exact name of the constraint to drop e.g. the
primary key

"
Yes, you know it, it's tablename_pkey. In this case it's contact_pkey.
You can drop it by dropping index with it's name (you could drop
constraint with it's name but it was not available in 7.2)
Adding a column and adding unique attribute is also doable without temp
table. You can look at (some) previous updates. But some of them are
incorrect ;)

By saying that I'll fix it later I meant that I want to introduce
function to modify tables and don't want to make temporary fixes now.

------------------------------------------------------------------------

Thu, 08 Sep 2005 19:47:08 +0000 : Souvent22

Thanks. yes, you're right. didn't know if that was universal across the
board though since that'st he default, but someone could give their
primary key constraint a diff. name.  But, with your comment, I'll wait
and let you take care of fixing my pgsql portion of my patch. Thanks for
your comments though, really helpful...and not confusing. :).

------------------------------------------------------------------------

Fri, 09 Sep 2005 19:16:19 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/contact_full_1.patch (6.23 KB)

Got the postgres correct this time. No temp table needed.

------------------------------------------------------------------------

Tue, 13 Sep 2005 14:23:09 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/contact_full_2.patch (6.35 KB)

patch re-roll due to head change.

------------------------------------------------------------------------

Tue, 13 Sep 2005 14:36:11 +0000 : chx

Does this merit a DB change? what about passing md5(subject) instead? or
is that too hackish? seems to be lot simpler to me...

------------------------------------------------------------------------

Tue, 13 Sep 2005 15:15:16 +0000 : m3avrck

chx, md5() would work but that does seem a bit hackish ;)

------------------------------------------------------------------------

Tue, 13 Sep 2005 16:24:34 +0000 : Cvbge

Attachment: http://drupal.org/files/issues/drupal-head-contact-27140_0.diff (6.81 KB)

It's case 'pgsql' not case "pg_sql"

Also, I've noticed now that you use db_next_id().
First, the format is db_next_id('{tablename}_fieldname'), so in this
case it should be db_next_id('{contact}_cid')
Second, using db_next_id() with postgres needs a sequence defined.

Also update path needs changes in case there are already some contacs
in the table - the new column cid needs to be updated and sequences
set.

I've fixed this [for postgres] and attached new patch. I'm not sure how
mysql behaves when adding a primary key and there is an existing data,
buy you probably have to take care of it (if mysql allows you to add a
column that is a primary key and allows it to have nulls then... well,
I don't want to swear ;))

I've tested the code and it seems to run ok.