« The Regular Members group
Privacy options
Image resizing for non-uploads »
Poll

Would you like to have topic privacy options in Wedge?

Yes -- everyone, just logged in users, and just the author.
4 (14.8%)
Yes -- everyone, just logged in users, just the author, and author's buddies (the regular SMF feature), even if it hurts performance a bit.
2 (7.4%)
Yes -- everyone, just logged in users, just the author, and author's contact lists (like buddies, but you can create multiple lists and put people in one or more of them), even if it hurts performance a bit.
16 (59.3%)
Yes -- everyone, or just me (i.e. just the ability to write drafts...)
0 (0%)
Yes, but I don't really care, I would never enable the feature on my forum.
1 (3.7%)
No, I don't care, and my users wouldn't either.
4 (14.8%)
Total Members Voted: 23

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Privacy options
« Reply #45, on February 16th, 2012, 11:25 PM »
I'm starting to wonder if I shouldn't just give up on topic privacy... :(

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Privacy options
« Reply #46, on February 16th, 2012, 11:26 PM »
It's a worthy cause and it looks like it would be quite popular, so I wouldn't give up on it, but I get the impression it does need some serious planning to make work.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Privacy options
« Reply #47, on February 17th, 2012, 12:06 AM »
Nobody uses this feature on noisen actually. Neither does anyone here. I'm just... Convinced it's a great feature.
But I'd like it to be as simple as possible. Plus I need to add contact list support so there's always gonna be a performance issue.
Shit.

live627

  • Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
  • Posts: 1,670
Re: Privacy options
« Reply #48, on February 17th, 2012, 04:44 AM »
You could make it known that such a feature exists[1], divert[2] it to a plugin, or postpone it for 1.1.
 1. I don't know if you already did it on Noisen
 2. is that the right word to use here?
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Privacy options
« Reply #49, on February 17th, 2012, 09:50 AM »
The problem with baking something like privacy into a plugin is that other plugins may or may not use it properly.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Privacy options
« Reply #50, on February 17th, 2012, 10:09 AM »
Well, I'll try working on this today... -_-

I've decided that I'll try and put the approved test into query_see_topic.
If there's a permission involved in the original query, then I'll add an additional approved test. Is that all right with you?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Privacy options
« Reply #51, on February 17th, 2012, 10:17 AM »
I'm not quite sure what you mean by additional test... if (empty($settings['postmod_rules'])), approved shouldn't be tested at all. Otherwise, the test needs to be approved = 1 || (approved = 0 && (t.id_member_started = $user_info['id'] || allowedTo('approve_posts')))[1]
 1. And remember, post approval is a board level permission, so in topics and message indexes where you have a board or topic id, allowedTo will function normally, but in any place outside of those, like search, recent, unread, etc. the test has to be done separately.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Privacy options
« Reply #52, on February 18th, 2012, 12:16 AM »
Instead of (I tried to find the query you mentioned...)

Code: [Select]
WHERE t.id_board = {int:current_board}' . (!$settings['postmod_active'] || allowedTo('approve_posts') ? '' : '
AND (t.approved = 1 OR (t.id_member_started != 0 AND t.id_member_started = {int:current_member}))') . '
AND ' . $sort . ' ' . $sort_methods[$sort_by]['cmp'] . '

We could have that:

Code: [Select]
WHERE {query_see_topic} AND t.id_board = {int:current_board}' . (!$settings['postmod_active'] || allowedTo('approve_posts') ? '' : '
AND (t.approved = 1 OR (t.id_member_started != 0 AND t.id_member_started = {int:current_member}))') . '
AND ' . $sort . ' ' . $sort_methods[$sort_by]['cmp'] . '

See..? Does it make sense? It doesn't save any time in the query if it has to execute the approved test, but at least it'd work...
Posted: February 17th, 2012, 07:17 PM

BTW, what did we decide already, regarding the position for {query_see_topic}...? As an INNER insertion, or a subselect in the WHERE?
Posted: February 17th, 2012, 07:18 PM

Bump..?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Privacy options
« Reply #53, on February 18th, 2012, 12:39 AM »
I didn't list a specific query, more the fact that those are the rules for topic visibility.

We didn't decide anything specific re where qst should go, though from a convenience perspective, being just a WHERE condition would be pretty handy, especially since being a subselect it wouldn't interfere with the criteria of the parent query so you wouldn't have to ensure t was aliased or anything.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Privacy options
« Reply #54, on February 18th, 2012, 02:00 AM »
I was working on this in the short time I was away... Managed to make it work as a WHERE argument.
Heck, it's really very easy at this point...
admin? WHERE 1=1
guest? WHERE t.approved=1 AND t.privacy="default"
member? WHERE (t.id_member_started=myself OR (t.approved=1 AND (t.privacy="default" OR other privacy settings...))

My main breakthrough is the t.id_member_started test... It's totally upsetting that I never optimized the query into testing for that FIRST... :lol: I used to do it differently, nice waste of time. Then again, at the time I was on a SQL server that didn't support subselects... Which I need for my 'contacts' privacy test.

As for the t alias... I'm afraid there's nothing that can be done. I can't just test for "privacy" because the query might be mixed in with a table that has a privacy field in it... Unless I rename the field, but, well... I'd also have to do that for the 'approved' field in topics because I'm definitely inserting it into query_see_topic. Really, not sure about the whole renaming thing.
Posted: February 18th, 2012, 01:28 AM

A bit of an issue maybe...
Didn't really think about this -- what if the user has moderation perms on the board that a private topic is on...? It should be visible to them, shouldn't it...?
Maybe this could be tested though $user_info['mod_cache']['mq']... But the problem I mentioned, is that it expects the query to have a "boards AS b" in it... :-/

I don't know how to deal with this particular case.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Privacy options
« Reply #55, on February 18th, 2012, 02:24 AM »
Quote
As for the t alias
Not if it's a subquery, i.e. WHERE (SELECT...)
Quote
what if the user has moderation perms on the board that a private topic is on...? It should be visible to them, shouldn't it...?
I can argue this one both ways, because I can see the justification behind them being a mod = being able to see the topic, but at the same time I don't see why they should get a free pass if topic privacy is in play, when only the admin really gets a free pass.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Privacy options
« Reply #56, on February 18th, 2012, 08:21 PM »
The only subquery is when contacts are being looked through. Otherwise it's as easy as usual.
Posted: February 18th, 2012, 09:57 AM

Crap... So, I've got this query in SSI.php (always...)

Code: [Select]
WHERE {query_see_topic}
AND att.attachment_type = 0' . ($attachments_boards === array(0) ? '' : '
AND m.id_board IN ({array_int:boards_can_see})') . (!empty($attachment_ext) ? '
AND att.fileext IN ({array_string:attachment_ext})' : '') .
(!$settings['postmod_active'] || allowedTo('approve_posts') ? '' : '
AND t.approved = 1
AND m.approved = 1') . '

After some consideration, it doesn't really make sense to call query_see_topic at the beginning. Because if I'm not the author, then it'll return false, even before evaluating the allowedTo('approved_posts') that would automatically give me a positive and thus skip the approved test...

So I would logically put that instead of the t.approved test, but not in a 'higher' level place.
That makes sense, right?
Now... What about approved_posts? Is this a forum-wide permission only? If yes, then it WOULD make sense to have it in our query_see_topic test, wouldn't it...? Plus, I've seen plenty of places where SMF just tests for postmod_active, and not for that permission... That would allow us to 'clean up' that code.

Any opinions?
Posted: February 18th, 2012, 12:49 PM

Seriously everyone... Just have a look at ssi_recentPosts().
It doesn't test for allowedTo('approve_posts') before it runs m.approved = 1... Does that make sense? AFAIK, SSI calls can and should support permissions...
Posted: February 18th, 2012, 03:06 PM

Bump.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Privacy options
« Reply #57, on February 19th, 2012, 01:05 AM »
approve_posts is a board level permission, not a global one and in SSI, it isn't loaded. Nor is it loaded for search, recent, unread or indeed anywhere that doesn't explicitly contain topic= or board= in the URL string, so it will always return false, with the exception of true for administrators due to the hard-wiring of always-return-true-for-group-1-users.

Does SSI need to support post moderation? That's a good question. My gut instinct says that for the purpose for which post moderation is normally applied and where SSI is used, it's actually not necessary to worry about and that it would actually likely confuse users rather than reassure them.

The original SSI code does this - excludes unmoderated posts even if the user is otherwise empowered to see them, so in those cases I wouldn't worry about testing for approve_posts or is-owner, I'd simply validate that the post is approved or not (and subject to privacy anyway)

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Privacy options
« Reply #58, on February 19th, 2012, 01:44 PM »
Quote from Arantor on February 19th, 2012, 01:05 AM
approve_posts is a board level permission, not a global one and in SSI, it isn't loaded. Nor is it loaded for search, recent, unread or indeed anywhere that doesn't explicitly contain topic= or board= in the URL string, so it will always return false, with the exception of true for administrators due to the hard-wiring of always-return-true-for-group-1-users.
So, back to square one...
I'd suggest:
- doing {query_see_topic} anywhere in the WHERE when no particular params are given,
- doing {query_see_topic} instead of t.approved = {int:is_approved} anywhere t.approved shows up in a query.
Quote
Does SSI need to support post moderation? That's a good question. My gut instinct says that for the purpose for which post moderation is normally applied and where SSI is used, it's actually not necessary to worry about and that it would actually likely confuse users rather than reassure them.
I don't know.
I do know, though, that global moderators (group ID 3 or something?) probably don't get the benefits of viewing unapproved posts... That should be changed, too...?

Oh, I'm so rusty when it comes to permissions. Heck, I was more knowledgeable about them back when I did my first query_see_topic implementation...

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Privacy options
« Reply #59, on February 19th, 2012, 02:00 PM »
Quote
I'd suggest:
- doing {query_see_topic} anywhere in the WHERE when no particular params are given,
- doing {query_see_topic} instead of t.approved = {int:is_approved} anywhere t.approved shows up in a query.
That's a feasible plan but it still doesn't solve the inherent problem of whether a user is permitted to view unapproved posts or not - because allowedTo() still only works inside certain boards, so for all the non board stuff it still doesn't actually work and either we need to do some kind of storage of boards=>groups with privilege or we run boardsAllowedTo() which isn't that cheap.
Quote
I do know, though, that global moderators (group ID 3 or something?) probably don't get the benefits of viewing unapproved posts... That should be changed, too...?
Group 3 is board moderators, group 2 is global moderators. But in SSI's case, nobody except admins gets the ability to view unapproved posts - and I'm fine with that. You want to view unapproved posts? Do it in the forum where you can review it in context and deal with it properly. But SSI is not a forum replacement, it's an export bridge to external code - and really, that's where you don't need or want unapproved posts (because you're never setting up the requirements to be able to actually approve anyway)

To change it so that global moderators (or any non-admin, really) can view unapproved posts, means you have to make multiple other queries to identify who can actually approve posts, with boardsAllowedTo('approve_posts') and then you can do the comparison[1] which strikes me as a headache, especially given that I don't think it's actually that useful an exercise.
 1. Bearing in mind that boardsAllowedTo() returns array(0) for admins so a simple in_array won't work either.
« The Regular Members group
Image resizing for non-uploads »