Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

+1 from me, our private message table is currently close to 10mb.

catch’s picture

Version: 4.7.x-1.x-dev » 6.x-1.x-dev

Just found this by accident - still a good idea I think.

NaheemSays’s picture

Version: 6.x-1.x-dev »

Moving to HEAD.

Saying that, I have never know a user who is happy to lose old messages, or be faced with a restriction notice saying the user cannot send more messages.

Anonymous’s picture

Message quota functionality is a great idea but is something that should be worked on later after we have the core features working and officially released.

-Paul

catch’s picture

nbz, it should be configurable of course - I have some users with thousands of messages just to themselves, and these are the same people who managed quite well with a 2mb hotmail quota for years. I just went to the last page of my issue queue and found this old feature request by accident, have a much bigger server now, so it's less of a concern.

Berdir’s picture

Status: Active » Needs review
FileSize
3.5 KB

Attached is a first working version. It should work but is not yet finished, I'm especially unhappy about the wording of the admin screen and the error messages (as always ;) ). For, example, how should we refer to the messages a user has? That part is still a bit a mess, the code should however should be pretty clean.

Features:
- Can be configured to count either threads or messages. Threads means that a user can still answer/recieve responses in existing threads but he can't create new threads when the limit is reached.
- Send quota: Allows sending limits per time (for example, a maximum of 5 messages in 5 minutes). The defined time is always considered as "backwards from now", what means that a user could write (with the previous example) a message per minute endlessly.
- Recieve quota: Allows to limit the amount of messages/threads a user has. If the maximum is reached, the user can't recieve or send messages/threads anymore.
- Allows rule-based exceptions for both quotas, the highest value a user has in his roles is considered.
- Displays a short info at the end of the list which tells a user how much of his quota is used. Format of the hook is already converted to use form_alter, as changed in #348907: Per thread/Multiple thread actions.
- If the recieve quota is reached, a dsm error message is displayed in the message list.
- If a message is sent to a user which recieve quota is reached, a warning message is displayed and the user is removed from the recipient list. If no valid users are found after the check, the message is not sent at all.

Current issues:
- We (as in I ;) ) need to find a better way to pass validation messages and stuff around. Currently, this modules displays a dsm warning in the validate hook, which is not good. See #375999: #288183 followup: Developer friendlify privatemsg_new_thread API.
- Relies on [#348909] for the stuff that is displayed in the list, should work except of that
- Bad wording and naming conventions (quota, threshold, amount, some_other_insane_word_I_used, ..)

Berdir’s picture

FileSize
14.81 KB
34.22 KB
39.88 KB

Some screenshots.

No, I'm not going to make a video, it isn't fancy enough :)

NaheemSays’s picture

Looking at the second image, should be send/recieve, not just send.

NaheemSays’s picture

Just wondering over how the blocking is done in this message via the validate function and the separate hook_privatemsg_block_message that is used in the pm_block_user.

Do we need to keep both methods? Is one recommended over the other?

(I can see a problem with multiple dsm's about the author exceeding the quota when using the hook...)

NaheemSays’s picture

A additional thought:

The rate of sending new messages (best to limit 1 message per x seconds instead of x*5 of x* 5 seconds - simpler to understand for the end user) and also the max number of recipients per message should IMO be controllable in the main module and not rely on a submodule.

This would also allow the quota module to just be a quota module.

Berdir’s picture

Hm..

Actually, I never thought about doing it with the block hook, I just started with validate, I didn't connect it with block at all. And, did you see #376023: #288183 followup: change hook_privatemsg_block_message to work on multiple recipients already? litwol asked if we can replace the current block hook with _validate, so that modules implementing it could save some queries by checking all recipients at once.

Downside is, with the current implementation, it needs a little trick to remove the recipients from the list (by saving the id's in a global variable and deleting them in presave_alter.

I don't really have an opinion on that, if we keep the block hook, I'm fine with moving the recipients part of the validation to the block hook.

litwol’s picture

privatemsg_flood_control? i think its out of scope of the core module.

Berdir’s picture

@nbz
No need to put that in the main module imho.

What about using 1 as the default value to make it easier to understand ? I like the more flexible approach more, just because I'm sending 2-3 messages in a short time doesn't mean I'm a spammer, or trying to flood the system ;). Technically, there is no difference between the 1 vs n messages test.. the module simply counts the messages there were sent in the last x seconds by that users.

As I said above, the wording is bad, maybe there is a better way to explain it.

TaPes’s picture

Version: » 5.x-3.0
Category: feature » support
Status: Needs review » Active

Message quota for drupal 5?

Berdir’s picture

Version: 5.x-3.0 » 6.x-1.x-dev
Category: support » feature
Status: Active » Needs review

Please don't change existing issues, it is very unlikely that any active development for 5.x will be done.

Liam McDermott’s picture

FileSize
3.49 KB

Just added a 'Messages per-thread' setting for this, which forces messages to be split into a new thread when that thread has a given number of messages. The point is to make the old school, one message per-thread, style of the Drupal 5 Privatemsg possible again.

Had to use a hook_form_alter(), as $message isn't passed by reference into hook_privatemsg_message_validate() yet. This should be refactored after #398916: Remove hook_form_alter in favor of a more flexible privatemsg_new form goes in. :)

Thought it would be easier to provide a whole new copy of the module than a patch, but will provide patches to any further changes if people prefer.

Berdir’s picture

The problem with hook_form_alter is that it ignores messages sent by the api functions.

However, you should be able to use the privatemsg_message_presave_alter hook, as you are not actually validating anything but change the message (remove thread_id, if available).

You could maybe add a setting to switch between "create new thread" (presave) and "deny to send message" (validate).

Liam McDermott’s picture

FileSize
3.57 KB

Thanks for the advice berdir, here's an updated version. :)

Berdir’s picture

You might want to use GROUP BY in your count messages query, because pm_index might contain multiple lines for the same uid, when the user has written some messages himself. (currently, this might change...).

Other than that, your changes look good.

NaheemSays’s picture

This can now (probably) be changed to use the block_user_messages since #376023: #288183 followup: change hook_privatemsg_block_message to work on multiple recipients has been comitted.

highvoltage’s picture

I hate to sound unappreciative and cranky but I dont think quota is a good word to use in this context. Everyday people do not use "quota" in this manner. I can just imagine the mountains of users who are going to send me help messages over the alert that their quota is reached. Drupal wording is such an endless frustration...

To most people a quota is something they need to fill at their jobs, if they're familiar with it at all, not a maximum limit for their inboxes. I know there are language differences in drupal developers that lead to funky naming because of a lack of knowledge of colloquial usages, but... common guys.

/headache

Why not use "Your inbox is full" like 99.99% of everyone else.

litwol’s picture

Status: Needs review » Needs work

@highvoltage, no harm done.

What words do you recommend we use for this ?

There are various use cases:
1) inbox empty
2) inbox full
3) inbox % used.
4) user X cant receive messages because his box is full
5) You are viewing your own box and your limit is reached, you are notified to delete some messages...
6) ????

