Two things:

1. There is no permission to now allow users to filter by users - this seems to be available to all.
2. "create privatemsg_filter" seems to be a bad permission name. "create privatemsg_filter tags" would IMO be a better idea. Would this also need an update function to replace all the original permissions from whereever it is stored to the new one? is it worth the "pain"?

CommentFileSizeAuthor
#5 privatemsg_filter.patch1.6 KBnaheemsays
#3 privatemsg_filter_permission.patch1.06 KBnaheemsays
#1 privatemsg_filter.permissions.patch1.05 KBnaheemsays

Comments

naheemsays’s picture

naheemsays commented
Status: Active » Needs review
StatusFileSize
new privatemsg_filter.permissions.patch1.05 KB

actually there is a permission for number 1, but it is not used. attached patch fixes that.

EDIT - a word of note - the number 2 from the above also applies to the delete permission. Both would read better if they had " tags" at the end of the permission names.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

litwol and I talked about tags per user in IRC. If this is going to happen, we need to change/extend the permissions anyway.

naheemsays’s picture

naheemsays commented
StatusFileSize
new privatemsg_filter_permission.patch1.06 KB
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Looks good now :)

naheemsays’s picture

naheemsays commented
StatusFileSize
new privatemsg_filter.patch1.6 KB
naheemsays’s picture

naheemsays commented
Status: Reviewed & tested by the community » Needs review

Added a tag count check before adding the tag select form.

liam mcdermott’s picture

liam mcdermott commented
Status: Needs review » Reviewed & tested by the community

Just to clarify exactly what this patch does:

  1. Fixes bug where Privatemsg_filter ignores the 'use privatemsg_filter' permission;
  2. Only adds the tag selection form (beneath a message thread) if there are any tags available;

I've successfully applied the patch and tested both fixes. It works marvellously and is ready to be committed. :)

litwol’s picture

litwol commented

This line in the conditional statement for privatemsg_filter_privatemsg_view_messages_alter assumes that tags are sitewide. Should i assume this will be patched once we commit the tags-per-user patch?

db_result(db_query('SELECT COUNT(*) FROM {pm_tags}'))
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

The patch, in its current form, creates a new privatemsg_tag module, which is afaik a complete rewrite, so it will need to be patched anyway. But it might take some time until that is ready, while this is just a small fix to the current module.

litwol’s picture

litwol commented
Status: Reviewed & tested by the community » Fixed

homerun.

Status: Fixed » Closed (fixed)

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