Smarter room #2484

woogerboy21 · 2017-03-15T21:38:26Z

Added the ability for registered user - VIP/DONATOR/PRIVILEGED room.

Since the users privilege level does not have any type of tier concept to it, we can only compare the actual literal string value of the privilege level and check to see if a user is a part of the given privilege level to allow or deny them into a given room.

As explained when the privilege level concept was being introduced, we have admin level which is a bit comparison to allow the concept of admin > mod > reg > non-reg, but privilege level doesn't work like that. Its a sub category. So something like VIP !> DONATOR and vice versa.

With the logic in this PR a DONATOR for example can not enter a VIP room, nor can a VIP enter a DONATOR room, but we do compare admin level value(s) so regardless of admin/mod privilege levels they can enter the rooms. How ever an unregistered user is not allowed to enter the room if defined with the "vip" , "donator" or "privileged" database table privlevel"column value.

Clear as mud?

This PR adds a "privlevel" to the room so we can do easy comparisons between a rooms definition of "privilege" to a users definition of "privilege" and allow/deny access based on the combinations of definitions rather than a hard written code block for every single privlevel added to the db table. This way new privilege levels can be added into the database columns for the enum value(s) in the users/rooms table columns and their should be no additional coding required to restrict room access based on the new enum definitions.

Added privileged room

… definitions

…ble block

Added the ability for registered user - VIP/DONATOR/PRIVILEGED room. Since the users privilege level does not have any type of tier concept to it, we can only compare the actual literal string value of the privilege level and check to see if a user is a part of the given privilege level to allow or deny them into a given room. As explained when the privilege level concept was being introduced, we have admin level which is a bit comparison to allow the concept of admin > mod > reg > non-reg, but privilege level doesn't work like that. Its a sub category. So something like VIP !> DONATOR and vice versa. With the logic in this PR a DONATOR for example can not enter a VIP room, nor can a VIP enter a DONATOR room, but we do compare admin level value(s) so regardless of admin/mod privilege levels they can enter the rooms. How ever an unregistered user is not allowed to enter the room if defined with the "vip" , "donator" or "privileged" database table privlevel"column value. Clear as mud? This PR adds a "privlevel" to the room so we can do easy comparisons between a rooms definition of "privilege" to a users definition of "privilege" and allow/deny access based on the combinations of definitions rather than a hard written code block for every single privlevel added to the db table. This way new privilege levels can be added into the database columns for the enum value(s) in the users/rooms table columns and their should be no additional coding required to restrict room access based on the new enum definitions.

ZeldaZach

Good start, but you never use the helper function you made

ZeldaZach · 2017-03-16T00:36:04Z

common/server_room.cpp