Please brainstorm various use cases and end-user meaningful ways to describe them. I will certainly include it into the module if the suggestions make sense.

Cheers.

Berdir’s picture

I can only repeat myself:

Attached is a first working version. It should work but is not yet finished, I'm especially unhappy about the wording of the admin screen and the error messages (as always ;) )

This is a first _technical_ preview, I am not a native speaker and I am only familiar with the "technical" english, because that's what I use. So, I am open for open for wording suggestions, also keep in mind that there are different areas: "Public/Normal" user interface, Administration user interface, Code. Not every area needs to use the same word.

Update: While I'm glad about every feedback I can get, what really helps are exact improvement suggestions, something like a list of "My ugly text" => "Your suggestion".

highvoltage’s picture

Yeah, sorry about not adding something more useful than the complaint x_x. Keeping the language dead simple is the way to go for alerts presented to users. As long as they know that the inbox is being referred to and how it's current state affects what they want to do(sending or recieving), it should be fine.

A few examples:

-Your inbox is full, you must delete some of your existing messages before you can receive new ones.

-Bob's inbox is full, he cannot receive new messages.

-Your inbox is 46% full.

-You have reached your send limit, please wait *insert time* before trying again.

Or to go even further you can add an explanation as users might become frustrated by this particular setting...

-To prevent spam, the administrator has set a send limit of five messages within ten minutes. Please wait *insert time* before trying again.

afeijo’s picture

I also suggest 2 options:

-msgs older than X days could be removed automatically (system wide option or user option)
-the user could opt to lost the oldest msg if a new one was sent, in that case the sender wouldn't get a limit msg stopping him to send it

NaheemSays’s picture

@Afiejo - the problem with both of them is that a user will not be consenting to the removal of the messages.

As for the latter option, I can see it causing problems where you can delete an important message (maybe even before its read?) from a user's inbox by sending enough spam.

tomtom122’s picture

Hello @all,

with the new dev version from privatemsg it seems the quota - module don't work anymore.
Can anybody tell me whether the module gets an upgrade.

Thx
Tomtom

gunzip’s picture

subscribe

frankcarey’s picture

couple months since this was talked about... might this make it in for 1.0? Is there anything that needs to be finished?

mark.’s picture

I also noticed that the message quota no longer works with the dev version of privatemsg.

Subscribing.

frankcarey’s picture

I just finished redoing a "quota" module for a client that limits the number of messages a user can send over a period of time and gives a warning message to users when they access the message form about how many messages they have left.

This was before I realized how extensive Berdir's module was already (oops), so it's not based on any of that code :( , but I think it's solid and gives the most common features requested for this type of module, to limit spam.

Berdir’s picture

Hah ;)

I will try to reroll this soon, however, I *need* help with that. Not with the technical stuff, I can handle that, but with the user facing (and probably also documentation) strings. See #21-24 for a start.

Oh, and if you want to help, please go through the patch and propose exact changes.

Example:
- Instead of "'Enable Recieve Quota of messages'" I suggest to use "some better string".
- ...

mark.’s picture

FileSize
13.88 KB

I gave it a shot on the admin interface. These string replacements make more sense to me at least.

litwol’s picture

Oh something popped into my mind. I think we should allow users over quota receive messages, but they will simply not see those messages. ha! I think that users will be *very* enticed to delete old messages when they are aware that they have X messages they received but may not be read. Nothing like a harmless bait. This solves the problem of *whether sender should be blocked from sending when receiving inbox is full* or *whether mesages just get undelivered*, etc.

Yes so simply receiving message but not allowing it to be viewed until inbox quota limit have been resolved should be an easy median between various solutions... Especially if we make the *quota is full* resolution configurable in admin (prevent receiving, receive but dont allow reading, prevent receiving and notify sender, etc...)

What does this mean for database storage? On database level it means that we store more messages, yes and that is inevitable. However i will strongly argue that physical storage is nearly free today and especially when talking about storing text. Performance *may* be negatively impacted, but we take great care to be consistent on performance and if extra messages introduce obvious slow performance to this module then we will surely address it immediately.

skizzo’s picture

