user interface

the traditional method of inbox/outbox is outdated. look at gmail for example. the module is coded to be as flexible as possible so creating lists such as 'outbox' will be very easy soon with the help of filter module.

What nbz said is right.

However, there is one issue with the current "All Messages", that gmail makes diffferent and imho "right". Unanswered messages sent by the user shouldn't be displayed by default, it is simply irritating. This is also the reason why we should keep something like "sent messages" or "outbox". However, If we can do it with filter and provide the feature to save filters, it would be fine for me..

Regarding #1: Maybe we can get rid of "Reply to xy" anyway and use something like "Write a reply" or something like this instead?

I tried to work something out....

Like often, at the end, it wasn't as easy as I thougt first... So I ended up extending the query builder to support HAVING syntax.

The patch does hide threads written by the user that have no answers, but they are still displayed under Sent messages. And I also renamed "Send reply to %recipient" to "Write a reply".

The problem with not listing all the names is its possible that some other module is blocking some users from recieving the reply. The list of users who will recieve a reply needs to be listed and shown somehow.

Having a look at Gmail - I think it actually uses a different method, something we could probably consider and do after the api function: Auto tagging.

* When sending a message - for all recipients the thread could be auto tagged with "inbox", but not for the author.

* When replying to a message, if it is to the author of the first message, then it should also be auto tagged with the same term.

The default message listing when the filter module is enabled could use a default "inbox" tag - with a separate option for all messages.

From what I can see, the main requirement for this is allowing the message data to be acted upon after a message has been sent - like pre and post hooks.

Yes, that would work too. And it would mean that it would be possible to remove threads from the inbox :)

But this would also require the filter sub-module (Not that I have anything against it, just wanted to note it)

And it would mean that we need to talk about saving and handling of saved filters...

I'm with michtoen... the more traditional folders interface is far more intuitive, Gmail notwithstanding. I notice that Gmail has not long ago added "move to" which is really a step back toward the folder concept. I don't think tagging and filtering has much a future, but whether it does or not, millions will prefer folders and inboxes for a very long time. Even gmail has an inbox.

This implementation do the trick without changing the core of privatemsgapi, and also not using HAVING at all. It works as an extension module to modify the 'all messages' sql filter.

It can be used by the users requesting 'inbox' flow, while the issue is resolved.

I haven't tested it yet, just a few things I noted while looking through the code:

- Why on earth did you copy the whole assemble function? Just use it, see http://drupal.org/node/369399
- instead of implode, I suggest to use db_placeholders(): http://api.drupal.org/api/function/db_placeholders/6. This generates as many %d as you need and you can simply use $tids as parameter array
- Small typo, your form group is called 'privamte_inbox'

Hi Berdir.. quick answers..

- Why on earth did you copy the whole assemble function?

a) Because on the moon you can call recursively a function without any matter, but on earth if you do you get and endless loop and the request never ends. The modifications are being done within the scope of a assemble call (it's a _sql_ alter function).

b) Because I need to perform a database query using the information of current $fragments, but neither the 'query' or 'count' queries built by the assemble function did the point for me. I need an special query that's exactly a mix of both (using the count part, but selecting the thread_id part), so I should do using a current assemble proccess (the sql: list) to take the arguments, but I can't call assemble again.

- instead of implode, I suggest to use db_placeholders()

I totally agree with you here, but as a fresh breath for the installation in the end I decided to perform the implode directly to avoid more resource consumption.

- Small typo, your form group is called 'privamte_inbox': Yes, I see.. brbrbr quick typing for uploading.

Berdir, I would like to remark this part of my update: "It can be used by the users requesting 'inbox' flow, while the issue is resolved." I made this upload to give people something they can start playing with while the privatemsg team concentrates on resolving what to do with inbox/folders/tags and so, just to improve a little the usability of the module for non skilled people.

Just trying to clarify the gmail workflow.

about functionality

- there's one special tag: 'sent' - it's a virtual tag, id doesn't exists. It's just a view of the messages with a fixed filtering option. This is the reason why it's not inherited by replied messages.
- 'Inbox' is currently a "tagged" folder (any 'inbox' tagged msg is shown here).
- 'trash' is a regular tag: if you reply yourself a trashed msg the reply goes to trash
- 'All mail' is a NOT TAGGED filter (everything tagged 'trash' is hidden here). Show all (untagged) messages and all tagged messages but trash.
- the absence of tag implies the message is archived (not deleted), shown in "All mail"
- when replying a message, the reply is tagged as the original message.

about usability:
- some tags remove other tags: when you tag a message as trash, all the other tags are removed.
- Only the first and latest (or last) participants of the conversation are shown in the message list.
- The subject and a short part of the last message are shown as subject of the message.

This is more or less the gmail behaviour. Now about privatemsg

about functionality:

- the view 'all messages' should be understood as "all conversations I'm involved in", even if nobody answer the conversation.
- Any sent messages is shown in the 'sent' folder

about usability:

- tags are not automated, and also no API is considered yet.
- All the participants are shown in the message list.
- Only the subject is shown in the message list

This is all. And now we can start playing around filtering the queries. The filter (tagging part) module allow users to tag messages and filter later, but filtering "all conversations I'm involved in" or filtering "Sent" views.

In my opinnion current privatemsg status is far from being gmail-ized, with or withouth tags, at least for rc3 or 1.0 release, and privatemsg also is far from previous folder based d5 branch. User should realize that, but also tagged messages are much more usable than folded.

I would preffer the threaded and tagged version much more than the folder based, but I guess gmail-ization is going to take some more work.

Well, the "privatemsg team" is pretty open, so why don't you consider yourself as part of it and do it right instead of providing a workaround :)

Answer to your feedback
Only if you call the same query id twice. You are currently more or less doing this, but this isn't nice, because you are executing a big and slow query twice then.. I am also not sure what you are executing and hiding exactly anyway.. After a first look, it seems that you hide all thread where the user has written in.. this seems to be a bit too much. Also, you have the $account parameter which is the user object of the user for which the list is loaded.

Brainstorming for a better solution:
- Add a default Tag "Inbox" to privatemsg_filter
- Automatically tag all new messages with "Inbox" for all recipients (not the author). This could be configurable
- Provide a way to allow privatemsg_filter to add pages/listings to the /messages page. Most probably only global, not on a user-specific level, because of limitations in Drupal Core. I could think of a wizard-like thing in the admin area, where you can create a filter first and make a menu/local tab item out of it.