+{
+    qDebug() << "USERPRIV: " << QString::fromStdString(userInfo.privlevel());
+
+    ServerInfo_Room roomInfo;


You create a roomInfo object, but never use it in the function, nor call getRoomPermissionDisplay()

You wouldn't call get..Display here, but this line doesn't need to be here, you're right

Whoops left what looks like a debug line in, I'll 🔪 it. As for the function, its called in the tab_server.cpp file to pull the room permissions to display in the table on the tab. 🍝

ZeldaZach · 2017-03-16T00:36:38Z

servatrice/migrations/servatrice_0022_to_0023.sql

@@ -0,0 +1,6 @@
+-- Servatrice db migration from version 22 to version 23
+
+alter table cockatrice_rooms modify column permissionlevel enum('NONE','REGISTERED','MODERATOR','ADMINISTRATOR');


" over ' since the other line uses "? 🥇

For sql you use ' for strings; " is used to quote table and column names

Yeah they should be ' , not sure how one line came out one way and the other another. I will fix the line below it with " going "->'.

Daenyth · 2017-03-16T01:02:25Z

common/server_room.cpp

+{
+    qDebug() << "USERPRIV: " << QString::fromStdString(userInfo.privlevel());
+
+    ServerInfo_Room roomInfo;


You wouldn't call get..Display here, but this line doesn't need to be here, you're right

Daenyth · 2017-03-16T01:02:48Z

servatrice/migrations/servatrice_0022_to_0023.sql

@@ -0,0 +1,6 @@
+-- Servatrice db migration from version 22 to version 23
+
+alter table cockatrice_rooms modify column permissionlevel enum('NONE','REGISTERED','MODERATOR','ADMINISTRATOR');


For sql you use ' for strings; " is used to quote table and column names

woogerboy21 · 2017-03-16T02:28:38Z

Should be all fixed up now. I'll see about throwing test box up for you guys to play with tomorrow. Heading to 🛏

ZeldaZach · 2017-03-16T02:32:37Z

common/server_room.cpp

+bool Server_Room::userMayJoin(const ServerInfo_User & userInfo)
+{
+
+    ServerInfo_Room roomInfo;


Is there a reason this is here if we never use it? Or do you use it implicitly and I didn't notice?

I swear I just answered this.... its used on line 624 of the protocol handler.
https://github.com/Cockatrice/Cockatrice/pull/2484/files#diff-d3b638a03e1222b79c2b7b5b8935096fR624

But that's a different function and this one isn't in scope there. Also it's a different type, this one is Server_Room and the other is ServerInfo_Room. I think you're mixed up here

Oh you mean the declaration of the roomInfo on line 42? If so yeah that might be a whoops / carry over from when I copied / pasted the code from the block that had it declared in before moving it into its own function. I thought you guys were referring to the actual function itself.

Yes, line 42 :)

woogerboy21 · 2017-03-17T02:11:30Z

Did you manage to get a test box up and play with it? I had planned on doing so for you to try but life jumped in the way today. On 3/16/2017 10:10:41 PM, Zach H <notifications@github.com> wrote: @ZeldaZach approved this pull request. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [#2484 (review)], or mute the thread [https://github.com/notifications/unsubscribe-auth/ABTFhxPkIRg63_wbzi9FplFPJxTBPqFFks5rmeuggaJpZM4MekDJ].

ZeldaZach · 2017-03-17T02:12:16Z

I approved the code based on syntax and how it appears to function, i didn't get a chance to test it on a box. Will want to test before merging ofc

woogerboy21 · 2017-03-21T19:29:19Z

@ZeldaZach
I finally had time to stand up a box with this PR rolled in. People can test things out by using the host name of testing.woogerworks.com and port 4747 . You should be able to use the username/password you have on the main woogerworks server. I don't recall when the users table was pulled in on the testing server so if you recently changed your password the testing servers password might be your previous password.

ZeldaZach · 2017-03-22T20:32:05Z

Tested and this seems to function as it should.

Daenyth · 2017-03-23T01:39:24Z

cockatrice/src/tab_server.cpp

+    * on the server tab needed to inform users of required permissions to enter a room. If the room has a privilege level
+    * the server tab will display the privilege level in the "permissions" column in the row however if the room contains
+    * a permissions level for the room the permissions level defined for the room will be displayed.
+    */


great comment

woogerboy21 added 13 commits March 12, 2017 16:56

Added VIP only room

5c2eca1

Added DONATOR room.

1608dd6

Added NOTE to code block about possible future functionality expansion

78f78d9

Corrected && to ||

966f9f5

Added privileged room

Extended Room to include privilege level.

b04c048

Updated room join logic

65a4aa5

Updated server tab permissions column display based on perm+privlevel…

487e8cd

… definitions

Fixed new client -> old server blank permissions column on server tab

da6b879

Added comment and simplified the server tab permissions display varia…

3316fa1

…ble block

Cleaned up moderator and above permissions check for entering a room.

b37e300

Corrected display catch all for mis-configured ini room definitions.

5f245a9

Updated formating per request

f9b85f7

woogerboy21 requested review from Daenyth and ZeldaZach March 15, 2017 21:39

woogerboy21 added App - Cockatrice Tickets relating to the cockatrice application App - Protocol / API Tickets that will require protocol level support in addition to client and server changes App - Servatrice Tickets relating to the servatrice application labels Mar 15, 2017

ZeldaZach requested changes Mar 16, 2017

View reviewed changes

Daenyth approved these changes Mar 16, 2017

View reviewed changes

Removed qDebug + fixed "->'

a5bc9ce

woogerboy21 mentioned this pull request Mar 16, 2017

Added privileged level only room #2464

Closed

ZeldaZach reviewed Mar 16, 2017

View reviewed changes

Hong Kong Phooey!

57742ba

ZeldaZach approved these changes Mar 17, 2017

View reviewed changes

Merge branch 'master' into smarter_room

90985c0

Merge branch 'master' into smarter_room

e6655bb

Daenyth approved these changes Mar 23, 2017

View reviewed changes

ZeldaZach merged commit 2f23a9c into Cockatrice:master Mar 23, 2017

woogerboy21 deleted the smarter_room branch July 24, 2017 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Smarter room #2484

Smarter room #2484

woogerboy21 commented Mar 15, 2017

ZeldaZach left a comment

ZeldaZach Mar 16, 2017

Daenyth Mar 16, 2017

woogerboy21 Mar 16, 2017

ZeldaZach Mar 16, 2017

Daenyth Mar 16, 2017

woogerboy21 Mar 16, 2017 •

edited

Loading

Daenyth Mar 16, 2017

Daenyth Mar 16, 2017

woogerboy21 commented Mar 16, 2017

ZeldaZach Mar 16, 2017

woogerboy21 Mar 16, 2017

Daenyth Mar 16, 2017

woogerboy21 Mar 16, 2017 •

edited

Loading

Daenyth Mar 16, 2017

woogerboy21 commented Mar 17, 2017 via email

ZeldaZach commented Mar 17, 2017

woogerboy21 commented Mar 21, 2017

ZeldaZach commented Mar 22, 2017

Daenyth Mar 23, 2017

		@@ -0,0 +1,6 @@
		-- Servatrice db migration from version 22 to version 23

		alter table cockatrice_rooms modify column permissionlevel enum('NONE','REGISTERED','MODERATOR','ADMINISTRATOR');

Smarter room #2484

Smarter room #2484

Conversation

woogerboy21 commented Mar 15, 2017

ZeldaZach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woogerboy21 Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woogerboy21 commented Mar 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woogerboy21 Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woogerboy21 commented Mar 17, 2017 via email

ZeldaZach commented Mar 17, 2017

woogerboy21 commented Mar 21, 2017

ZeldaZach commented Mar 22, 2017

Choose a reason for hiding this comment

woogerboy21 Mar 16, 2017 •

edited

Loading

woogerboy21 Mar 16, 2017 •

edited

Loading