As a user, I would have no problem removing a batch of old messages...
...if those messages could be easily forwarded to my e-mail account first.
And I would be putting to use my space on Gmail :-)

TapSkill’s picture

I think a configurable quota/limitation setting would be nice, but I don't think I would use it except as a safety device to keep people from spamming. In that sense, I would use some sort of time limit per message sent. For example, you can only send one message per every five minutes.

Berdir’s picture

Status: Needs work » Needs review
FileSize
3.66 KB

Re-rolled with the text string changes of #33 and and also did a few replacements, for example quota => limit in some places.

I did not test it very well, but send/recieve quotas seemed to work.

mark.’s picture

How difficult would it be to limit the number of recipients a user is allowed to send a single message to?

Sending a message to all of the users of a site, with a single click of a mouse seems like a spammer's dream.

frankcarey’s picture

Another user needed my module to limit the number of messages a user can send per period of time, so i cleaned it up a little and posted it to d.o
http://drupal.org/project/privatemsg_limits

It doesn't look like to much of Berdir's work and mine overlaps since it looks like the patch focuses on limiting inbox size, not number of messages. This module could also be used to set a maximum number of recipients per post as well by setting the time period to zero, and a limit to what ever you wanted the per message limit to be. I'd appreciate any feedback since I'm planning to user this on a clients site. If you'd like to work this back into privatemsg core, just let me know.

Berdir’s picture

@frankcarey
I haven't looked in detail at your module, but message_quota supports sending quota/limits too, in messages (or threads) per timeframe, as your module. Only difference seems to be that yours can have a configurable warning/output level.

frankcarey’s picture

I see that I have to look more closely at the quota module code as well. When i was creating limits, I was wondering if it would make sense to make more of a limit/quota api so that custom limits could be more easily added by hook or overridden. I have this partly setup by passing a $limits object in and out of my functions, but it probably needs more than that... thoughts?

Berdir’s picture

Not sure...

However, I am pretty sure that we should merge our modules together, feel free to re-use my code. I will probably make a patch against your module to incorporate my additional features sooner or later, but I don't have much time at the moment.

I will probably release my other sub-modules (rules integration and attachments) as separate projects too, the chances that they will be commited soon are rather small, especially for attachments, which can probably be deprecated in D7 (fields + filefield in core... :)). And maintaing a module in a issue hurts.. physically ;)

frankcarey’s picture

agreed. I'd be happy to give you CVS access to the privatemsg_limits project, which would make collaboration that much easier, sound good?

yajnin’s picture

subscribe

00110000’s picture

So up until now this was working fine for me. I recently updated the private message module and afterwards when visiting my messages in my inbox I get this:

warning: Invalid argument supplied for foreach() in /home/user/user.com/sites/all/modules/privatemsg_quota/privatemsg_quota.module on line 313.

Which apparently is this in the quota module:

foreach ($account->roles as $id => $role) {

Anyone know how to fix this? Or is anyone else receiving this warning as well?

frankcarey’s picture

@Berdir I granted you permissions on the privatemsg_limits module. Feel free to commit your upgrades to HEAD.

http://drupal.org/project/privatemsg_limits

00110000’s picture

Bump?

Berdir’s picture

FileSize
4.76 KB

Updated the module as I'm going to use it too now.

- Changing the version to 6.x-2.x, it should still work with privatemsg-1.0 but that might chance in the future once new features are added to the 2.x branch and this modules depends on them.
- Implements recipient amount limits
- Improves some strings and also the admin UI in general. Please tell me if the new strings are easier to understand and provide better ones if you have ideas!
- Other clean ups and documentation improvements
- The plan is to merge this with http://drupal.org/project/privatemsg_limits but I want to discuss this with frankcarey first. (@frankcarey: can you join #drupal-games from time to time and ping me if I'm there?)

OFF’s picture

Very Nice!!

GeekyLass’s picture

Hi.
I'm using this to limit the ability for users to send to more than 1 person at a time. I have to say it works great. Thank you!

BenK’s picture

Subscribing...

GeekyLass’s picture

Hi.
This is working wonderfully but I found an issue in terms of for what I am using it for. :)

As posted above, I'm using it to limit how many users a message can be sent to at one time to one user. The message "Separate multiple names with commas." still appears when I've limited all roles to only being able to send to one user per message. Making this message change from "Separate multiple names with commas." to "Enter a username" or something when you've limited sending to one user as a time would be really helpful.

I tried removing the "Separate multiple names with commas." message by changing code in the private messaging module but ended up getting errors. Adding this change or ability to change it in this module depending on the settings you have would be helpful.

If this is a seperate private messaging issue, let me know and I'll post it separately.

Berdir’s picture

Never change the code in a module, unless there is really no other way. And in the case of forms, there is always http://api.drupal.org/hook_form_alter.

Sounds like a nice feature, will add on the next re-roll.

antlib’s picture

> Sounds like a nice feature, will add on the next re-roll.

Yay! Looking forward to it :")

GeekyLass’s picture

Yay. That's cool.

I would use more... drupal friendly ways unfortunately I'm not a strong enough coder, or understand enough about all the hooks, and other things that drupal already has built.

crea’s picture

Subscribing

Bilmar’s picture

subscribing

joostvdl’s picture

subscribing

Berdir’s picture

Version: 6.x-1.x-dev »

FYI, I installed and configured the privatemsg_quota module at the newly installed demo site: http://demo.worldempire.ch, if someone wants to test it.

Bilmar’s picture

Hello Berdir,

Below is my first test of this module using your new testing website. My websites are not in need of this feature, but as you have been great with support with other privatemsg features I am interested in I thought it was only right to test other privatemsg modules and give feedback =)

