Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#27 | drupal-head-contact-27140_2.diff | 6.88 KB | Cvbge |
#25 | contact_patch_0.txt | 5.59 KB | Souvent22 |
#24 | patch_10.txt | 5.62 KB | Souvent22 |
#21 | drupal-head-contact-27140_0.diff | 6.81 KB | Cvbge |
#18 | contact_full_2.patch | 6.35 KB | Souvent22 |
Comments
Comment #1
m3avrck CreditAttribution: m3avrck commentedIt 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.
Comment #2
Souvent22 CreditAttribution: Souvent22 commentedRedid 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.
Comment #3
Souvent22 CreditAttribution: Souvent22 commentedThis is the update.inc for the database cahnges that goes along with this patch.
Comment #4
Souvent22 CreditAttribution: Souvent22 commentedUpdated patch.
Comment #5
Souvent22 CreditAttribution: Souvent22 commentedUpdated database patch.
Comment #6
Cvbge CreditAttribution: Cvbge commentedHi,
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..)
Comment #7
Souvent22 CreditAttribution: Souvent22 commentedOk, 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.
Comment #8
m3avrck CreditAttribution: m3avrck commentedCheck your database updates, the UNIQUE fields don't look right syntactically.
Comment #9
Souvent22 CreditAttribution: Souvent22 commentedOk, i apologize for the last upload. Pleasle ignore. I send the wrong file. :-[. Anyway, here's a tested, tried, and correct patch.
Comment #10
m3avrck CreditAttribution: m3avrck commented+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!
Comment #11
Cvbge CreditAttribution: Cvbge commentedHi, 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:
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.
Comment #12
Souvent22 CreditAttribution: Souvent22 commentedHello. 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.
Comment #13
Souvent22 CreditAttribution: Souvent22 commentedAlso,
I created a temp name for my temp table. However, for prefix purposes, should I enclose it? Example:
or
Thanks.
Comment #14
Souvent22 CreditAttribution: Souvent22 commentedClarification:
or
Comment #15
Cvbge CreditAttribution: Cvbge commentedYes, 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.
Comment #16
Souvent22 CreditAttribution: Souvent22 commentedThanks. 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. :).
Comment #17
Souvent22 CreditAttribution: Souvent22 commentedGot the postgres correct this time. No temp table needed.
Comment #18
Souvent22 CreditAttribution: Souvent22 commentedpatch re-roll due to head change.
Comment #19
chx CreditAttribution: chx commentedDoes this merit a DB change? what about passing md5(subject) instead? or is that too hackish? seems to be lot simpler to me...
Comment #20
m3avrck CreditAttribution: m3avrck commentedchx, md5() would work but that does seem a bit hackish ;)
Comment #21
Cvbge CreditAttribution: Cvbge commentedIt's
case 'pgsql'
notcase "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 bedb_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.
Comment #22
Cvbge CreditAttribution: Cvbge commentedAbout 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 :))
Comment #23
Dries CreditAttribution: Dries commentedPatch looks good but needs a bit of work:
1. You don't need to urlencode numeric IDs: urlencode($category->cid)).
2. You can write %d rather than '%d' in queries.
Except for that (and without any testing), the code looks good. This change can go in after the code freeze; it's a bugfix so you might want to focus on other stuff first.
Comment #24
Souvent22 CreditAttribution: Souvent22 commentedPatch re-rolled against current HEAD.
Comment #25
Souvent22 CreditAttribution: Souvent22 commentedRe-roll.
Comment #26
Dries CreditAttribution: Dries commentedCommitted to HEAD. Thanks.
Comment #27
Cvbge CreditAttribution: Cvbge commentedFixes for the postgres part.
I had to change the db_{add,change}_column to be able to add default values which are not simply text values, but it's trivial change ;)
Comment #28
Cvbge CreditAttribution: Cvbge commentedMy patch should also fix general update error reported in http://drupal.org/node/36571
Comment #29
Dries CreditAttribution: Dries commentedCommitted to HEAD. Thanks.
Comment #30
Cvbge CreditAttribution: Cvbge commentedComment #31
(not verified) CreditAttribution: commented