Forums

Unfortunately no one can be told what FluxBB is - you have to see it for yourself.

You are not logged in.

#1 2014-07-24 18:05:05

Sxderp
Member
Registered: 2012-11-02
Posts: 92

TODO: isAdmMod()

I was just going through the Fluxbb2 code and came across a TODO: message.

// TODO: Is this even necessary or is a better check for is_moderator() (that returns true for admins, too) better?

I agree with the TODO: in that isMod should be the only function required (returning true for admins as well). I see no reason to separate the functions as I can think of no circumstance where you would want an mod to do something but not an admin.

Also, I suggest renaming isModerator to isMod.
Just because it's less characters and it still conveys what is being checked.

Last edited by Sxderp (2014-07-24 18:05:23)

Offline

#2 2014-07-24 19:49:33

chris98
Member
From: England, United Kingdom
Registered: 2013-05-31
Posts: 906
Website

Offline

#3 2014-07-24 21:05:50

Franz
Lead developer
From: Germany
Registered: 2008-05-13
Posts: 6,047
Website

Re: TODO: isAdmMod()

Yeah, I really need to go through those TODOs, shouldn't I?

@chris98: Most of it is here: https://github.com/fluxbb/core


fluxbb.de | develoPHP

"As code is more often read than written it's really important to write clean code."

Offline

#4 2014-07-24 23:43:31

Sxderp
Member
Registered: 2012-11-02
Posts: 92

Re: TODO: isAdmMod()

Franz wrote:

Yeah, I really need to go through those TODOs, shouldn't I?

@chris98: Most of it is here: https://github.com/fluxbb/core

You should. I'd like to pitch in where I can (small stuff like this). Although I have to admit that I've never worked with something like Laraval (or even object oriented programming) so I've spent a lot of my time just trying to figure out how it all 'works.' Thankfully Fluxbb is simple enough to be a entry point into this kind of programming.

I do find it slightly hard to navigate. Mostly because there's generally so little in each file it's hard to see what is actually going on.

Last edited by Sxderp (2014-07-24 23:48:23)

Offline

#5 2014-07-25 06:46:18

GWR
Member
Registered: 2010-08-06
Posts: 112

Re: TODO: isAdmMod()

As soon there is a logical difference between moderators and admins one should not merge multiple functions into one.

There might be "isModerator()" and "isAdministrator()". YOU may know that "isModerator" returns the idea of "isAtLeastModerator" but do others know that too? It is not recognizeable for freshman regarding your code.

But. If there is a function "canModerate()" this indeed can include moderators _and_ administrators. Because it implies only the check if the user can do a specific thing/action.


IsModerator - only returns if the user is in the group "moderator"
IsAdministrator - only returns if the user is in the group "administrator"
CanModerate - returns true if the user is in a group able to moderate things
CanAdministrate - returns true if the user is in a group able to administrate things (useable if you have things like "SuperAdmin")

Of course you can hide such information with Uber-functions like "canDo("action")" which hides the whole ACL-functionality from exterior users - so checks an ACL list versus the groups a user has attached).
Also do think about situations with "SuperAdmin" and "Admin" with the SuperAdmin restricting the "Admin"-group to only be able to ban users but not edit topics ... odd situation but there might be situations needing such odd changes. This is not possible with an "if your level is higher than X, you can do Y". Hmm but this is only a matter if real ACL come into the forums logic.



Conclusion (of my personal opinion): IsModerator including admins is not fine granulated as it could be. To check if a moderator is not a admin you would need "IsModerator() and not IsAdmin()" - the "condition chain" could get longer with increased amount of user groups having some specific privileges. Of course this could stay simplified with IsMod() and IsAdmin() but it is a core rewrite, so basics could still change.


bye
Ron

Last edited by GWR (2014-07-25 06:46:46)

Offline

#6 2014-07-25 09:01:51

chris98
Member
From: England, United Kingdom
Registered: 2013-05-31
Posts: 906
Website

Re: TODO: isAdmMod()

Hmm...

I have mixed feelings about that. I like some areas of it, but some things I don't really like, I also have a couple of questions (I've never used laravel before)

1.) Are those files practically the final version, or is there a lot more to do to them? I think a lot are fairly empty, and there are a lot of files that could be combined?

2.) Due to the fact that there are so many files, how will you access topics, profiles.etc? Would it be with the same viewtopic.php?id=123 profile.php?id=456 or would it be something like IPB - index.php?module=post&act=viewtopic&id=123?

3.) Sessions or Cookies? I thought cookies until I came across a couple of session.php files then I wasn't sure.

4.) I came across this, and saw the "edit-profile.html" and "profile.html", are there different files for viewing profiles and editing them? That's one part of other software I hate, I think it over-complicates them.

I also have a couple of other questions which I couldn't see in the code:

Will you be able to assign users to moderate forums, but not have access to the admin CP?

Are there are images or demos of the styles yet or will you be keeping the default ones?

I suppose these next few are more feature requests, but can admins login again to use the admin CP?

I know that daris has a colorize group mod, but do you intend to implement something similar (optional for the forum admin) in 2.0? I think it would be a good idea.

Login using ques. On my forum, I've noticed that several bots are constantly brute-forcing the login forms, for fifteen to twenty minutes at a time.

I feel I've been fairly negative and harsh above, so I'll tell you some of the things I like about it as well:

* The new profile features, such as being able to send messages to users

* More info on the profile page (I.E. latest post)

* Functions calls for direct user functions

* The code appears to be a  lot more structured, cleaner and organised

* The new admin dashboard (especially this)

Great job so far! big_smile

If there is a function "canModerate()" this indeed can include moderators _and_ administrators.

Just what I as going to suggest. But how about a function for both admins and moderators, and a one for each? Not that moderators would need to do anything admins wouldn't, but you could be sure you're addressing the correct user group them.

Last edited by chris98 (2014-07-25 09:59:37)

Offline

#7 2014-07-29 16:02:29

Franz
Lead developer
From: Germany
Registered: 2008-05-13
Posts: 6,047
Website

Re: TODO: isAdmMod()

Thanks for the feedback! I'm currently working on some of the TODOs. Quite a few are irrelevant now, not only because I plan to introduce a real ACL system.

I really need to come up with some simple, well-defined tasks that can be taken over by community members, don't I?


fluxbb.de | develoPHP

"As code is more often read than written it's really important to write clean code."

Offline

#8 2014-07-30 12:13:04

chris98
Member
From: England, United Kingdom
Registered: 2013-05-31
Posts: 906
Website

Re: TODO: isAdmMod()

That will make it a lot more secure, and a lot more flexible than it is currently. I'm looking forward to seeing that implemented!

I really need to come up with some simple, well-defined tasks that can be taken over by community members, don't I?

That sounds like a great idea smile

Offline

Board footer

Powered by FluxBB 1.5.8