Possible Issues:

1. The Receive Settings Role-based settings looks to not be overriding correctly.
I made role A set to 5, but user1 who has role A is still only able to receive 3 messages as the Number of allowed messages or threads setting value is set to 3. http://www.screencast.com/users/trupal218/folders/Jing/media/f8e73a08-09...

2. Send Settings Role-based for role A set to 5, Number of allowed messages or threads value set to 2, but User1 was able to send 3 messages and on 4th message received "Message limit reached" error. http://www.screencast.com/users/trupal218/folders/Jing/media/f1d381b8-a4...

3. Recipient Settings Role-based settings looks to not be overriding correctly.
admin and role A was given override values but was restricted to value in Number of recipients per message: 1
http://www.screencast.com/users/trupal218/folders/Jing/media/8af9a23b-6f...

4. same username can be added into To: twice with Recipient Settings Number of recipients per message: 1
from third attempt it doesn't add http://www.screencast.com/users/trupal218/folders/Jing/media/fe4691a5-4a...

5. two different user realnames can be added into To: with Recipient Settings Number of recipients per message: 1
I'm able to add Kahlan Amnell (user1) and Zeddicus Zu'l Zorander (user2) both into To: using their realnames, not using usernames
http://www.screencast.com/users/trupal218/folders/Jing/media/4f2c87fc-10...

6. Unchecking all settings for Quota and saving causes issues
- recipient setting looks to still only allow 1 recipient in Write new message (not able to add multiple users)
- now able to send unlimited message from admin to user1, but user1 inbox gives "Message limit reached" error even though each new message is recieved and there shouldn't be a limit anymore. Is a site 'flush cache' needed?
both issues above are shown in http://www.screencast.com/users/trupal218/folders/Jing/media/0e73ab23-c6...

All Quota Settings used during testing (no other settings changed)
http://www.screencast.com/users/trupal218/folders/Jing/media/fa65a6a9-18...

Thank you

Akela’s picture

This sounds most interesting.
Subscribing :)

Michsk’s picture

+1, i see there are enough testers so i will just have to sit and wait for it.

giggler’s picture

+1 subscribing...

gugki’s picture

Hey, great work! :D
How far did the merge with PrivMsg Limits go so far?

BenK’s picture

To get this committed, one thing we'll have to think about is how this functionality interacts with the new ability to send to roles. Should the quotas apply to sending to roles, too? Should sending to a role count as one message or as many messages? And should there be special permissions that allow you to bypass quota limits when sending to a role?

Just food for thought....

--Ben

Berdir’s picture

Title: Message quota » Message limits
FileSize
37.52 KB
6.46 KB

Ok, I did some work on this to get it up to speed ;)

@frankcarey: I plan to add this to the privatemsg project but obviously only to the 2.x branch. What do you say about marking your module as privatemsg-1.x compatible only and maybe provide a simple upgrade path (migrating the settings) to this version?

- Added *many* tests (682 passes). Pretty much all features are now tested and verified that they work
- Fixed a few bugs when limiting to threads. Will require a small patch to privatemsg.module, I will handle that in a separate issue
- Renamed the module to privatemsg_limits, renamed threshold to amount, quota to limits in the code and strings. Note that you have to re-configure the module as all variables have changed.

Response to #60:
1 + 2 + 3: The new patch comes with patches to verify that the role overriding settings work correctly. Can you please re-test? (Demo site hasn't been updated yet, will do that soon)
4 + 5. Strange stuff but that is not affected in any way by this module. Looks more like an issue with realname integration...
6. Can't reproduce with this patch, when disabling the limits, I can send messages again, including multiple recipients.

What's needed to get this in:
+ Technical testing as in, does it work...
+ UX testing: Are the settings understandable? Do they work as expected? Are the error messages clear for a user?
+ Thinking about other recipients types, but imho, its quite simple: Each recipient (be it a user or a role with 100000 members) counts as a single recipient and a single message.
+ Incorporate the additional features of the existing privatemsg_limits module.

Berdir’s picture

Note:
- I have updated the demo site, it should now be possible to use that to test this module again.

@todo: Remove variables when uninstalling...

BenK’s picture

Status: Needs review » Needs work

Hey Berdir,

I've begun testing your latest patch. Because there are a lot of configuration options in this sub-module, I'm going to divide my testing and UI suggestions into chunks based on the four fieldsets at admin/settings/messages/limits (Generic settings, Receive settings, Send settings, and Recipient settings). In this comment, I'll discuss Generic settings:

A. I think the "What Should Be Limited" setting is confusing in this fieldset. The setting affects the functionality of the "Receive settings" and "Send settings" fieldsets, yet this isn't totally clear because it looks like it is only related to other items in the "Generic settings" fieldset. But other items in the "Generic settings" fieldset only talk about "threads," which makes the default value of "Messages" for the dropdown even more confusing. I really think that "Receive settings" and "Send settings" should each have their own "What Should Be Limited" dropdown menu. (This would also enhance the flexibility of the module because site admin's may want to control Receive settings based on threads and Send settings based on messages). If this were done, then you could remove "What Should Be Limited" from this fieldset (since it has no affect on the two remaining items). Then the fieldset could be retitled "Thread settings".

B. Berdir, do you want to start using the term "conversation" instead of "thread"? This would help us match other elements of the user-facing interface. Just a thought. I've written the titles and notices below using "thread" terminology but we may want to change this to "conversation".

C. Re-title "Number of messages in a thread:" to be "Maximum number of messages in a thread:"

D. This note deals with the description for number of messages in a thread. It currently reads:

"When a thread reaches this number of messages, the specified action below will happen. 0 is unlimited. Hint: setting this to 1 will make this PrivateMsg module more like the old, single message interface."

To simplify and clarify, I'd change it to:

"The maximum number of messages that can be included in a thread. If this maximum has already been reached and a user attempts to add an additional message, then the action specified below shall be taken. Leave this set to 0 if you want unlimited messages per thread."

E. Re-title "Number of messages in a thread action:" to "Maximum messages action:"

F. Change description from "This will happen when a thread has reached the maximum number of messages per-thread" to "This action shall be taken once a thread has already reached its maximum number of messages."

G. If the "Create new thread" action is specified, there should be a message displayed to the user when the new thread is created. Currently, it's confusing why sudden a message reply is appearing on a totally different thread. The message could be something like:

"Your message would have exceeded our [numberofmessages] messages per thread limit. As a result, we've created a new thread for your message."

H. If the "Block message from being sent" action is specified, this is the message that is displayed"

"The message cannot be sent because the limit of messages per thread has been reached."

Change this to:

"This message cannot be sent because the thread already contains [numberofmessages] messages (the maximum number of messages permitted per thread). To send this message, please create a new message thread."

Well, that's it for the "Generic settings" fieldset. On the plus side, I tested all of the functionality controlled by this fieldset and it seems to be working great. As always, I'm eager to hear your thoughts....

Cheers,
Ben

Berdir’s picture

Title: Message limits » Message quota
Status: Needs work » Needs review

Awesome :)

I'll wait with a re-roll until you've tested the other features as well. From your list, there doesn't seem to any technical issues that would stop you from doing that :)

Berdir’s picture

Title: Message quota » Message limits
Status: Needs review » Needs work

Uhm. Firefox is sometimes strange. Even though i reloaded the page and saw your response, firefox kept the default select values from *before* my re-roll.

BenK’s picture

Hey Berdir,

In follow-up to #68, in this comment I'll discuss Receive settings:

First of all, I've tested the functionality of this fieldset and everything is working great. I can successfully limit both the number of messages and the number of threads. I can also override the global max limit by role.

Now for possible usability improvements and enhancements:

I. As referenced in #68, create a new dropdown in this fieldset titled "What Should Be Limited". This will control whether settings within this fieldset should be based on "Messages" (default) or "Threads". Place this dropdown first in the fieldset.

J. "Limit number of messages a user can have" checkbox. Change this to read: "Limit the total number of messages/threads that a user may have."

K. "Number of allowed messages or threads:" Change this to: "Maximum number of messages/threads per user:"

L. Description text currently reads: "How many messages/threads a users can receive before he has to delete old messages. When the limit is reached, users are not allowed to receive or send new messages/threads." Change this to: "The total number of messages/threads that a user may have before he has to delete old messages/threads. When this limit is reached, users are not allowed to receive or send new messages."

M. I'm wondering if we should have an additional dropdown menu that allows the site admin to determine whether "Inbox" messages, "Sent Messages", or "All Messages" should be counted toward the max number of messages/threads. I could see many use cases, for instance, where you might only want to count the Inbox. What do you think?

N. Re-label "Role-based limits" to "Override Maximum Limit By Role"

O. Change description from: "You can override the limits for each role. If a user has multiple roles, the highest value will be used. Use unlimited, if you do not want to limit a role." Change to: "You may override the maximum limit specified above on a per-role basis. If a user has multiple roles, then the highest maximum value shall be used. Enter "unlimited" if a role may have unlimited messages/threads. Enter "0" if you do not want to override the maximum limit setting."

P. I'm wondering if we should have a new option that allows certain messages to still be received by a user even if they have reached their maximum limit. For instance, a website may want messages from the site administrator to still be delivered to the user (such as messages reminding the user to clear out their inbox so they can receive more messages). How about a setting called "Bypass Recipient Limits When Sending"? Any role that was selected (in a dropdown or series of checkboxes) would be permitted to successfully send messages to recipients even when the recipient's message/thread maximum limit was already reached. Ideally, this should also work for messages sent to a role. Thoughts?

Q. Here is the string that is currently displayed to the user when his max limit is reached: "Message limit reached, you can't send new messages until you delete some." Change this to: "Your message mailbox is currently full. You are allowed a maximum of [numberofmessagesorthreads] [messagesorthreads] in your mailbox at one time. You won't be able to send or receive new messages until you delete some existing ones."

R. The above message should probably be displayed at user/uid/messages so that site administrators can see this when visiting a user's messages page. But instead of using "your" change the message to read: "This message mailbox is currently full. [username] is allowed a maximum of [numberofmessagesorthreads] [messagesorthreads] at one time. [username] won't be able to send or receive new messages until some existing ones are deleted."

S. Here is the string that is displayed if you try to reply with your max limit reached: "You have exceeded your message limit. You need to delete messages before you can send new ones. Change this to: "Your message mailbox is currently full. You are allowed a maximum of [numberofmessagesorthreads] [messagesorthreads] in your mailbox at one time. You won't be able to send or receive new messages until you delete some existing ones."

T. This string currently reads: "[username] can't receive more messages because he reached the allowed limit." Change this to: "This message cannot be sent because [username]'s mailbox is full."

U. A message sender's "Reply to thread" fieldset is hidden when the other user in the conversation has exceeded his maximum message/thread limit.... But the message sender (who hasn't reached his limit) might be confused why he can't reply. So a message should display saying that: "You cannot reply to this thread because [username]'s mailbox is full."

