Wedge

Arantor · « Reply #90, on February 22nd, 2012, 02:51 PM »

Quote

Have you got some thoughts to share on all of this, Pete..?

Oh yes. :D

Quote

The #1 issue with all this is that there are too many things to go through if you don't know better.

That's a very big issue, yes. That's why for example r1373 (and 1374) was necessary.

Quote

Have I got...

To a point the mod cache is not significantly useful but important - because it exemplifies the point I've been going nuts over: board permissions that apply outside of a board context in some fashion, that have a bearing on access even outside of their board.

For the most part, you only need to be bothered with standard permissions checks (allowedTo style) because it's surprisingly infrequently that you worry about board permissions outside of a board context. Even in SimpleDesk where I had the same situation to encounter, even there it actually was not a problem in the same fashion (and there was no such thing as general vs 'board' permissions, everything was implied to be a board permission of sorts)

But in SD's case, I didn't have separate logical structures to contain things - when you loaded permissions, you did so in a single hit, and loaded access to every board's permissions in a single hit, meaning you always knew which departments you had what permissions for.

Quote

Am I a moderator on this board?

This is a special case by definition because you can be a moderator on a board without being a board moderator (and without being a global moderator) It is this level of complexity that actually makes SMF so difficult to configure, even if conceptually/codewise using the permissions system is made so easy.

Quote

At this point, it becomes really unclear what 'can_mod' is for, why 'is_mod' is reset in SSI

can_mod, supposedly, is 'I can moderate somewhere' while is_mod is 'I can moderate *here*'. Primarily IIRC can_mod was supposed to be used to control access to the moderation centre but this changed in RC4 when fixing a related bug.

Quote

There's a strong, huge feeling that SMF has been overhauled many times when it comes to permissions, and once again different developers had their own idea of how to make it 'simpler', and failed to make them commonly used.

Firstly, there have actually not been that many overhauls - there's 1.x and there's 2.x permissions from what I remember of 1.0 vs 1.1. There are three permissions related changes in 2.x vs 1.x that are of relevance to us:

* profiles
* post approval
* the mod_cache

(The whole simple vs classic thing is purely cosmetic. It's also stupid, but that's a debate for another day.)

The whole profiles thing is interesting. I actually remember an argument I had with Antechinus about that[1] over reverting to 1.1.x local board permissions vs global ones, but honestly that's about as confusing! The reason for the argument was because of an important lack of understanding - there was a *reason* profiles were adopted instead of local/global board permissions, and that was to simplify applying the same permissions to multiple boards at once.[2] It did, incidentally, fix a few bugs in 1.1.x's 'profiles' as they stood, too.

Fortunately for the most part, the profiles mechanism doesn't really make that much difference for us in terms of mechanics, because whatever we do, it will probably use some of the same infrastructure.

Then there's post approval. The whole permission implementation of that had some convenience-for-UI-building factors (and still does, in ways I have not yet solved for moderation filters) but it causes the problem we've been bouncing around here - that you need to have some conceptualisation of board-level permissions being applied to a context outside of boards.

Prior to post approval's introduction, there was never a context in SMF where you'd ever need to understand board level permissions properly in places that have a meta board context - yes, search/unread/unreadreplies/recent had *some* idea of board level permissions, for whether you could reply to a post or edit it or whatever, but all that was doable after the event, as it were - you only had to worry about board *access* to see posts, not a permission attached to whether you could see an individual post.

This brings us to the mod cache - whose sole existence, if I'm any judge, is to mitigate the very performance issues that I'm getting at. Now, here's the thing: the reason it's not used that much in SMF is simply because there are that few places where having board-level permissions outside of a board just doesn't make much difference. Primarily it supports post approval - but the rest are nice convenience features, and that allowedTo() is probably more important.

So, where does that leave us? It leaves us with a permissions system that I've wanted to overhaul for a while - because SMF's permissions system is unintuitive. It's very, very powerful but the price paid is the complexity of actually trying to use it - and almost everyone I know (including me) has misused it and used it incorrectly to do things that shouldn't be done that way.

For example, if you have two or three 'teams' that are all, practically speaking, global moderators but have different badges, the correct thing to do is make new groups for each team that all inherit from a parent group so you manage the permissions centrally - but most don't.

The really sad part is, I don't have an answer yet. I've had bits of answers floating around and my gut instinct is to dump SMF's permissions core and merge in SimpleDesk's in some respects but that's not really where I want to go with this.

What is clear is that SMF's permissions are broken, and a lot of that is down to perception: as I identified when figuring out SD's permissions, a lot of the problems in SMF are really caused by people conflating groups and roles, and building on that incorrect premise. It's almost like we need to separate groups from permissions properly.

1.	It was in that conversation I realised how doomed SMF really was.
2.	The point of the argument is that to fix a problem you must understand how the problem came about. Going backwards doesn't mean you go forwards if you don't understand what you're undoing in the process.

Nao · « Reply #91, on February 22nd, 2012, 03:58 PM »