I will probably do something like this in the future, but have more urgent issues right now. But I can help and and answer questions if someone wants to do it.

nice double-post, even in the same minute :)

Making privatemsg more gmail-ish is definitly not an easy task and we can't copy everything, but some things would be useful. Also, remember that private messages are *not* (g)mails, there are some big differences

- Tags are for threads, not single messages
- participants are per thread, not per message (atleast in the UI), so it is not possible to add more recipients or remove them.

I cant take on it, and also I'm currently seeding the simplest solution:

Instead of tagging or not, I guess a correct perspective could be: include us in a conversation if we have received at least one message (instead of the current way: include us if we just send or receive any message). As the list query queries any author
involved in a conversation, even if we have received or sent an email in the conversation the user will be included. It's even lighter
than the original one..

removing this part could do the trick

// Also add a record for tha author to the pm_index table - set column "new" to 0.
// db_query($query, $mid, $thread_id, $message['author']->uid, 0);

Berdir? what do you think? Am i missing something?

This would probably convert the "all messages" to an inbox listing, but break everything else.. sent display, thread display, answering, ... Not really easy.. :)

Actually, there's no reason why these functions should break.

Corrected (by testing):

Everything works fine. All the queries perform a DISTINCT discrimination to avoid the 'duplicated' thread involucration caused by participating in a thread several times (sending and receiving), and this discrimination is not affected at all (tested).

The only problem appearing is not showing sender user from the participants list, but it's because it was clearly removed from the list_sent query (to avoid showing ourselfs as recipient even if we are in the conversation):

WHERE pmia.thread_id = pmi.thread_id AND pmia.uid <> %d) AS recipient

Removing also that part of the query will leave 'Al messages' in a more like 'all conversations where I'm participating', what looks for me a clean start point. Using current 'all messages' (including sent messages).

I guess, to the question: Should unanswered sent messages be shown in the 'all messages' view? for you it's yes, for me it's no :?
I'm trying to argue why the answer is no for me but I can't find a reason, well just a light thing about the number of times I appear in a conversation.

- Provide a way to allow privatemsg_filter to add pages/listings to the /messages page. Most probably only global, not on a user-specific level, because of limitations in Drupal Core. I could think of a wizard-like thing in the admin area, where you can create a filter first and make a menu/local tab item out of it.

This.

I am still of the opinion that the current "sent messages" tab needs to be removed from the main module - with potentially privatemsg_filter providing the (configurable?) "subpages" by modifying the list query, but this seems like a tall ask.

Ok, I'm taking on this. I'lll do my best to have finished for the release of the 1.0 version of privatemsg.

steps:

- I'll start with the last patch from the issue: "Get rid of sent messages": http://drupal.org/node/303087#comment-1152804
- define minimal functionality of general purpose (most used) tags:
inbox
sent
trash (before removing),
archive (should we get this one back :?),
- code the minimal functionality in tagged view filtering

do you agree?

I think that messages sent by me without answers should show up in all messages.. but not in a "inbox". Also, I'm not saying that we can't make it work when removing adding the author as recipient, I'm just saying that it would break those pages if we just remove that line and what should we do with already existent data ? We would need to delete all rows int pm_index where pm_index.uid = pm_message.author

About the default tags:

- inbox -> definitly
- sent -> Not sure if we need a tag for that. We know which messages have been sent by a specific user whithout adding a tag. We could instead also make privatemsg_filter aware of that
- delete -> same for that.. we have a deleted attribute for a message. In contrast to above, I'm quite sure we don't need that. We could add a setting to filter for deleted threads (deleted thread = all messages of that thread have been deleted)
- archive -> If it's not tagged with inbox, it's archived.

I'm not sure if we should start with that patch. It may be easier when we start adding features (adding the inbox tag should be easy, just add a message_insert hook in privatemsg_filter) and remove the hardcoded sent messages tab when we can replace it.

ok.. in parts..

I think that messages sent by me without answers should show up in all messages.. but not in a "inbox".

Agree 100%

Also, I'm not saying that we can't make it work when removing adding the author as recipient, I'm just saying that it would break those pages if we just remove that line and what should we do with already existent data ? We would need to delete all rows int pm_index where pm_index.uid = pm_message.author

Agree 100%, of course, commenting that line and changing the filter are not the only required steps, the index should be updated also.

In any case I dropped the idea of having a so clear "inbox like" in privatemsg in favour of having a tagged system where the inbox has a clear space. Also, Nbz stated that sent messages should be removed from privamtemsg code. I take over the filter module, express several ideas about the 'sent' and 'inbox' mechanisms, including other 'common' features to design a rebust filter system, and now you suggest that there's no tag needed for deleted, archive and sent. I'm getting confused with your 'perspective', but I'll consider very hard your ideas.

In my head there's a general idea about separating tag (categorization, even I noted in the tag_notes.txt the possibility of use taxonomy) and filter (searching, a refined filtering system). Of course, the 'archive' is not a category at all, it's the 'all messages' provided by the privatemsg module.

The idea of starting from that patch is because it's related, it's propposed by nbz and it's an open issue, so we can start closing things, but the main reason because I choose that approximation is because neither of both issues (the tags and the sent) are propposed for the rc3. In any case, you are right, let's start then with some assumptions:

- The object base for tags are threads.
- The object base for filtering are messages.

- Tags are to be rendered as local tasks within the messages menu scope.
- Tags are to be rendered in the "message links" block.

- Tags are to be taken from a special category of taxonomy (fixed site tags)
- Tags are to be managed separately (user defined tags)

Having these ideas clear I can get a start point.

do we agree?

I think the current deleted should stay - as it is not per thread, rather per message and IMO that is a good thing.

as for tags - we have a simple implementation in the filter module module already, but these seem to have some short comings:

1. Just filtering can allow tags with a # and other special characters and that does not work (save and filter does, so maybe drop the first, and add a timeout to the seconds session?).
2. Just noticed yesterday and not investigated, but own sent messages do not seem to appear when filtered by tag.

As for tags as menu items - I do not think its a good idea for all tags to be menu items - just some designated "special tags" that can be promoted to such a space.