V. The "used limit" indicator currently reads "Used limit: 100% (3/3)". This may not make sense to the user without any context. Change this to: 'You are currently using 100% ([numberofmessages/threads] [messages/threads]) of your [maxnumberofmessages/threads] [message/thread] limit.'

That's it for the "Receive settings" fieldgroup....

--Ben

BenK’s picture

Hey Berdir,

In this comment I'll discuss Send settings:

First, let me say that I've tried the various configuration options and they are working well. The maximum send limits are working and the role overrides are working, too. Now to the usability issues:

W. Create a new dropdown in this fieldset titled "What Should Be Limited". This will control whether settings within this fieldset should be based on "Messages" (default) or "Threads". Place this dropdown first in the fieldset.

X. The checkbox label currently reads: "Limit how many messages/threads a user can send in a given time period". Change this to: "Limit the total number of messages/threads that a user can send in a given time period"

Y. Change the label that reads: "Number of allowed messages or threads:" Change this to: "Maximum number of messages/threads per time period:"

Z. The description of this currently reads: "Number of messages or new threads a user is allowed to send in a given time period." Change this to: "The total number of messages/threads that a user may send in a given time period."

AA. The dropdown label currently reads: "Set the time period:" Change this to: "Set the time period to be used:"

BB. Add "1 week" and "1 month" to available options in dropdown menu.

CC. The description currently reads: "Users are not allowed to send more than the configured amound of messages/threads during the given period." Change this to: "Specify the time period to be used when limiting a user's sent messages/threads."

DD. Re-label "Role-based limits" to "Override Maximum Limit By Role"

EE. Change description from: "You can override the limits for each role. If a user has multiple roles, the highest value will be used. Use unlimited, if you do not want to limit a role." Change to: "You may override the maximum limit specified above on a per-role basis. If a user has multiple roles, then the highest maximum value shall be used. Enter "unlimited" if a role should not have a limit. Enter "0" if you do not want to override the maximum limit setting."

FF. If a person exceeds their sent message limit, the following is displayed: "You have exceeded your message sending limit for now. You can send your next message in 4 min 35 sec." Change this to: "Your message was not sent because you have exceeded your sending limit. You are allowed to send [numberofmessages] [messages/threads] every [timeperiod]. You can send your next message in 4 minutes and 35 seconds."

--Ben

Berdir’s picture

wow, I didn't expect that ou run out of characters :)

A few responses, will do a re-roll at a later point:

B: Yes, I like the idea of switching to conversion in UI strings. Can you create a separate issue for changing the existing ones in privatemsg.module (and probably others).

M: Interesting idea. Not sure how hard it is going to be, because the query handling for that is in privatemsg_filter.module and I'd like to avoid duplicating that. Also, users can use "Archive" to remove messages out of the inbox for example, so limiting that without having the "archive" button permission-controlled doesn't seem like a great combination ;). Maybe make this a follow-up? This might also be related to #735094: Make "inbox" and "sent" menu items optional in the filter Module.

P: What about a "bypass private message limitations" permission? Please provide a better name and description if you can come up with something :)

T: Hm. Problem is, that's not the same thing. If you send a message to two recipients and one of them can't receive the message, it will still be sent to the other recipient. Maybe it would be possible to extend the validation hook to allow to override the default "all recipients are blocked" message. Need to think about this.

U: Similiar problem here. privatemsg.module only knows that all recipients are blocked, not why. And privatemsg_limits.module may not know that all recipients are blocked. For example, it could be that 2 recipients are blocked because of pm_block_user configurations, one because he has disabled privatemsg and a fourth because he has the mailbox (that's a new term too btw, but I think I like it...) full. So, we can do a few different things: a) Display blocking reasons when replying b) Allow modules to specific that blocking reasons should be displayed for replies too with a new option. c) Always display the reply form but disabled if there are no valid recipients. d) Display a simple message (what?) when there are no valid recipients. e) something else? Combinations are possible too :)

BB: Can do that. I an project where I've used this, we were also thinking about providing an alternative way: Currently, the timeframes are relative. If you are allowed to send two messages in 2 hours, and you send a message now and another in 15min, you have to wait 45min to send a third and then another 15min to send a fourth. This is because after the first hour has passed, you have still sent two messages during the last hour. So it always uses the time frime like this: "now() - timeframe until now()". IMHO, that makes perfectly sense for smaller time frames up to a day or so. A more common approach for bigger time frames like a month would be "per month". So, you can send 100 messages every month and if you send all of them between (for example) 20.08 and 25.08, you can send 100 messages again starting from the 01.09, they don't "regenerate" between the 20.09 and 25.09. What do you think? And how should that be displayed as options? Technically, it would be a switch between "Relative" and "Absolute". But I'm 100% sure that nobody would understand that :)

BB2: If we make it possible to switch between those two options, what do you think should we display for the "you can send your next message in ..."? A fixed date/time or a relative time? (on the 1. september vs. in 17 days)

FF: I can change the message but note that the time comes from this function: http://api.drupal.org/api/function/format_interval/6, so I can't change sec/min.

BenK’s picture

Hey Berdir,

In this comment I'll discuss the fourth and final fieldgroup, Recipient settings:

First, the functionality works great. I was able to set recipient limits successfully and override them successfully with role-based limits.

As for usability improvements, here they are:

GG. Checkbox label currently reads: "Limit how how many recipients a user can address in a single message". Change this to: "Limit the total number of recipients that a user can include in a single message".

HH. The label currently reads: "Number of recipients per message:" Change to: "Maximum number of recipients per message"

II. Currently reads: "Defines how many recipients a user can address in a single message. Users are still able to reply to threads with more participants." Change this to: "Defines the total number of recipients a user may include in a single message. This setting only affects new messages (and not replies on existing message threads). A role is considered to be a single recipient regardless of the number of users with that role." (By the way, I like your approach of treating other recipient types as a single recipient. That makes sense to me.)

JJ. Re-label "Role-based limits" to "Override Maximum Limit By Role"

KK. Change description from: "You can override the limits for each role. If a user has multiple roles, the highest value will be used. Use unlimited, if you do not want to limit a role." Change to: "You may override the maximum limit specified above on a per-role basis. If a user has multiple roles, then the highest maximum value shall be used. Enter "unlimited" if a role should not have a limit. Enter "0" if you do not want to override the maximum limit setting."

Well, now that I've discussed and tested all four fieldgroups, that should be it for now. I'll look forward to testing the revised patch as soon as you have it. Let me know if any of this is unclear.... Sorry for the long comments! ;-)

