I just figured out why there's a separate messages table (because the same message can be in the index multiple times for multiple recipients), but is it necessary to have the message author in the message table? At what point would there be multiple authors for a single message?

Comments

litwol’s picture

I put it there for query performance reasons. i dont remember exactly what. feel free to modify as you see fit. but i do remember there was an explicit reason behind it.

mrtoner’s picture

We'll see what happens as time goes on...

litwol’s picture

Status: Active » Closed (fixed)
naheemsays’s picture

Status: Closed (fixed) » Active

A (not so radical?) thought - we need to combine the two columns (author, recipient) into one (uid) like it was when litwol first released the drupal 6 dev version.

Cons:
1. There is a row of data for the author in the pm_index table. Ugly.
2. There are a bunch of patched waiting to be finished/applied that this will break.

Pros:
1. It will make checking all the participants in a conversation easier. (currently privatemsg_privatemsg_recipients_alter is almost useless in a threaded conversation - and I am using a work around instead of calling that in the conversations patch.)
2. It will make threading of conversations easier.
3. It will allow making sure the right messages are shown when creating conversation lists.
4. It will make deleting messages from a user's listings much easier.

I think the current batch of ready patches (sans threading - that is not ready) should be applied, the architecture change should be made and then the threading patch rerolled.

Comments?

litwol’s picture

I dont understand. nbz can we meet on IRC to discuss this ?

naheemsays’s picture

Using the proposed schema would simplify this query from th patch in #299491: Put a reply form on the message page

$data = db_query('SELECT recipient as uid FROM {pm_index} WHERE thread_id = %d UNION ALL SELECT author as uid FROM {pm_index} WHERE thread_id = %d', $thread, $thread);

that would be simplified to

  $data = db_query('SELECT DISTINCT(uid) FROM {pm_index} WHERE thread_id = %d', $thread);

and replace the stuff in the *recipient_alter function (renamed to *participant_alter) as the current version is single message centric.

The author on its own would be best to get from the message table.

For showing the conversation the current proposed query is

$conversation = db_query('SELECT DISTINCT(pmi.mid) FROM {pm_index} pmi, {pm_index} pmi1 WHERE pmi.thread_id = pmi1.thread_id AND pmi1.mid = %d ORDER BY pmi.mid ASC', $pmid);

and that makes me uncomfortable - it is making an assumption that a user can read all the messages is a thread (which is generally the case). It works, but not having a combined column means that when extending the functionality, e.g. adding delete data, we need two more columns per entry to take into account (author_del where the user is the author, recipient_del there the user is the recipient... don't ask about the case where the user is neither :P)

Changing to the uid schema, just add a clause to the where statement - and the read all message case will also be simple - only load the message if there is a row for it in the index.

All of this can be done without schema change, but I feel they would be much simpler with it.

naheemsays’s picture

Status: Active » Closed (fixed)

This won't work.

Just realised the the method of getting a list of all the participants of a thread requires that the data is not deleted.

A few other complexities too, and it is probably best to stick with what we got.