Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the MP Moderators forum group to check for mod authority. #4136

Merged
merged 1 commit into from Aug 15, 2019

Conversation

@Pentarctagon
Copy link
Member

commented Jun 28, 2019

People who have MP mod authority for testing or other reasons can continue to use the existing user_is_moderator column check, but for people who are supposed to moderate the server, now the forum group will be used.

This will make it easier to add/remove people to/from the group, and also ensure that the list of actual moderators matches up with the list available to users.

New config attributes added for this are:

  • db_group_table - should be, based on the phpbb documentation I found, phpbb_user_group
  • mp_mod_group - should be set to Multiplayer Moderators group ID

Re-PR of #3964 so I can put this in a separate branch of my fork.

I am also waiting for this before continuing with some additional changes I would like to implement/enforce such as this.

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

You know, it'd be nice if this SQL statement were portable... but I suppose that's not possible when using prepared statements...

@Pentarctagon

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

What do you mean by portable?

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

Portable to different databases.

@soliton-

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

Are you saying this is not standard conform SQL?

Given that we don't even have a DB abstraction layer that seems like a secondary concern anyway and has little to do with this PR.

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

It's not - standard-conforming SQL uses "double quotes" to wrap identifiers, rather than `backquotes` (MySQL) or [square brackets] (MSSQL).

But yeah, this code is just directly using the MySQL API, so while it would be nice to have something that works for any database system, that has no relation to this pull request.

@soliton-

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

Perhaps I'm blind but I don't see where double quotes are used. I only see backquotes used.

@Pentarctagon

This comment has been minimized.

Copy link
Member Author

commented Jul 27, 2019

Why are backquotes used in Wesnoth's SQL anyway? Aren't they only useful if you're using a reserved word as a table/column name, or something like that?

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Jul 27, 2019

Perhaps I'm blind but I don't see where double quotes are used. I only see backquotes used.

Yeah, that was my point. The standard-conforming way is to use double quotes for identifiers and single quotes for strings.

Why are backquotes used in Wesnoth's SQL anyway? Aren't they only useful if you're using a reserved word as a table/column name, or something like that?

Or a name beginning with a digit, or containing special characters. I'd say it's good practice to use it when splicing in a table or column name from a configuration file.

@soliton-

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

Ah, now I understand the wording.

Well, this would be trivial to fix but MySQL is not standard-conforming by default there and not sure what'd happen to the forum if we switch to a conforming SQL mode (ANSI).

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

I'm pretty sure MySQL does have some option to make it standard-conforming with respect to quote usage (though I don't know if that's per-session or in the configuration file).

@soliton-

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

Yes, it's the SQL mode I mentioned. It can be set globally or per session.

@Pentarctagon Pentarctagon force-pushed the Pentarctagon:mp-mod-group branch 2 times, most recently from baaba76 to e710484 Aug 13, 2019

@Pentarctagon Pentarctagon requested review from soliton- and removed request for shikadiqueen Aug 15, 2019

Show resolved Hide resolved doc/man/wesnothd.6 Outdated
Show resolved Hide resolved src/server/forum_user_handler.cpp Outdated
Show resolved Hide resolved src/server/forum_user_handler.hpp Outdated
People who have MP mod authority for testing or other reasons can con…
…tinue to use the existing user_is_moderator column check, but for people who are supposed to moderate the server, now the forum group will be used.

This will make it easier to add/remove people to/from the group, and also ensure that the list of actual moderators matches up with the list available to users.

New config attributes added for this are:
* db_group_table - should be, based on the phpbb documentation I found, phpbb_user_group
* mp_mod_group - should be set to Multiplayer Moderators group ID

@Pentarctagon Pentarctagon force-pushed the Pentarctagon:mp-mod-group branch from e710484 to 02ebcff Aug 15, 2019

@Pentarctagon

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2019

Updated as per review.

@soliton- soliton- merged commit 9470eea into wesnoth:master Aug 15, 2019

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.