Inbox can be done with ot without a tag - adding an old fashioned untagged inbox would be a single line addition to the sql alter query. the sent messages as a non-tag would also be adding a special condition to the sql alter query. these can be defined as had sub pages of all messages, or as "special tags" - tags that are promoted to subpages.

I am not the maintainer and I'm not a native speaker. I'm just writing down my personal opinion. If something is unclear, please ask again :). Also, I have pretty much the same opinion as nbz.

Currently, the privatemsg_filter module is optional. If we want to keep it like that, we need to able to do important stuff like deleting messages without it. Delete threads be adding a tag would not allow that anymore.

A "filter" is for me something that you can do with the filter widget on the thread list (which can be extended). This can include one or more tags, specific authors, text-search for title (there is a patch in the queue which allows to search the body too). Future improvements could include display/hide deleted messages and other things.

> - The object base for tags are threads.
Agree.

> - The object base for filtering are messages.
As a datasource, yes. but probably not for the user. The point is that most data, except tags, is saved on message-level. So I would say that filtering threads should operate on thread level but to do this, it needs to consider single messages too. If I want to filter my threads for a specific author or search for a title or even body, it needs to check each message but it should always display threads in the listing, not messages. Again, this is my personal opinion.

I'm going to change your grouping..

> - Tags are to be rendered as local tasks within the messages menu scope.
> - Tags are to be taken from a special category of taxonomy (fixed site tags)
To be able to render tags als local tasks, they *need* to be site-wide as local task definitions are too. As nbz's said, I don't think that a tag should automatically turn up as local task.. you may want to have 20 different tags, but that would completely break the page if they are all displayed as local task. Actually, my idea was not to use tags but a filter for such a local task. What I currently have in mind is a two-step form:

1. First put together your filter.. that could be exact the same form as it's used currently. For example, all threads tagged with inbox. Or all threads where I'm an author. Or all threads wich titles contain xyz, are from author xy and have tag z. Meaning: It's completely flexible.
2: Attach that filtered listing somewhere. I'm not sure how user-friendly this needs to be. The most flexible way and still pretty easy to implement is probably to simply have url-input field, a title, and maybe one could select between local task and a normal menu item maybe a display on block checkbox too... Having these options should give an admin full control on how he wants to have these lstings set up.

We could then provide not only default tags, but also default listings, or however we want to call them. For example, inbox, sent and deleted.

> - Tags are to be managed separately (user defined tags)
> - Tags are to be rendered in the "message links" block.
User defined tags could be another feature. However, it doesn't work together with my above idea nor displaying tags as local task. Because if every user has his own tags, we would need to add *all* as local_task and sort everything with out with access callback. That's not going to work well.. :) But we could display them easily in the links block, as you suggested.

I don't know if you have seen the api issue. It's finally in and took very much time for that, because it was a big and complicated patch that needed to be reviewed very detailed. I don't blame everyone (except myself) that it took so long, but getting a big patch to be commited will need much time, from us and from litwol, the maintainer.
Because of that, I think it would be easier if we could split it up. adding default tags, auto-tagging new messages with inbox, adding the listing mechanism, adding more filter options are all tasks that could be split up and worked on more or less separated. And when we have that in place, we can remove the "core" sent list.

What's the difference between inbox system and gmail system? ;) Gmail does have a inbox...

But yes, I think I unterstand what you mean. If we can implement something similiar as I described above, we would have even more than your "either x or y". My idea would allow to configure in detail which listing displays which messages. You could either have a "classic" inbox/sent/trash system (we could provide that as a default) or something completly different, for example some sort of a workflow system unanswered requests are in one listing and the rest in another, according to tags or whatever.

But it will obviously take some time to create it.

Just one more question about privatemsg ui.

The sql_alter hook allow modules to filter the query adding 'where' statements and also add information to the query by 'select' statements, but default messages list only show subject, participants and last updated information.

Should any 'selected' information by other modules be managed by it's own module template. or should privatemsg take care of those new selected information?

In the tagging system, I'm also selecting the tags of the thread to show them in the message list, but privatemsg doesn't know what to do with them. what do you think should I do?

That is definitly a good question.

That is a point I've already thought about too. The plan is to make exactly this possible in my patch at #348907: Per thread/Multiple thread actions, but I don't know exactly how yet. If you look at the current patch, I simply moved the current theme_privatemsg_message_row and parts of privatemsg_list_messages to a new theme_privatemsg_list_messages function, which is called by the fapi system. With the current implementation, if you want to add or change data of that page, you need to adapt and change the whole theme function.. obviously, that doesn't work for modules.

An Idea I'm currently trying out is to move a) the header definition and b) the foreach loop that builds up the definitive row for the theme_table call outside of the theme function and store the (generated) data in $form. This way, other modules could access that data with hook_form_alter and add rows and change content/order of rows. Then, the only thing that theme_privatemsg_list_messages needs to to is pass that data in the correct order to theme_table().

If I get it working, I'll explain in the mentioned thread how to use it..

Just a few notes to help the overall issue move forward smoother instead of getting stuck on the terminology.

Gmail system and Inbox/folder system have much similarities and very differences:
1) Gmail and inbox system are identican with regards to how users interact with it because it allows people to put things in places other than inbox.
2) Gmail and inbox system are different in user facing TEXT, instead of calling something 'inbox' it calls them 'tag something'.
3) Gmail system is much more flexible than inbox system.

So if it is counter intuitive for people to use gmail system with terminology like 'tag something', then we can very easily provide translatable text (if we are not already) for these terms to convert 'tag soemthing' into 'inbox' terminology. However to make tagging system identical to inbox system is we would have to force a limit of 1 tag per thread. So as you can see, tag system is much more flexible and i'm glad we went with it instead of conventional (outdated?) folders.

@michtoen it would be great if you could go through current privatemsg feature set and outline the road map that should be taken to bring the changes/features you are describing.

Note all the details from renaming link text to feature limitation/modification to full blown missing feature development. and then post them here.

Unfortunately we cant satisfy everyone's need, but what we can do is abstract each feature enough to be able to turn it on/off as per individual site admin's needs.

michtoen: the issue is that most sites have a different need for a private message system. There are sites that want to keep private message explicitly on-site (dating sites, for example) and are not interested to let users communicate via email because they would not be needed anymore..