Cheers,
Ben

BenK’s picture

Hey Berdir,

Here are my responses to your notes in #73:

B: Yes, I like the idea of switching to conversion in UI strings. Can you create a separate issue for changing the existing ones in privatemsg.module (and probably others).

>> Sounds good. I'll create this.

M: Interesting idea. Not sure how hard it is going to be, because the query handling for that is in privatemsg_filter.module and I'd like to avoid duplicating that. Also, users can use "Archive" to remove messages out of the inbox for example, so limiting that without having the "archive" button permission-controlled doesn't seem like a great combination ;). Maybe make this a follow-up? This might also be related to #735094: Make "inbox" and "sent" menu items optional in the filter Module.

>> Yes, I'll make this a follow-up. Once we get the sub-module initially committed, it will make all of this much easier.

P: What about a "bypass private message limitations" permission? Please provide a better name and description if you can come up with something :)

>> Sounds good. I like the idea of doing this with a new permission. How about we call it this: "bypass recipient message limit"? The description could read: "Enables a user to send a message to a recipient even when the recipient's message/conversation maximum limit has already been reached. Without this permission, the message would ordinarily be blocked."

T: Hm. Problem is, that's not the same thing. If you send a message to two recipients and one of them can't receive the message, it will still be sent to the other recipient. Maybe it would be possible to extend the validation hook to allow to override the default "all recipients are blocked" message. Need to think about this.

>> As an alternate option, you could change the existing string to: "[username] can't receive more messages because [username]'s mailbox is full. The message has been sent to any other recipients you may have entered."