Quote from Arantor on February 22nd, 2012, 02:51 PM

This is a special case by definition because you can be a moderator on a board without being a board moderator (and without being a global moderator) It is this level of complexity that actually makes SMF so difficult to configure, even if conceptually/codewise using the permissions system is made so easy.

I agree that it brings confusion, definitely. I'm just wondering what board moderators are allowed to do that is, or is not included in the moderate_board permission... :-/ If anything, is it simply a 'credit'? (i.e. your name in the breadcrumb. Oh BTW we need to move these to the sidebar...)

Quote

can_mod, supposedly, is 'I can moderate somewhere' while is_mod is 'I can moderate *here*'. Primarily IIRC can_mod was supposed to be used to control access to the moderation centre but this changed in RC4 when fixing a related bug.

'can_mod' => allowedTo('access_mod_center') || (!$user_info['is_guest'] && ($user_info['mod_cache']['gq'] != '0=1' || $user_info['mod_cache']['bq'] != '0=1' || ($settings['postmod_active'] && !empty($user_info['mod_cache']['ap'])))),

can_mod is "do I have access to the moderation center[1], or am I allowed to moderate at least one membergroup or board, or approve posts to some extend?"

That's a hell of a vague setting to me...

And the ONLY place it's used (although it's repeated a couple of times), is whether the user can see the Warning button on a userbox...
That smells like "REMOVE!" to me...

Quote

there was a *reason* profiles were adopted instead of local/global board permissions, and that was to simplify applying the same permissions to multiple boards at once.

The UI could be better, I think.
For instance - just an idea - one could simply 'hide' them profiles at first. You set up permissions for a board. Then when you create or edit another board, you get the ability to apply another board's permissions. At which point, its permission profile is used. Otherwise, a new permission profile is created for the new board. Only problem is the 'default' name for these profiles.

Quote

This brings us to the mod cache - whose sole existence, if I'm any judge, is to mitigate the very performance issues that I'm getting at.

I love how you are quick to adapt! Earlier today you said you'd never heard about it. Now you *clearly* know more about it than I do, and I've been studying it since yesterday night ;) I'm getting old I suppose...

Hey Aaron, this one's for you... There's Doctor Who on the telly (France 14 channel), dual language, I switched to English of course, and just a minute ago the Doctor said his line from your signature, "after all this time..." It made me smile :)

Quote

For example, if you have two or three 'teams' that are all, practically speaking, global moderators but have different badges, the correct thing to do is make new groups for each team that all inherit from a parent group so you manage the permissions centrally - but most don't.