Our task is to provide a system that is generic and flexible enough the work for as much sites as possible (it's not possible to be perfect for everyone).

Don't get me wrong, I'm not saying that the way how it currently works is perfect or even near of that.

What I'm trying to explain above could also be used for a simple site as you describe it. Instead of archiving a message, users would simply remove the inbox tag by clicking an "Archive" Button and do that without even knowing that they just used a tagging system. That's exactly how it works on gmail.

My development experience about "simple in functionality', 'simple in usability', and 'simple' is that no one has nothing to do with each other.

Google is "simple", just a textbox where you type something and magic happends. Figure out what "simple" in terms of tagging is: have a tag associated to a thread.. but, what a tag is, with or without description, user defined tag, private or public, or just provided by the site, you can use but you can't edit, you can't tag because all gets tagged by default, can I short the tags in the tag view, can be disabled (not deleted) or hidden, and so.. Now, thinking about privatemsg considering: searching, autofiltering, tagging, sorting all that, and also future integration with og or buddies and..

Ok, inbox, sent and archive (or trash) is a common implementation for pm, it's simple as concept, also gmail, hotmail, yahoo, phpbb, vbulletin, all of them have pm systems with the same concept but didn't discard any other available feature.

I guess everyone expect privatemsg to do what they want (I'm the first who contacted litwol to see about a near future of privatemsg for my own), I think it's in the right direction, I'm not sure if this will satisfy everyone, and I don't know when will do.. but if the system is flexible enough, you would be able to select the configuration that fits better for your need.

@berdir (#27)

Moving the 'main view' to a form could help a lot to several tasks to remove that uri args also, but almost the whole api can be removed because of the form submit and validation hooks (possible attached to the main form) changing the $form_state information.

Nodes don't follow the hook_form structure but are flexible enough to allow other modules to operate. Just a few more _alter functions should do the trick. For example, I've considered a lot these two days attaching other information to the messages_view alter hook, missig things I'm considering important, for example the thread Id. Just because a module modifying the thread view should need what thread are they looking at. In the current implementation thinks like that can't be done, because all in the $content variable gets finally rendered in the message view. In the end I get back and read the thread ID from the URI, what I think is wrong, but it's what the privatemsg does, so if the main view does, then other modules could follow the same rule.

I'm missing also a pm array of the whole thread that could be altered (before they got rendered) just to change or read information about each message. But all these things would delay rc3 or final release, so I'm still cautious about blaming for changes.

Things are evolving in the filter stuff and user interface I'm working on. I expect final results really soon (days) covering most of the 'folders' tagging, simple and functionality issues being discussed, so I hope we can get rid of all the 'hardcoded' and 'fixed' stuff in a future way.

Just out of curiosity, why not just port the already excellent 5.x module to D6?
I don't know much about the inner workings, but if a tagging system can be made to look and act just like an inbox/folders system, can't an inbox/folders system look and act just like a tagging system? So why not port the 5.x one and then add some options to make it look taggy like for those who want that?

@aharown07 i apologize for being harsh, but please do the research before voicing opinions. your last comment does little more than introduce fud into the conversation.

none the less to answer your question. Drupal 6 version is a complete ground up rewrite of the privatemsg system. none of the drupal 5 code originaly was used. more so, d5 code is incompatible with d6 code, so all the new (or old features) from d5 will need to be completely rewritten for drupal 6 version anyway.

Also please take the time to understand the bottom line is that tagging and folders are virtually the same, except that tagging is more flexible for the user and the developer. I've said this already but, tags can easily simulate folders through clever user interface redesign (copy changes), but folders cannot simulate tags.

My "last comment" was a series of questions. The research I did prior to asking them indicated that it is possible to port D5 modules to D6 and that this is not the same thing as a ground up rewrite. I also already knew that the the D6 version is a ground up rewrite. My curiosity had to do w/ why,... I thought there might some reason porting the D5 one is impossible.
In any case, I believe I now know everything I needed to know about this project.

The code would still have needed to be refactored and updated to allow additional features - and that would have been painful IMO. THere was an attempt by a few people to port the drupal 6 module but it was unsuccessful.

IMO the Drupal 6 version of the module is much better than the drupal 5 version.

Sorry if this kind of comments bother you.

To reduce the pain on the merge, I'm trying to avoid any changes in privatemsg.module, and I'm limiting my scope to the filter module at all (because I haven't patched it yet). So, I'm not going to open a request for this, but I've found this line to be 'not correct' in privamtemsg count query (line 727 of privatemsg.module in my local copy):

$fragments['select'][] = 'COUNT(DISTINCT thread_id) as unread_count';

should be

$fragments['select'][] = 'COUNT(DISTINCT pmi.thread_id) as unread_count';

in order to be able to inner join other tables containing a field with the same name.

I'm also preparing a summary of filtering | tagging | menu routing status and concepts, just to avoid user-developer confusion. As an example:

- What we (developers) call the "all messages" view (the root view for filtering purposes) is indeed the "All (undeleted) messages" view. Deleted messages are kept in database but not considered for the module as an option for the user. To avoid this kind of cofusion, we setup a hidden layer, where we let the users delete a message untagging it, not deleting at all.. We need to explain that a little bit more..

- When we search something in "all messages", we are searching "All (undeleted) messages", but user will expect to include 'deleted' also (maybe to find a lost message). The filtering option 'deleted' is not the 'deleted flag' of the pm_message table, users don't need to access the delete option. BUT, and this is a great but, the option in the message view for 'Delete this message' is an user option, and user can play with the 'delete' field of pm_message table totally free, what I think will lead to confusion about what a user expects to happend when a message is deleted, and what will actually happend when he 'deletes' a message or thread.

- When we filter using a tag, we are also talking about undeled messages. Filtering by a tag can (or can't) be done with an inner join. I mean, making an inner join the search query removes any untagged (just because it's an inner join), so the inner should be changed to 'left join' to allow searching tagged and untagged: (subject is test and tag is not inbox) tag is NOT inbox mean any non tagged messages and any tagged message but those with inbox label.

..

There are more 'confusing' topics but these are some valid examples of what I would like to share with you regarding UI and searching/filtering/tagging/blo :)

Update of this issue status:

I'm touching only information related to current '_filter' module.

I've stated in previous posts I was going to review and modify the patches and also probably the module itself. I've done. This post is a resume of current functionality of

_filter modifications.

Assumptions:

- privatemsg module handles the main listing and api operations, including just one view: "all messages" (stated earlier as all undeleted messages).
- No searching or filtering or nothing else should remain in the module (removing sent messages tab is a parallel goal to this one).

- feature requests about UI includes the ability to keep 'folder' system functionality, discussed by berdir, nbz, and litwol as tagging is more flexible.
- Linking or making local_task tabs in privatemsg list view should be configurable in some way (including or not Sent, or other features: tags?).

From here, these are the following achievements:

Separated concepts for quick code review:
- searching / filtering .. is the same, just filter the main list view
- tags .. introduce information in a thread, it's a user interface feature improving usability, but not functionality.
- message routing: what to do when a message arrives, or is sent or .. (applying filtering strings to message api)
- on site / system configuration: Lets site administrators configure the private message system options, like menues, permissions and so..

Ok, now the implementation.. lets start with the filter / search functionality

- Introduced the 'readable' query string in the filtering system. You can now filter messages using human readable query strings like: "from is admin and subject is coco and status is unread".
- OR searches can't be done because of the inner join operations. Some OR searches require a Left join, others no, and the search scope should be yet decided (NOT tagged as Inbox means 1) tagged, but not inbox, or 2) tagged or not, the tag should not be inbox).
- created an API to allow modules to cooperate in the filtering process: set filters and complete filter strings, this way, the attachments module can add options to the filter query: has attachments, size is bigger than.. etc.. . The information is used to filter the "all messages view".
- Removed the filtering string from the URI, now it's session based, more clean and errorless. The use of pager information was incorrect as it supposed to be only one pager in the same view. So now, you get the same results in a query using "messages/search/status is unread" than the previous "messages/?read=0
- query strings can also have placeholders (some user placeholders as !username, !email, (proposed system time) and so) , so you can create a query string using !username or !time. This way you can create a filter of style: "Last week messages", and current time will be inserted as part of the filter.
- Ability to keep search filters in database (also in human readable), so users may define their own searches.
- Administrator may define system searches (it's provided to the user on registration) with the same flexilibity (if you set the system to create a search like "Sent = from is !username", when the search engine builds the filter the placeholder will be replaced with the username
- Added two blocks, a human styled search engine (it's, a textfield where the user enters the filter string : "content is bills and from is bella and status is unread", including a "quicksave search" option. The other block is a searches list for quick access, configurable to hide some of the searches (example, site defined searches).
- The option for the system administration to 'protect' some of the searches, to avoid users creating-editing-deleting them.
- Search filter keeps working even if moving in the site, so multiple-filtering is enabled by default. If other module interacts with privatemsg, the current search filter will be added to other's.
- Created two menu callback handlers to allow administrators to create menu entries in navigation menu. Now it's possible to create filtering search options using the admin/build/menu to support for example "Sent messages" => "messages/search/from is !username" or using a saved search: "All Unread" => "messages/filter/Unread" (a filter Unread should exists).
- unread_count also follows the same filtering as the list view.
- the module creates messages/searches as the user settings for the searching options (delete, add and so)

This situation is the minimal required to remove the 'sent' stuff from and allow some site configuration flexibility.

Just an example setup could be:

the site defines the searches:
- Sent: from is !username
- Unread: status is unread

The site protect these two searches to avoid user menu hacking
the admin sets two new menu entries in the navigation menu (at admin/build/menu) under the messages menu pointing to:
- "messages/sent" => messages/filter/Sent
- "messages/unread" => messages/filter/Unread

The site enables the "user searches block" and sets Sent and Unread as hidden searches. From this point, Sent and Unread will work as system searches instead of user searches, and users can use them as provided by the privatemsg system. The filtering mechanism is still attached to the list view as it is in the current filter module.

Second task: the tagging system.

tagging follow almost the same rules as filtering in user/site functionality:
- site administrator can set tags to be created for every registered user.
- site administrator can protect tags so users will not be able to edit-create-remove them from their settings
- site administrator can hide tags in the quickaccess tags block some tags.

- Users may (may means if has permissions) now create tags while viewing a thread, the thread will be tagged on submit.
- the module manages "messages/tag/tagname" uri, so there's no need to include filtering code to 'folder' the privatemsg system.
when the uri includes tag/tagname, the module sets a filter to privatemsg list view and unread count queries.
- with "messages/tag/tagname" the administrator can now create links in the navigation menu pointing to user tags.

- the module creates messages/tags as the user settings for the tagging options (delete, add and so)

- the module can work with or without searching engine. If working with:
- tagging code integrates with filtering in the search engine to allow search filter strings as "status is unread and tag is tagname".
- the two filters can work together: tag filters about tag, and search filters about unread, for example.

Just an example setup could be:

the site defines the tags:
- Inbox
- Spam

The site protect these two tags to avoid user menu hacking
the admin sets two new menu entries in the navigation menu (at admin/build/menu) under the messages menu pointing to:
- "messages/Inbox" => messages/tag/inbox
- "messages/Spam" => messages/tag/Spam

Now, the routing operations:

Not being implemented yet, this is what I had in mind:

- serve as main center for message routing for a site, instead of leaving other modules to operate with the API directly:

- set permissions to configure the following routing:
- a message is received
- a message is sent
- a message is answered
- a message is read
- a message is deleted

This is the minimal set.

- Now, figure a normal message system, user may define what to do with incoming messages (a message is received) but not configure the other routes.
- The route definition for each of the previous routes could be a form like this:

- Mark as read
- Mark as unread
- Delete it
- Undelete it
- Apply the TAG:
- Remove the TAG:
- Forward it to:

This is the same form to be shown on user filtering operations (per thread actions code), but it's going to happend programatically when a message is "sent" or is "received" or is.. Using the same code or concept as "thread action" will allow some code reutilization for the following feature: applying filters. Using the search/filtering engine, allow the user to configure actions to happend when a "message is received".

The search includes all the information about the message filter criteria, if the received message matchs the filter then apply the defined action. This will be the final stage of user interaction of the module. Mixing actions and searches is a powerfull tool, and we have both, searches and actions.

I've read in some posts about automatic tagging on form submit, what I think it's wrong, I would preffer much more this routing system, so I'm right now working on it. The implementation will cover the 'autofolder' behaviour expected by all users. hacking the form would not care about the filtering status, It may be good for "received go to inbox allways" configuration, but will not cover many cases.

And finally, the site customization, the menu operations.

Finally I realized that the menu editor is not powerfull enough to control the whole kind of "generic configurations" for privatemsg systems, as it's based on hook_menu_link* API. It fits perfectly to set 'fixed' menu entries, like "Mark as read" or "Move to trash" for example, so many of the menus for the private message system can be handled using the builtin's drupal menu editor (if you have a path, example: "messages/search/status is unread" , then you can link to that path with a more readable title: "Go to unread"..

The problem with hook_menu_link API is about the routing information. _link manages callgates to real menu routes, it's: you need the real path existing in the routing table.

Also, we should forget about LOCAL_TASK docking or dynamic titles like those including unread count information and so. For this a more powerfull menu editing system is on the way, not having nothing to do with privatemsg just because it's a "generic menu editor", but can be used to finish the configuration of many user interfaces in several kind of privatemsg applications.

One (probably the first) action to be done in 'folder' based installations is to remove the "All messages" menu and set the "Inbox" entry (read #351542: user interface #comment-1270194 to understand what I'm talking about) , but "All messages" should be kept as the "Archive" view. The second thing is to check "why on earth" the menu entry doesn't show the unread count messages in inbox. Te answer is: the current menu tools doesn't allow that, so I'm conding an improving menu editor to handle some of the caveats for this. Even if the menu editor is NOT a privatemsg intended module, I think it could finish the stage about private message system configuration flexibility other users would expect.

This is more or less the current status. A demo? there's a demo.. but you know, having several persons using the same user can be confusing (messages/searches/tags appearing and dissapearing). Optionally you may create an account if you want.

- there's no autotagging, or any other automatic features. The version installed is an unpatched just downloaded 1.x-dev.
- Tagging can be done by hand only.
- searches and tags filter together, I mean, if you filter "tag is tag1" using the form and then you go (using the tags block) to tag2, you will see no messages.

- System created and protected the tags: Inbox and Trash, and also creates the tag1 and tag2 for the users to test.
- system hides Inbox from tags block.
- users may add any tag.

- System created and protected the searches: "Sent" => from is !username" and "Unread = status is unread", and protect them.
- system hides Sent search from searches box.
- users may add/edit any search.

I created (manually) two navigation menues:
- Sent, goes to search: status is unread, child of "messages" menu
- Inbox, filters by tag: Inbox (messages/tag/inbox)

The filtering is done from "all messages" list as is in -dev version, it's All undeleted messages..

the site: http://pm.drtsg.net
user 1: demo1:demo1
user 2: demo2:demo2

arfm.. edit: formating.. Now it's to late for me now XD

Ok, I'm trying to give some feedback, it's a very long text. Again, most of the following is my personal opinion..

First: I'm trying out the demo right now, looks good, very interesting work..

Second: You're not using the latest dev version. I really suggest to upgrade, so that the hooks etc. are available. There were also some ui improvements. Also, the sql hooks were renamed, check the documentation at http://drupal.org/node/369399.

Third: I'd really like to see your code too... a) to see how it's implemented and b) to test the admin side.

Assumptions: ...

Sounds good.

- message routing: what to do when a message arrives, or is sent or .. (applying filtering strings to message api)

I can't really follow you on the routing stuff.. more on that later on

- Removed the filtering string from the URI, now it's session based, more clean and errorless. The use of pager information was incorrect as it supposed to be only one pager in the same view. So now, you get the same results in a query using "messages/search/status is unread" than the previous "messages/?read=0

The current filter is able to do both. The downside of the session way is that you can't bookmark the search, for example (I'm using bookmarks on d.o for typical searches I do). Imho "messags/search/status is unread" seems to save that in the session and forwards to /messages. That is problematic because it seems as it wouldn't work and if I click somewhere else (for example Inbox) I actually have both.. tag is Inbox and is unread. Suggestion: If the URL is "messages/search/something". parse that directly and use it only for that URL. This is the same for folder-like systems and gmail.

- Administrator may define system searches (it's provided to the user on registration) with the same flexilibity (if you set the system to create a search like "Sent = from is !username", when the search engine builds the filter the placeholder will be replaced with the username

"provided on registration".. What happens if the admin adds site filters or changes the current ones?. Unfortunatly, I can't look at your code, but I would suggest to separate site and user filters. Maybe you can simply make the uid field in the "stored filter" table optional and if uid = NULL then it's a site filter. Same for tags, because we can only create menu items for sitewide tags/filters.

... configurable to hide some of the searches (example, site defined searches).
- The option for the system administration to 'protect' some of the searches, to avoid users creating-editing-deleting them.

If you go with my uid = NULL suggestion, you don't need to handle this explicitly, users will simply not see those.

- Search filter keeps working even if moving in the site, so multiple-filtering is enabled by default. If other module interacts with privatemsg, the current search filter will be added to other's.

As written above, I don't think that's a good Idea. If I am a user and click on Sent (from !username) and then on inbox (tag is inbox), the list would be empty because I doesn't have any inbox message from himself.

- Created two menu callback handlers to allow administrators to create menu entries in navigation menu. Now it's possible to create filtering search options using the admin/build/menu to support for example "Sent messages" => "messages/search/from is !username" or using a saved search: "All Unread" => "messages/filter/Unread" (a filter Unread should exists).

Do we really need the first one? We could also save a filter for "from is !username" as "sent". and use "messages/filter/sent". It's a step more, but the URL of the first is imho ugly and unpredictable (whitespace and so on, what happens with special characters.. ). Because of that, I would also suggest to limit the saved filters names to [a-zA-Z_-] or something similiar.

- unread_count also follows the same filtering as the list view.

This is interesting, wanted to suggest that anyway. Keep in mind that currently the unread_count query is only executed once per site, we may need to think about that. Maybe adding some sort of a "key" for different unread_count queries, so that privatemsg_filter could act on these. We could then add that to all privatemsg menu items. But this would probably mean that we have to add a persistent cache on unread_count to limit the number of queries.

- the module creates messages/searches as the user settings for the searching options (delete, add and so)

Can't follow you on that one..

- Users may (may means if has permissions) now create tags while viewing a thread, the thread will be tagged on submit.

I like that. Maybe we could make the textfield an autocomplete-thing and allow multiple tags. See user autocomplete on how to do this.

- the module manages "messages/tag/tagname" uri, so there's no need to include filtering code to 'folder' the privatemsg system.
when the uri includes tag/tagname, the module sets a filter to privatemsg list view and unread count queries.
- with "messages/tag/tagname" the administrator can now create links in the navigation menu pointing to user tags

We can seach for "tag is xy", do we really need messages/tag/xy ? As soon as something needs to be a bit more complex (for example two tags, or tag + search) we need to create a filter anyway. I agree on making it simple for the administrator, but maybe there is a way to have a simple UI while using a filter in the background. While reading forward, it seems that filter and tag is a separated module, is that correct? Then I can see the reason..

Now, the routing operations:

I can't follow you on that one, not sure what you are talking about. Some generic hints:

- privatemsg.module should handle the message sending/recieving stuff
- it provides different hooks, so that other modules can add their stuff.. That includes validating, before/after sending, deleting and so on. See http://drupal.org/node/369399
- Did you see my privatemsg_rules.module at #327938: Rules Integration?. Currently, it supports only the event "message sent" and the actions "send message" and "answer thread". I'm open for suggestions to add more things (validating, tag stuff, ..). I think we can simply improve that and use the Features that Rules provides instead of re-inventing the wheel.

Finally I realized that the menu editor is not powerfull enough to control the whole kind of "generic configurations" for privatemsg systems...

Unfortunately, yes. Imho, we would need to have atleast the following possibilities
- URL: defines where the listing/filter is located.
- Menu type: Either local task or a normal item, should be enough.. maybe default local task too
- a title callback function (for unread): Probably a simple on/off checkbox for the unread callback, maybe even a field for another one. Hint: the title callback has currently Messages hardcoded, we may need to change that.
- Title/Name and description
- Filter: Select which filter belongs to that menu item.

We can store that information in a table and have a dynamic hook_menu then, which adds the menu items.

Some other feedback:
- Imho, the user search field should be an autocomplete. The easy way would be to use the autocomplete of privatemsg.module, it would however display all users, not only those who participated in our messages. Maybe use a link like messages/user-name-autocomplete/filter and if "filter" is there, the filter module can add restrictions to the list.
- The human-readable search stuff is really nice, I'd like to see how flexible the implementation is => /me wants code ;)
- The placement of the form elements is a bit.. uncommon, for example the save seach fieldset. Also, it looks as if the buttons are only related to the first textfield, which is not true. But I like that they are on the side and not below, that saves space. Maybe group the textfields in a fieldset and the button in another. The save search fieldset may also look better then.
- From what I can see, this looks like really much new code. Before this can be included in privatemsg, we need to review that in detail and test it. So it will take same time to get it in.

Thanks for your time berdir. Apologies because english is not my natural language also, so I guess I could be confusing sometimes.

Just short replies this time.. (I hope!)

about code issues: I remember I just downloaded the last version of the module before installing, maybe I did the upload wrong, I'll check. Give me sometime to do code cleanups (that's the main reason I didn't attached in the comment) and to update hooks.

Imho "messags/search/status is unread" seems to save that in the session and forwards to /messages. That is problematic because it seems as it wouldn't work and if I click somewhere else (for example Inbox) I actually have both.. tag is Inbox and is unread.

In the comment I talked about this as the intended behavour. The reason why it redirects is because the site links may confuse for testing, but the redirection was hardcoded in the last second, and is not in the main module workflow. Also, the module was instructed to not remove the filter query even if leaving the "messages/" section of the site, so you can go/back to other site places but the filter keeps active (usefull when you want to set a default filter). Currently the filter works anywhere within the messages/ menu folder. Why? because this implementation of filter allow other modules to set their own filters (for example, the tag part) to don't disrupt their link/menu options. The site was configured thinking about several posibilities of modules interacting with the main sql query, if tags code (tags or whatever module setting it's own filter) sets it's own attach, it's because was expected by the site administrator. (More about the filtering code later)

About searches and tags provided at registration time.. hehe.. I commented in a previous comment about separating them without answer, so I decided to go my own. These tags/searches, provided at registration not neccesarely are system tags, they are a prepopulation of "common searches" or "common tags", and they will be used mainly by the user. Making them "system tags" means, the user can't operate on them, but user can USE them. Optionally if the admin want's to "hide" (hide is not protect) any tag/search in the menues there's an option in the block settings. I considered a lot having uid = 0 for site searches/tags, but I figured some crazy admin making a feature request to allow anonymous users to search threads (not reply or participate). As in some point I decide to include placeholders for the filtering options, I realized that some fo them could be special depending on the user, and Finally I discarded because of this and just to reduce filtering times and sql logic in the filters. Current implementation creates the filters for every user and keeps 'system' searches and tags protected (you will see when you got the code). Note: updating system tags is "update {pm_tags} set tag = '%s' where tag = '%s'. That will update a system tags for all users, it's not a trouble even of you get a 3k affected rows result.

More quoting:

- Search filter keeps working even if moving in the site, so multiple-filtering is enabled by default. If other module interacts with privatemsg, the current search filter will be added to other's.

As written above, I don't think that's a good Idea. If I am a user and click on Sent (from !username) and then on inbox (tag is inbox), the list would be empty because I doesn't have any inbox message from himself.

Again it was intended in the installation. Also, the site was setup with several menu example types to see how it works, not as a final usable site. I've created different menu types (one for search/query other for filter/name and so). The tag code filters if the url has a tag/tagname string. So, I've decided to keep both in this installation to see it's results, not as the "best usable site". It's important to remark here that the filtering module may use some information of the tags code to filter by tag, so the site administration can disable the filtering being done by the tag code, and leave filtering as the only filtering method, but to be honest, I separated filter and tag code as two different modules. In fact, tag just add information to the thread, and there is at least one site that will disable searching in privatemsg but need tagging: my setup. If I only install the tags module, I still need a way to filter by tag if I don't install the filter module, so that's why tag module do it's own filtering. The key is that this can be disabled superseed by filtering module, and also the tag filtering is integrated in the filtering module at all with this kind of workflow:

1º most of the code was ripped from user module (the part of admin/user/user and the filtering engine).

When building the filter form (not the block search form), the filtering module define some filter options by default:
status is unread
author is XXXXXX

This is the code of the default filters:

  $filters['subject'] = array(
            'title' => 'subject',
            'where' => "pm.subject LIKE '%s'",
            'inner_join' => '',
  );
  $filters['from'] = array(
            'title' => 'from',
            'where' => "pmu.name LIKE '%s'",
            'inner_join' => 'INNER JOIN {users} pmu ON pmu.uid = pm.author',
//          'autocomplete' => 'messages/user-name-autocomplete',
  );
  $filters['status'] = array(
            'title' => 'status',
            'where' => "pmi.is_new = %d",
            'inner_join' => '',
            'options' => array(
              '1' => 'unread',
              '0' => 'read'
            ),

Note: the autocomplete part was previously set, but the privatemsg autocomplete includes a ',' I don't want to remove for now.

If the filter has 'options', then it's rendered as select, otherwise as textfield.

Then, a drupal_alter of ('get_user_filters') is requested so other modules can define other filter options. In the case of tags code:

/*
 * Implementation of hook_get_user_filters_alter
 * 
 * reply with a filter for tags.
 * Define own TAGS and query other modules to alter them..
 */
function privatemsg_tags_get_user_filters_alter(&$filters, $account){
// Default module filters
  $options = _privatemsg_tags_user_tags($account);
  if(count($options)){
    $filters['tag'] = array(
      'title' => 'tag',
      'where' => "pmti.tag_id = %d",
      'inner_join' => 'INNER JOIN {pm_tags_index} pmti ON pmti.thread_id = pmi.thread_id',
      'options' => $options,
    );
  }
}

Note: in this code, the module also query other modules to alter the default tags list. _privatemsg_tags_user_tags returns an array of [tag_id]=> [tag_name] form. This is how tags module integrates with filtering, it's all the code needed.

With this information the form is rendered, and if the tag option of the form is selected, then the filter will consider TAGS as filtering string. Note that it's the same format of the privatemsg sql alteration flow, but with the name (of the form element) and the 'title' to be shown.

A filter form submit with the tag information will set $form_state['values'] = tag_id to be used as $fragments['query_args']. Then there's a loop in the filters array (get_user_filters) to check for active setting, and apply the sql information of 'where' 'inner_join' and so.

With the title and the option, you can build a 'human readable' string, and with the 'name' and the $form_value result you build the real query. Doing the opposite way is easy: you got "status is unread", then look in the filters array for the option with 'title' = "status", and if it has options, then check if the 'unread' is a valid option, otherwise ignore the query string. If no " is " is found in the query, than the search string is used with the first filter option of the array, and by default is 'subject' (could be content: subject and body).

That's how the human readable filter string works.

The main part of the filtering also takes care of not doing "double joins" if an option is set twice (where tag is XX and tag is YY).

it seems that filter and tag is a separated module, is that correct? Then I can see the reason..

As you see, you where right.

I'll reply later about routing and menu, I don't have time now.

Ok. Just some very short (I hope.. :)) comments. I'd like to look at the code before discussing more about it.

- Please create new issues when you encounter privatemsg.module shortcomings. (for example the missing table alias) This issue gets longer and longer, and I really doubt that more than 2-3 people will read our endless comments :). You don't need to provide a patch, just describe what should be changed in your opinion.

- I'm fine with having tags and filter as separated modules, especially if integration is as simple as you posted.

- About anonymous users. I really doubt that will happen, litwol recently commited a patch that does use privatemsg_user_access instead of user_access, this function simply hard-codes a anonymous check and excludes them. privatemsg is not intended for anonymous users and will probably not work correctly for them. And NULL !== 0.

- About the url/session stuff. I would suggest a multi-layer system. Not sure how exactly, maybe something like:
1. Basic: these filters are always used. For example "is deleted" or however you plan to implement that. System-Level.
2. Filter Form: Filters added manually with the form. Stored in the session. User-Level.
3. URL: Depend on the URL that is openend, for example messages/filter/something. URL-Level.
Each Layer can overwrite the previous and is only used when appropriate.

- Last point: If possible, we should try to split this up. for example, handle complex menu creation, routing and stuff in a later patch. The bigger your patch is, the harder will it be to get it commited.

@ilo, Thank you for all the hard work. I really like what i see in the demo site. few words of caution. Please split the whole effort into multiple smaller issues. If this issue continues the way its now and we end up with one huge patch that will take weeks before we get anywhere close to having it committed.

smaller patches are easier to review and they conflict much less. unfortunately you are using a ver outdated version of -dev release because since then there have been many excelent patches that were committed that clean up API and introduce many improvements.

Your first priority task should be bring your code base up to date with the latest -dev release. try to use CVS check out (let me know if you need help with that, i will gladly walk you through the setup process).
Next priority is to split up your whole effort into multiple smaller tasks that are easier to follow individually. that will not only help everyone else follow the progress easier but will help me be able to review the patch faster.

Since you are not the only person that is contributing to this project, it is very important we all follow few basic rules that helps us all avoid conflicting with other developers' efforts. Therefore smaller and more focused patches are advised when changing large features.

I am easily accessible by email or even better by IRC in #drupal-games channel on irc.freenode.net

Ok, just two notes before continuing.. First thanks to all for your time and attention.

My intention was to split the overall issue into four main branches (filter, search, routing, and menues) and then create several issues for each, but I used the thread to comment the whole 'world is changing' process and ideas. I'm mainly using this thread to update my own knowledge about the module Idea and to avoid working in the wrong direction, and the thread was really nice for that just because of the title, and who knows, maybe this also helps other developers to get a clear idea.

You are right, I will create the issues as soon as my mind is ready to understand what you think is "good direction" and focus mine to what I think is "good direction", and it's almost very very close.

By the way, I was using the DRUPAL-6--1 branch of the cvs. For the site I downloaded this release: http://drupal.org/node/268824, it's called 6.0-1.x-dev version but seems to be outdated. meeec!

The reason why I didn't upload any code was to avoid making more use of your time yet as I'm not sure if I understand the whole idea. Please, remember I'm too young in this module (just 4 days).

Ah, and berdir, I totally agree with the url/session stuff! Firsrt priority task: giving you code :)

@michtoen
Thanks for your feedback!

I'm going to set this issue to fixed, now that we have a inbox system. Of course, feedback, ideas etc. are always very welcome, but I suggest to start new issues or commenting on the already existing, more specific issues. See also #372201-13: Roadmap for official 1.0 release for a list of issues we are currently working on. (comment in the linked issues, not directly there).