U: Similiar problem here. privatemsg.module only knows that all recipients are blocked, not why. And privatemsg_limits.module may not know that all recipients are blocked. For example, it could be that 2 recipients are blocked because of pm_block_user configurations, one because he has disabled privatemsg and a fourth because he has the mailbox (that's a new term too btw, but I think I like it...) full. So, we can do a few different things: a) Display blocking reasons when replying b) Allow modules to specific that blocking reasons should be displayed for replies too with a new option. c) Always display the reply form but disabled if there are no valid recipients. d) Display a simple message (what?) when there are no valid recipients. e) something else? Combinations are possible too :)

>> I'm not sure about this. At the very least we need to display a simple message such as: "You cannot reply to this thread because there are no valid recipients. This could be because [username]'s mailbox is full or they have recently disabled private messaging." I think we need this because we're disabling functionality for the person replying even though it's not his fault.

BB: Can do that. I an project where I've used this, we were also thinking about providing an alternative way: Currently, the timeframes are relative. If you are allowed to send two messages in 2 hours, and you send a message now and another in 15min, you have to wait 45min to send a third and then another 15min to send a fourth. This is because after the first hour has passed, you have still sent two messages during the last hour. So it always uses the time frime like this: "now() - timeframe until now()". IMHO, that makes perfectly sense for smaller time frames up to a day or so. A more common approach for bigger time frames like a month would be "per month". So, you can send 100 messages every month and if you send all of them between (for example) 20.08 and 25.08, you can send 100 messages again starting from the 01.09, they don't "regenerate" between the 20.09 and 25.09. What do you think? And how should that be displayed as options? Technically, it would be a switch between "Relative" and "Absolute". But I'm 100% sure that nobody would understand that :)

>> I like having these "Relative Time" and "Absolute Time" options a lot. I think a dropdown menu would be fine (with "Relative Time" being the default). I actually don't think that "Relative Time" and "Absolute Time" are all that confusing as long as we can put an explanatory description next to the options. Here's a possible description:

"Relative time means that elapsed time is calculated from the moment each message was sent (usually makes sense for shorter time periods). At the moment a single sent message becomes older than the specified time period, then one new message becomes available for the user to send. On the other hand, absolute time means that elapsed time is calculated based on the time period itself (usually makes sense for longer time periods). A user would not be allowed to send more messages until that entire time period has elapsed, regardless of when messages were sent within the time period. If users are allowed to send 100 messages per month (with no more messages available to send until the first of the following month), then you are using absolute time."

BB2: If we make it possible to switch between those two options, what do you think should we display for the "you can send your next message in ..."? A fixed date/time or a relative time? (on the 1. september vs. in 17 days)

>> I like "17 days" more than the fixed date. Since we're also telling users what our sending limit is, I think this makes more sense.

FF: I can change the message but note that the time comes from this function: http://api.drupal.org/api/function/format_interval/6, so I can't change sec/min.

>> OK, that would be fine.

Thanks!

--Ben

Berdir’s picture

Status: Needs work » Needs review
FileSize
47.48 KB
7.4 KB
3.57 KB

All the string replacements that aren't mentioned below should be fixed...

A. As you suggested, splitted the what should be limited select up and make to separate options for send/receive.

B. As discussed, let's use conversation instead of thread.

G. I added the message but I'm wondering if we want to make it configurable. One of the main reasons why this feature exists is that it allows you to sent it to 1 and basically disable threading. And if you do that, the message shows up for every single reply. How should that option be named?

M. Not yet done.

P. Added the permission and the check. I added the description as a comment to the hook_perm() definition as permissions don't have descriptions in D6 but we can use it when porting to D7.

R. Oh, nice catch! The code there was actually wrong and tested the limits for the current user. Fixed and added the strings.

T. I used "This message cannot be sent to !name because !name's mailbox is full." now, that makes more sence given that there could be more recipients and it is also displayed when trying to reply, see below.

U. Ok, privatemsg.module now displays a message and lists the reasons why the recipients are blocked (currently only if all are blocked, it can be changed to always display that list..)

V. Changed including singular/plural handling when you only have a single message/conversation. Not handled for the limit (unlikely in english but some languages have multiple plural forms and write it for example different for 20). But I have no idea on how to solve this anyway.

BB. Added 1 and 4 weeks (format_interval() doesn't display month, for quite obvious reasons :)). The relative/absolut thing hasn't been added yet, we might want to postpone that as I'm not sure how to solve it just yet.

I think we're pretty close with this one :) The one thing that is still missing is adding the warning message that http://drupal.org/project/privatemsg_limits supports. I think allowing to directly upgrade isn't really necessary, you just need to configure a few settings...

Attaching a full patch (but without translation template), a patch for privatemsg.module only and the tar.gz for the module.

Status: Needs review » Needs work

The last submitted patch, privatemsg_limits2.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
2.94 KB
46.86 KB

Re-rolled the patches.

Status: Needs review » Needs work

The last submitted patch, privatemsg_limits3_form_changes.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review

I think I broke the tests, the patch is fine if someone wants to review.

Berdir’s picture

Nasty. Because I gave the users in the test the access profiles permission, the username is now a link and so the code that figured out which unblock link to click for which user was broken.

Is fixed now, let's try again..

Berdir’s picture

Berdir’s picture

#78: privatemsg_limits3.patch queued for re-testing.

chuckbar77’s picture

Hi,

This feature looks really cool.

I was wondering if these error message show for each user or one message for all users?

-This message cannot be sent to name1 because name1's mailbox is full.
-This message cannot be sent to name2 because name2's mailbox is full.
-This message cannot be sent to name3 because name3's mailbox is full.
-This message cannot be sent to name1, name2 and name3 because their mailboxes are full.

Just wondering as if a message is sent to a role with many users, where say 40 people have full mailboxes, then the message for each user will look scary.

Regards

Berdir’s picture

There is a message for every user that is a direct recipient. When sending messages to a role, you will not see the blocking messages of the users that belong to a role.

chuckbar77’s picture

Thanks for the clarification.

It would be nice if it is possible to show the plural error message even when there are multiple recipients (not to role).
Multiple error messages with the same text is always scary for end-users =)

Can't wait to try this module out.

Regards

Akela’s picture

The fact that this patch exists was the last element that convinced me to switch my forum to drupal. Thank you so much!

jrivelli’s picture

I am looking to limit the recipient list to just one person. I have read through this whole queue and it seems this only focuses mainly on how many messages can be in an inbox?

Berdir’s picture

Just try it out, it does much more than that.

jrivelli’s picture

Alright, Im gonna look up how to add a patch and redo it, i forgot :P

jrivelli’s picture

Hmm I don't currently have shell access. There is no other way to do a patch is there?

Berdir’s picture

You can do the patch local and then upload the files. Also, please try to avoid spamming the issue, every time you respond here, mails are sent to dozens of others.

You can also just wait a few days, this is going to be commited soon I think.

Berdir’s picture

#78: privatemsg_limits3.patch queued for re-testing.

Berdir’s picture

Status: Needs review » Needs work

The last submitted patch, privatemsg_limits3_form_changes.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
2.82 KB
47.13 KB
7.52 KB

Updated, fixed some tests and a conflict.

Note that the tar.gz doesn't contain the changes to privatemsg.module and the patch doesn't contain the translation template.

miro_dietiker’s picture

Rerolled against current dev. Some hunks previously failed.

Providing patch only with the full integration. We're using this in production now.

Isn't it time to commit this feature soon? We can always improve it later. If it is too early we should instantiate a separate project for this module.

miro_dietiker’s picture

Oops patch missing!

Status: Needs review » Needs work

The last submitted patch, privatemsg_69856_97_limit.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
miro_dietiker’s picture

Berdir, my patch basedir seems to be somehow wrong for the testbot... Dunno how to fix this with git diff if my basedir is some levels higher.

Berdir’s picture

Status: Needs review » Needs work

There is a --relative option, maybe that might work? I have never tried something like this...

And yes, this will be commited soon, I was basically waiting for another confirmation that it works after my last re-roll, which response indicates :)

miro_dietiker’s picture

Status: Needs work » Needs review
FileSize
46.34 KB

Works with --relative --no-prefix

Attached below.

Berdir’s picture

Version: » 7.x-1.x-dev
Status: Needs review » Patch (to be ported)

Testbot likes the patch and so do I, commited!

This needs to be ported to 7.x-1.x, will do that later..

miro_dietiker’s picture

Love! ;-)
Great to see this go in.

Berdir’s picture

Status: Patch (to be ported) » Fixed

Ported to D7 and also fixed two minor issues I found during the port.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Witch’s picture

Will this patch be commited?