There's another thing with inherited board permissions. We absolutely need to document this in a Help icon or something. Have something right next to the option list, saying what they'll do. I always, ALWAYS get confused between copied permissions and inherited permissions (i.e. nothing is copied, just using the original's on the fly). Perhaps we ought to rename the terminology. "Copy permissions" and "Mirror permissions", maybe...

Quote

The really sad part is, I don't have an answer yet. I've had bits of answers floating around and my gut instinct is to dump SMF's permissions core and merge in SimpleDesk's in some respects but that's not really where I want to go with this.

We'll probably have to leave it at that for the first versions. There's no time to deal with that, and the SMF importer will get the original permission profiles anyway, so people are likely to have already solved their permission problems already... (It's for new users that it gets annoying.)

1.	Instead of the other way around... In that respect, I don't know why Load.php has $context['allow_moderation_center'] = $context['user']['can_mod']; at some point when it should be using the permission, directly...

Arantor · « Reply #93, on February 22nd, 2012, 04:19 PM »

Quote

I'm just wondering what board moderators are allowed to do that is, or is not included in the moderate_board permission...

They get their name on the linktree, on the board index and their badge is altered on the fly. They're also, silently, added to group 3 when in that board, which means you can do permissions checking in that respect.

Quote

can_mod is "do I have access to the moderation center[1], or am I allowed to moderate at least one membergroup or board, or approve posts to some extend?"

Its origin runs a little deeper than that. Historically, access to the moderation center really was designed to be based on having any one of those permissions, then you had the somewhat odd structure of having the actual 'view the moderation centre' permission, which could be overridden by other permissions, sometimes for features not even enabled (since global moderator actually had the approve permission applied even if approval was disabled)

Quote

Instead of the other way around... In that respect, I don't know why Load.php has $context['allow_moderation_center'] = $context['user']['can_mod']; at some point when it should be using the permission, directly...

Again that's the historical aspect. It was never supposed to be entirely based on the permission but showing you the area if you could do at least one thing there - much as you get access to the admin tab if you can do any of the admin like features (e.g. news, manage permissions, manage membergroups etc.)

Quote

And the ONLY place it's used (although it's repeated a couple of times), is whether the user can see the Warning button on a userbox...
That smells like "REMOVE!" to me...

DOOOO EEEET. Basically, yeah, it was a historical thing that was never quite realised the way it was intended.

Quote

The UI could be better, I think.
For instance - just an idea - one could simply 'hide' them profiles at first. You set up permissions for a board. Then when you create or edit another board, you get the ability to apply another board's permissions. At which point, its permission profile is used. Otherwise, a new permission profile is created for the new board. Only problem is the 'default' name for these profiles.

Which is virtually how 1.1.x did it, except it still created proper per-board permissions rather than having discrete profiles.

Quote

I love how you are quick to adapt! Earlier today you said you'd never heard about it.

I wasn't very clear on that point. I knew of the mod cache subsystem, but I wasn't aware of all the details and precisely what it covered - namely the ap element. But combine the places it is used, with the knowledge of what it actually contains and it becomes rather clear (to me at least) what it was intended for. It's like having 3/4 of the pieces of the puzzle and then suddenly you find that last piece and it all suddenly makes sense.

Quote

There's another thing with inherited board permissions. We absolutely need to document this in a Help icon or something. Have something right next to the option list, saying what they'll do. I always, ALWAYS get confused between copied permissions and inherited permissions (i.e. nothing is copied, just using the original's on the fly). Perhaps we ought to rename the terminology. "Copy permissions" and "Mirror permissions", maybe...

Oh, it's horribly confusing, which is why I didn't even implement it in SimpleDesk but left it simply as the ability to copy a role, rather than to mirror it.

The other thing with the mirrored permissions is that most mods for SMF never used them properly - not even SimpleDesk got it right, and I don't believe Aeva did either. (Case in point, I reordered all the groups on my one site to have a bunch of groups, one per user, for unique badges/titles because I'm like that. Each of these new groups was based on a higher group that controlled permissions - but neither SD nor Aeva responds to the child groups properly and I had to end up assigning the permissions in both to all the groups, not just the parent group.)

Quote

We'll probably have to leave it at that for the first versions. There's no time to deal with that, and the SMF importer will get the original permission profiles anyway, so people are likely to have already solved their permission problems already... (It's for new users that it gets annoying.)

Whatever happens, it's going to screw someone up. Gut instinct tells me to fix it sooner rather than later.

Quote

leave 'groups' in as a possibility? Can always be removed later...

I like the idea but I'm concerned it would potentially lead to greater complexity. I'm also not sure you can actually optimise it *that* far, though, and there's a lot of places that use ?topic in the URL to handle permission loading.

Quote

- add a 'privacy_id' field?
- best solution for performance? What should the extra index(es) look like?

Storing it in the topics table is going to hurt. Storing it outside the topics table is going to hurt too, just in a different way, and neither is lightly ignored.

Putting it in the table as a textual id with a list of groups means the query becomes WHERE t.privacy = something OR (t.privacy = 'groups' AND FIND_IN_SET(...)) which is expensive enough.

Meanwhile, putting it in a separate table means either a subquery, or a join, both of which being applied to potentially every topic in a board (or worse) to validate access is going to get very unfriendly very quickly.

I don't think you're crazy, but I also don't think there's a good way to achieve this meaningfully for topics.

Regarding applying it to thoughts, I'd actually be inclined to handle that in permissions (permission to post, permission to read, permission to read replies, permission to reply, etc.)

Nao · « Reply #94, on February 22nd, 2012, 05:40 PM »

Quote from Arantor on February 22nd, 2012, 04:19 PM

They get their name on the linktree, on the board index and their badge is altered on the fly.

Which I don't like, BTW... On wedge.org/new, we no longer have our Developer badges, because we're moderators. It sucks.

Quote

They're also, silently, added to group 3 when in that board, which means you can do permissions checking in that respect.

What permission checking? You mean do in_array(3, $user_info['groups'])?
Don't they automatically get all post/topic-related permissions, more simply?

Quote

Again that's the historical aspect. It was never supposed to be entirely based on the permission but showing you the area if you could do at least one thing there - much as you get access to the admin tab if you can do any of the admin like features (e.g. news, manage permissions, manage membergroups etc.)

Sure, but the admin area (which was never just only about administrators) isn't like the mod center, which was designed specifically to allow people to deal with whatever sub-par permissions they had, without giving them access to the admin panel.

Quote

Quote
And the ONLY place it's used (although it's repeated a couple of times), is whether the user can see the Warning button on a userbox...
That smells like "REMOVE!" to me...
DOOOO EEEET. Basically, yeah, it was a historical thing that was never quite realised the way it was intended.

As for the warning button...
Heck.
allowedTo('issue_warning').
This must be some kind of a joke... I mean, moderators get the 'right' to see the Warning button, but then if they try to click it (from what I gather), they'll get kicked out because area=issuewarning only checks for issue_warning perms to be set...
And AFAIK, this kind of permission isn't given automatically to moderators.

Quote

Which is virtually how 1.1.x did it, except it still created proper per-board permissions rather than having discrete profiles.

Yeah, and it's just simpler in appearance, but still the same under the hood...

Quote

I wasn't very clear on that point. I knew of the mod cache subsystem, but I wasn't aware of all the details and precisely what it covered - namely the ap element.

Makes more sense to me, coming from you... :P

Quote

But combine the places it is used, with the knowledge of what it actually contains and it becomes rather clear (to me at least) what it was intended for. It's like having 3/4 of the pieces of the puzzle and then suddenly you find that last piece and it all suddenly makes sense.

So, what about it then...? Shall we use it in query_see_topic?
Tell you what. How about you look into it a bit? Now that my work is committed, you may want to simply deal with it by yourself, because nobody will deny you the fact that you're one of the most knowledgeable SMF/Wedge devs when it comes to permissions. And optimization.

Quote

Oh, it's horribly confusing, which is why I didn't even implement it in SimpleDesk but left it simply as the ability to copy a role, rather than to mirror it.

Mirroring is still a great thing. I mean, it allows you to give someone a different membergroup name, even if it's the same group in the end.

So, do you think we should rename..?

Quote

The other thing with the mirrored permissions is that most mods for SMF never used them properly - not even SimpleDesk got it right, and I don't believe Aeva did either.

Probably yeah. Although Shitiz did most of the coding on permissions... :niark:

Quote

(Case in point, I reordered all the groups on my one site to have a bunch of groups, one per user, for unique badges/titles because I'm like that. Each of these new groups was based on a higher group that controlled permissions - but neither SD nor Aeva responds to the child groups properly and I had to end up assigning the permissions in both to all the groups, not just the parent group.)

Oh... Bad Aeva, bad :-/

Dragooon, are you reading this?

Quote

I like the idea but I'm concerned it would potentially lead to greater complexity.

Complexity internally? I don't think so. In the UI? Definitely. But it can also be 'fixed', like I said... By only giving membergroups and contact lists that are available to the user. For instance, if a forum has dozens of special groups but all default members are in the default group, then they won't see the option at all -- Wedge can easily remove the 'groups' choice because it's either Members or Guests for them...

Quote

I'm also not sure you can actually optimise it *that* far, though, and there's a lot of places that use ?topic in the URL to handle permission loading.

I'm not sure what this implies.

Quote

Storing it in the topics table is going to hurt. Storing it outside the topics table is going to hurt too, just in a different way, and neither is lightly ignored.

Ah, I thought it might be the case...

Quote

Putting it in the table as a textual id with a list of groups means the query becomes WHERE t.privacy = something OR (t.privacy = 'groups' AND FIND_IN_SET(...)) which is expensive enough.

No find_in_set, because I could see that, in order to keep it simple, we could instead allow only one group for privacy... Although I could imagine people wanting to send to multiple contact lists, which sort of defeats the concept of having a select box for privacy... Argh. (Well, maybe that'd be a good opportunity to implement checkboxes in our dropdowns... :lol:)

When it comes to contact lists, would be a sub-select. (Pretty much like what UltimateProfile does...)

Quote

Meanwhile, putting it in a separate table means either a subquery, or a join, both of which being applied to potentially every topic in a board (or worse) to validate access is going to get very unfriendly very quickly.

Fun...

Now, of course, this performance issue would only happen for members (guests and admins are easily discarded), and only for topics/thoughts where a non-default choice is set... (And when it comes to topics, it's always fast enough to scan that table innit?)

Quote

I don't think you're crazy, but I also don't think there's a good way to achieve this meaningfully for topics.

SMF team already complained enough about the performance killer that is m.approved... Heck, it's just a simple flag...!

Quote

Regarding applying it to thoughts, I'd actually be inclined to handle that in permissions (permission to post, permission to read, permission to read replies, permission to reply, etc.)

I don't see how/why...

Oh, another possibility...

topic has privacy set to contact list = 42.
user is in contact lists 41 and 42.
user wants to view. Test is done on "privacy=contacts_41 OR privacy=contacts_42". Well, obviously that would make a VERY long query if user was in thousands of contact lists, but then we're pretty sure it's only ONE field that's indexed... Even if it's a string field by now.

:^^;:

Arantor · « Reply #95, on February 22nd, 2012, 06:06 PM »

Quote

Which I don't like, BTW... On wedge.org/new, we no longer have our Developer badges, because we're moderators. It sucks.

There is an override in loadMemberData that if you're an admin, the badge won't be overridden by the moderator badge.

Quote

What permission checking? You mean do in_array(3, $user_info['groups'])?
Don't they automatically get all post/topic-related permissions, more simply?

They do get all the related permissions automatically, but on top of that a plugin or whatever can expressly check for being a moderator with that in_array check, should they want to do so (as opposed to checking allowedTo(moderate_board))

Quote

Sure, but the admin area (which was never just only about administrators) isn't like the mod center, which was designed specifically to allow people to deal with whatever sub-par permissions they had, without giving them access to the admin panel.

But it does give them the admin panel - just the bits they have access for. Give a group access to edit news, and the only thing they see in action=admin is the news option. The mod centre was designed to work the same way in theory but practice doesn't agree.

Quote

This must be some kind of a joke... I mean, moderators get the 'right' to see the Warning button, but then if they try to click it (from what I gather), they'll get kicked out because area=issuewarning only checks for issue_warning perms to be set...
And AFAIK, this kind of permission isn't given automatically to moderators.

Actually, in a base install it is - subject to the feature being enabled.

Quote

Yeah, and it's just simpler in appearance, but still the same under the hood...

Well, 1.1.x's code and 2.x's code are rather different (and the installer makes a royal mess of it, too, creating a new profile for each and every board that has local permissions) but the concept is workable.

Quote

Complexity internally? I don't think so. In the UI? Definitely.

UI definitely. Internally... very probably, because of hoops you end up jumping through to perform the tests and still keep it fast.

Quote

So, what about it then...? Shall we use it in query_see_topic?

Hell yes. The biggest complaint I've had is taking into account approve-posts permission - and there it is, right there, pre-cached and 'dealt with' already.

Quote

Tell you what. How about you look into it a bit? Now that my work is committed, you may want to simply deal with it by yourself, because nobody will deny you the fact that you're one of the most knowledgeable SMF/Wedge devs when it comes to permissions. And optimization.

Permissions, yes, optimisation, maybe. There are people who know MySQL better than I do in the SMF fold (Vekseid, Mark Rose to name two)

Also, I thought you were still working on it at present?

Quote

Mirroring is still a great thing. I mean, it allows you to give someone a different membergroup name, even if it's the same group in the end.

So, do you think we should rename..?

I think renaming it would help, but I think overhauling the UI will help even more in the long run - but the rename is good for now.

Quote

I'm not sure what this implies.

The implication is that a surprising amount of places simply do not use the underlying functions to confirm access at all. I discovered this last night - Display for example does not itself do {query_see_board} in the topic to validate access to the topic, it relies on loadBoard having already done it. Point is... make *sure* loadBoard is working correctly as far as access to boards goes, and to topics if a topic is specified, and that has a lot of knock-on effects, not just for display, but anywhere ?topic is in the URL (e.g. attachments, remove topic, just to name two)

Quote

Although I could imagine people wanting to send to multiple contact lists, which sort of defeats the concept of having a select box for privacy...

See, the notion of having a single group or contact list never actually occurred to me, I assumed right off the bat that multiple lists was the aim.

Quote

When it comes to contact lists, would be a sub-select. (Pretty much like what UltimateProfile does...)

I'm not nearly familiar enough with what UP offers, perhaps I should learn! But yes, it's looking like a sub-select or a join, which can get creaky if not done carefully. I foresee lots of experimentation on this one.

Quote

Now, of course, this performance issue would only happen for members (guests and admins are easily discarded), and only for topics/thoughts where a non-default choice is set... (And when it comes to topics, it's always fast enough to scan that table innit?)

Right now, it's 'relatively' cheap. It's still fixed-length rows, but the minute a text field is added to it to contend with the whole aspect of multiple groups attached to it, it's going to get expensive (and the alternatives, like SET columns, aren't really good alternatives). Also note that MySQL is still not too hot about optimising OR branches; it can optimise AND branches cleanly enough but it still ends up doing a lot of work for OR cases.

Quote

SMF team already complained enough about the performance killer that is m.approved... Heck, it's just a simple flag...!

I understand the nature of their complaint on it - being a binary state field, you can't actually optimise it that heavily in an index. An index works best if it's selective, or at least sufficiently distributed such that picking a value excludes a sizeable percentage of the rest of the table. approved being a two-state field is never going to exclude vast amounts of the tableset so you never get any advantage out of indexing it.

We'd actually get more benefit from keeping an accurate track of moderated posts in a topic and topics with moderated posts in a board - so if there's nothing that has pending moderation status in that board or topic, don't perform that test.

Quote

I don't see how/why...

Because you don't have to expressly store it in the table then. For example, if you handle 'view replies' as a permission, you can simply exclude massive portions of the table, very cheaply.

Quote

topic has privacy set to contact list = 42.
user is in contact lists 41 and 42.
user wants to view. Test is done on "privacy=contacts_41 OR privacy=contacts_42". Well, obviously that would make a VERY long query if user was in thousands of contact lists, but then we're pretty sure it's only ONE field that's indexed... Even if it's a string field by now.

The test is really (privacy = contact AND privacy-list IN (user contact lists)), isn't it? Since the user's contact list will be part of the user's own data and thus available for query insertion?

Nao · « Reply #96, on February 23rd, 2012, 08:26 AM »

Quote from Arantor on February 22nd, 2012, 06:06 PM

There is an override in loadMemberData that if you're an admin, the badge won't be overridden by the moderator badge.

I still don't like it. I don't like that you're forced to wear a Moderator badge when you should have the Developer or Boss badge. Teaches people some respect :P

Quote

They do get all the related permissions automatically,

Are you sure really...? I tried looking for 'issue_warning' for instance, and neither Load.php or Subs.php have it. Only ManagePermissions.php which seems to do some voodoo about it, but what does it mean? That once you're set as a board moderator, these permissions are assigned to you automatically, hard-wired style, visible in the database and all? And that these permissions are removed when your status is removed?

Quote

but on top of that a plugin or whatever can expressly check for being a moderator with that in_array check, should they want to do so (as opposed to checking allowedTo(moderate_board))

Confusing, really... More confusing than the allowedTo().

Quote

Hell yes. The biggest complaint I've had is taking into account approve-posts permission - and there it is, right there, pre-cached and 'dealt with' already.

Yep... Although it does get complicated, query-wise.

Can you share your opinion on this? It's untested...

Code: [Select]

<?php
$user_info['can_skip_approval'] = empty($settings['postmod_active']) || allowedTo(array('moderate_forum', 'moderate_board', 'approve_posts'));
$user_info['query_see_topic'] = '
(
	t.id_member_started = ' . $uid . ' OR (' . ($user_info['can_skip_approval'] ? '' : (empty($user_info['mod_cache']['ap']) ? '
		t.approved = 1' : '
		(t.approved = 1 OR t.id_board IN (' . implode(', ', $user_info['mod_cache']['ap']) . '))') . '
		AND') . '
		(
			t.privacy = \'default\'
			OR (t.privacy = \'members\')
			OR (
				t.privacy = \'contacts\'
				AND t.id_member_started != 0
				AND FIND_IN_SET(' . $uid . ', (SELECT buddy_list FROM ' . $db_prefix . 'members WHERE id_member = t.id_member_started))
			)
		)
	)
)';

I have a headache -- literally this time. So, hard to focus.

Quote

Also, I thought you were still working on it at present?

Well, no, as you see in rev 1375, I decided to commit whatever I had in the works, so that you could help with it if you wanted...
After all, it's not like adding query_see_topic will break anything. It can be done gradually :)

Quote

I think renaming it would help, but I think overhauling the UI will help even more in the long run - but the rename is good for now.

And how to rename? Can you do it for me? You're the English speaker ;) I wouldn't be able to determine what's best, between "mirror from profile..", "mirror profile", "alias of profile..." or whatever makes sense.

Quote

The implication is that a surprising amount of places simply do not use the underlying functions to confirm access at all. I discovered this last night - Display for example does not itself do {query_see_board} in the topic to validate access to the topic, it relies on loadBoard having already done it.

Hmm, well... As long as it works...

Quote

Point is... make *sure* loadBoard is working correctly as far as access to boards goes, and to topics if a topic is specified, and that has a lot of knock-on effects, not just for display, but anywhere ?topic is in the URL (e.g. attachments, remove topic, just to name two)

I'm afraid I still don't get it. But don't bother...

Quote

See, the notion of having a single group or contact list never actually occurred to me, I assumed right off the bat that multiple lists was the aim.

Yes, always been. Something that Noisen doesn't have. Noisen has group owners, which is essentially the same, only easier to implement eventually because it just requires adding an id_owner to the groups table and giving admin access to the manage membergroups page. But, for some reason that escapes me, I ended up removing the feature (possibly because it was seen as too complex?), so here are are.
I just said that if we have to store an extra integer for the contact list ID, then we might as well enable Wedge to store a group ID in that same place...

Quote

I'm not nearly familiar enough with what UP offers, perhaps I should learn!

Not worth it :P Let's just say that UP has three good things: a decent gallery system (but AeMe, which came later, is way more developed), a nice profile layout (we'll get to it eventually), and a more complex buddy system, where there's an extra table with asynchronous friend relationships. (Unless I added that manually on Noisen, but I don't think so. I did add a "hidden" flag manually, which is definitely gonna be in Wedge.) Thing is, many mods use the buddy_list field in the members table, so the guy was smart enough to keep the friend list in both places, refreshing them at the same time. Best of both worlds.

Quote

Right now, it's 'relatively' cheap. It's still fixed-length rows, but the minute a text field is added to it to contend with the whole aspect of multiple groups attached to it, it's going to get expensive (and the alternatives, like SET columns, aren't really good alternatives).

I see... I imagined that varchar fields only made the index bigger, not slower, than an integer field.

Quote

I understand the nature of their complaint on it - being a binary state field, you can't actually optimise it that heavily in an index. An index works best if it's selective, or at least sufficiently distributed such that picking a value excludes a sizeable percentage of the rest of the table. approved being a two-state field is never going to exclude vast amounts of the tableset so you never get any advantage out of indexing it.

Hmm. So the same problem awaits us. A vast majority of topics and thoughts are gonna be public, so granularity or whatever will not be to our advantage...

Quote

We'd actually get more benefit from keeping an accurate track of moderated posts in a topic and topics with moderated posts in a board - so if there's nothing that has pending moderation status in that board or topic, don't perform that test.

Hmm...!

Quote

Because you don't have to expressly store it in the table then. For example, if you handle 'view replies' as a permission, you can simply exclude massive portions of the table, very cheaply.

Still don't get what it has to do with thoughts...?!

Quote

The test is really (privacy = contact AND privacy-list IN (user contact lists)), isn't it? Since the user's contact list will be part of the user's own data and thus available for query insertion?

Err, yes? I suppose so...
So it's faster, then? Indexable?

(Sorry, technical issue caused bump :P)

Arantor · « Reply #97, on February 23rd, 2012, 10:50 AM »

Quote

I still don't like it. I don't like that you're forced to wear a Moderator badge when you should have the Developer or Boss badge. Teaches people some respect

Maybe that suggests a rethink of how the moderator badge is applied is in order. Perhaps, even, have it in addition to (rather than instead of) the standard badge?

Quote

Are you sure really...? I tried looking for 'issue_warning' for instance, and neither Load.php or Subs.php have it. Only ManagePermissions.php which seems to do some voodoo about it, but what does it mean? That once you're set as a board moderator, these permissions are assigned to you automatically, hard-wired style, visible in the database and all? And that these permissions are removed when your status is removed?

They won't, it's a standard permission. But specifically it is a general permission, not a board one, so board level moderators never get the chance to have it in the first place (because that's the quirk of group 3 - it never has general permissions)

So yes, it's a bug - they have the power to see the warn button but not to actually use it.

Quote

Confusing, really... More confusing than the allowedTo().

Sure it is, but as group 3 is special in other ways you can expressly do things with it that you can't with a bare allowedTo() check.

Quote

Can you share your opinion on this? It's untested...

The only issue I have is with the FIND_IN_SET - but there's no way around it.

Quote

After all, it's not like adding query_see_topic will break anything. It can be done gradually

I'm not used to features that can be added gradually :P

Quote

I wouldn't be able to determine what's best, between "mirror from profile..", "mirror profile", "alias of profile..." or whatever makes sense.

I'll think about it, got a lot of personal stuff going on today :(

Quote

Hmm, well... As long as it works...

It does now...

Quote

I'm afraid I still don't get it. But don't bother...

Point is: some queries that look like they need query_see_topic actually don't, because loadBoard should take care of them, or at least if loadBoard takes care of them, other places don't have to as much.

Quote

But, for some reason that escapes me, I ended up removing the feature (possibly because it was seen as too complex?), so here are are.

This is the part that concerns me.

Quote

I just said that if we have to store an extra integer for the contact list ID, then we might as well enable Wedge to store a group ID in that same place...

*nods* But that's only really true for holding a single value - the minute you start trying to hold a relationship in there it's going to go to hell.

Quote

Let's just say that UP has three good things

See, I didn't even know that, never really looked.

Quote

I see... I imagined that varchar fields only made the index bigger, not slower, than an integer field.

Nope. Actually, a text field is better than a varchar in one way and worse in another.

Here's the deal: if all the rows are fixed length (numeric, or specifically char fields), the table is faster - because it can use the index to identify a row and get there very quickly, as it doesn't have to go row by row to find it, it can just say 'row abc x bytes-per-row' and get to its position. (Since the position is not recorded in the index, or at least it never used to be)

But with variable length fields, it can't pre-calculate the position and has to go through the table to find them. While there are ways to speed that up, it's still not as fast as fixed-width rows.

Text/blob fields are special. As far as fixed/variable width rows are concerned, they're actually fixed, because they only contain a reference within a secondary data area where the content is located. That's how, for example, you transcend certain limits (a row in MyISAM at least can only be 64K in total, and a text field counts two bytes towards that, but only two bytes regardless of its actual length) - and that's not a performance issue in of itself when dealing with tables with text/blob fields.

The problem comes in when you query a table with such records, and actually retrieve the text/blob. In those situations, if you do any ordering, it triggers a filesort rather than an in-memory table.

Quote

Hmm. So the same problem awaits us. A vast majority of topics and thoughts are gonna be public, so granularity or whatever will not be to our advantage...

Yup. But in the case of thoughts at least, depending on permissions checks (which are basically free in DB terms), you can exclude things on a more granular nature, rather than on a permissions flag. Consider, if you do 'permission to view thoughts' and 'permission to view replies', the former obviates any need to query at all, the latter means you either do a query without a WHERE clause, or you do a query with a nicely broad (and indexable) WHERE clause, i.e. WHERE id_parent = 0

Quote

Err, yes? I suppose so...
So it's faster, then? Indexable?

Oh... I just realised I'm a doofus (based on your earlier post), because I mistakenly thought it was the current user's friend list which is already loaded and available, rather than the topic starter's (which can't be done without some kind of lookup)

Nao · « Reply #98, on February 23rd, 2012, 11:40 AM »

Before I get to this long post which I haven't had time to read yet -- I noticed that many m.approved calls in Wedge do not test for postmod_active before, unlike ALL of the t.approved calls. I'm thinking this *might* be a problem...?

Arantor · « Reply #99, on February 23rd, 2012, 11:45 AM »

Actually, it's only a problem since I messed with it. In the old code, disabling post moderation would actually silently force everything unapproved to become approved. Now it's a discrete option you have to use should you want to do so - but the odds of that happening are actually surprisingly slim (since it requires removing every moderation filter not just ones that trigger moderation)

Nao · « Reply #**100**, on February 23rd, 2012, 11:58 AM »

Hmm... Are you implying that, if m.approved is always 0 (i.e. post mod never enabled), the performance penalty is actually 0, too? In which case, yes, I suppose it's fine to leave it in, but then maybe it should have been the case for t.approved as well...

Arantor · « Reply #**101**, on February 23rd, 2012, 12:02 PM »

Sadly, no. If it never checks for being enabled or not, but always checks anyway, there's a performance hit attached. It's just not one you can avoid by turning the feature off...

That's really more of an SMF issue; in their case, post moderation is a very specific feature, disabled by default, so you get penalised by default whether you use it or not.

In our case, I'm really expecting users to use it more than SMF users make use of post moderation, meaning that while you're still getting penalised for that query overhead, at least it's done mindfully of the fact that moderation is likely to be in place.

Nao · « Reply #**102**, on February 23rd, 2012, 03:34 PM »

Quote from Arantor on February 23rd, 2012, 12:02 PM

Sadly, no. If it never checks for being enabled or not, but always checks anyway, there's a performance hit attached. It's just not one you can avoid by turning the feature off...

Well, if it's turned off, m.approved should no longer be tested against, should it...?

Quote

That's really more of an SMF issue; in their case, post moderation is a very specific feature, disabled by default, so you get penalised by default whether you use it or not.

...Unless you never enable it..?

I'm not exactly sure I remember what changes you made to the approval system, if any...?

Quote

In our case, I'm really expecting users to use it more than SMF users make use of post moderation,

Is it enabled by default in here?

Arantor · « Reply #**103**, on February 23rd, 2012, 03:41 PM »

Quote

Well, if it's turned off, m.approved should no longer be tested against, should it...?

No, it shouldn't.

Quote

...Unless you never enable it..?

No, you're still penalised either way. If you never check for it at all, you're basically assuming it's enabled whatever - and with the performance issues attached.

Quote

Is it enabled by default in here?

No. But in any case, moderation filters is quite a different beast. The way postmod_active works now is that if there are any rules to be applied (of any kind), postmod_active is true. No rules to apply (of any kind) = no post moderation. So there's a performance gain potential if you *never* use that feature.

However, I foresee people using it in all kinds of funky ways, and as such I think it's likely to be 'on' (in SMF terms) more often.

Nao · « Reply #**104**, on February 23rd, 2012, 03:53 PM »

Quote from Arantor on February 23rd, 2012, 03:41 PM

Quote
Well, if it's turned off, m.approved should no longer be tested against, should it...?
No, it shouldn't.

Well, that's what I'm saying: it is... It's tested whatever the postmod_active state.
BTW, where the hell do I enable postmod?! Now that core features are gone... Well, I even made an admin search and didn't find any 'master' setting?!
(Okay, see below...)

Quote

No, you're still penalised either way. If you never check for it at all, you're basically assuming it's enabled whatever - and with the performance issues attached.

...I don't get it. If testing for m.approved=0, m.approved=1 or nothing at all, same performance. How can it be, since in the last case we're sure not to ever use the approved index...?

Quote

No. But in any case, moderation filters is quite a different beast. The way postmod_active works now is that if there are any rules to be applied (of any kind), postmod_active is true. No rules to apply (of any kind) = no post moderation. So there's a performance gain potential if you *never* use that feature.

Hmm... That does seem like something that might confuse people. It confuses even me...
Is it working, at least? Last time you didn't have the Save feature for it, so I left it aside but now I need to have postmod enabled just to test my code... :P

BTW, in Display.php, if you are in a topic that has approved=0 but postmod is disabled, you get an error message saying max() needs at least one entry. That's on line 838... I suppose that means $messages is empty even if I'm an admin and I can see said topic...
Which brings me to... Heck, can anyone actually view an unapproved topic if they don't have permission to, but they know its ID? Because I can't see any code to stop them in Display.php! Actually noisen.com/wedge.org do have such code, that's where it gets funny. Well, they don't add approved flags to the mix but at least they support 'justme' privacy.
I suppose I'll add query_see_topic to the query at line 122... But it's quite scary all of a sudden. I thought I was finished and I only had the UI left to do, but I WILL definitely have to go through every single query_see_topic occurrence in the noisen patch to determine whether it's needed... :^^;:

Wedge

Home

Login

Register

Would you like to have topic privacy options in Wedge?

Arantor

Re: Privacy options

Nao

Re: Privacy options

Re: Privacy options

Arantor

Re: Privacy options

Nao

Re: Privacy options

Arantor

Re: Privacy options

Nao

Re: Privacy options

Arantor

Re: Privacy options

Nao

Re: Privacy options

Arantor

Re: Privacy options

Nao

Re: Privacy options

Arantor

Re: Privacy options

Nao

Re: Privacy options

Arantor

Re: Privacy options

Nao

Re: Privacy options