feat: DiscordRoleFilter #68
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| <?php | ||
|
|
||
| /** | ||
| * This file is part of SeAT Discord Connector. | ||
| * | ||
| * Copyright (C) 2021 Troyburn <[email protected]> | ||
| * | ||
| * SeAT Discord Connector is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU General Public License as published by | ||
| * the Free Software Foundation, either version 3 of the License, or | ||
| * any later version. | ||
| * | ||
| * SeAT Discord Connector is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License | ||
| * along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
|
|
||
| namespace Warlof\Seat\Connector\Drivers\Discord\Driver; | ||
|
|
||
| class DiscordRoleFilter | ||
| { | ||
| /** | ||
| * | ||
| * @var array of string | ||
| */ | ||
| private $filter_specs; | ||
|
|
||
| /** | ||
| * | ||
| * @var bool | ||
| */ | ||
| private $everyrole; | ||
|
|
||
| /** | ||
| * | ||
| * @var string EVERYROLE_ENTRY | ||
| */ | ||
| public const EVERYROLE_ENTRY = '@@everyrole'; | ||
|
|
||
| /** | ||
| * Arbitrary string "RoleName Example2 RemoveMe" exist to self document a valid setting in database. | ||
| * | ||
| * @var string DEFAULT_CAN_REMOVE_ROLES | ||
| */ | ||
| public const DEFAULT_CAN_REMOVE_ROLES = '@@everyrole:RoleName Example2 RemoveMe'; | ||
|
|
||
| /** | ||
| * Arbitrary string "RoleName Example2 RemoveMe" exist to self document a valid setting in database. | ||
| * | ||
| * @var string DEFAULT_CAN_ADD_ROLES | ||
| */ | ||
| public const DEFAULT_CAN_ADD_ROLES = '@@everyrole:RoleName Example2 RemoveMe'; | ||
|
|
||
| /** | ||
| * Arbitrary string "RoleName Example2 RemoveMe" exist to self document a valid setting in database. | ||
| * | ||
| * @var string DEFAULT_VISIBLE_ROLES | ||
| */ | ||
| public const DEFAULT_VISIBLE_ROLES = '@@everyrole:RoleName Example2 RemoveMe'; | ||
|
|
||
| /** | ||
| * DiscordRoleFilter constructor. | ||
| * | ||
| * @param string $spec | ||
| * @param string $explodeOn | ||
| */ | ||
| public function __construct(string $spec = '', string $explodeOn = ':') | ||
| { | ||
| $this->filter_specs = explode($explodeOn, $spec); | ||
|
There was a problem hiding this comment. maybe collection could be useful here ? There was a problem hiding this comment. does the configuration system allow storage and default conversion to collections ? There was a problem hiding this comment. collection will allow you to manipulate array of things, including way to exclude data from final result. |
||
| while (($key = array_search('', $this->filter_specs)) !== FALSE) { | ||
| unset($this->filter_specs[$key]); /* remove empty string items */ | ||
| } | ||
| $this->everyrole = in_array(self::EVERYROLE_ENTRY, $this->filter_specs); | ||
| } | ||
|
|
||
| /** | ||
| * Check a single role against the filter spec. | ||
| * | ||
| * @param string|null $s | ||
| * @return mixed | ||
| */ | ||
| public function checkOne(string $s = null, $defaultValue = false) | ||
| { | ||
| if (empty($s)) { | ||
| return $defaultValue; | ||
| } | ||
|
|
||
| if ($this->everyrole === true) { | ||
| return true; | ||
| } | ||
|
|
||
| /* theoretically it should be possible to modify this matching | ||
| * element to return true, false or $defaultValue (with an implied | ||
| * non-bool type) to implement ordered precedence rules and | ||
| * question mark prefixed inversion of a spec written like: | ||
| * "Role.*:!RoleABC:RoleXYZ" | ||
| */ | ||
| return in_array($s, $this->filter_specs) ? true : $defaultValue; | ||
| } | ||
|
|
||
| /** | ||
| * Check all the roles in an array of role against the filter spec. | ||
| * | ||
| * @param array $ary | ||
| * @param mixed $defaultValue Expected to be bool or null. | ||
| * @return mixed or type of $defaultValue | ||
| */ | ||
| public function checkAll(array $ary = null, $defaultValue = false) | ||
| { | ||
| if (is_null($ary) || empty($ary)) { | ||
| return $defaultValue; | ||
| } | ||
|
|
||
| if ($this->everyrole === true) { // superflous? | ||
| return true; | ||
| } | ||
|
|
||
| foreach ($ary as $item) { | ||
| $res = $this->checkOne($item, null); | ||
| if (! is_null($res)) { | ||
| return $res; | ||
| } | ||
| } | ||
|
|
||
| return $defaultValue; | ||
| } | ||
|
|
||
| /** | ||
| * Check the arguments given as role against the filter spec. | ||
| * Returns true|false accordingly. | ||
| * @param $args | ||
| * @return bool | ||
| */ | ||
| public function check(...$args) : bool | ||
| { | ||
| // $args = func_get_args(); | ||
| foreach ($args as $val) { | ||
| if(is_array($val)) | ||
| $res = $this->checkAll($val, null); | ||
| else | ||
| $res = $this->checkOne($val, null); | ||
| if (! is_null($res)) { | ||
| return $res; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| } | ||
|
|
||
| ?> | ||
|
There was a problem hiding this comment. php closing tag is non longer expected There was a problem hiding this comment. habbit, PHP is my 15th language |
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,11 @@ | ||
| <?php | ||
|
|
||
| return [ | ||
| 'client_id' => 'Client ID', | ||
| 'client_secret' => 'Client Secret', | ||
| 'bot_token' => 'Bot Token', | ||
| 'use_email_scope' => 'Use email as Unique ID? Email scope will be requested from users.', | ||
| 'visible_roles' => 'Manage visibleRoles for Discord Role filter', | ||
| 'can_add_roles' => 'Manage canAddRoles for Discord Role filter', | ||
| 'can_remove_roles' => 'Manage canRemoveRoles for Discord Role filter', | ||
| ]; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead providing hardcoded roles list as value, wouldn't it be better to put this as field tooltip into connector settings ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC the design goal is not to break peoples systems during an upgrade.
An empty value does not make the overall module (seat-discord-connector) work as most users might expect, the empty value will disable functionality.
Not sure tooltip information alone helps. I'm sure the tooltip information can be improved, I didn't spend much time on it (since I know what it does lol).
This is about a fresh install or an upgrade to this version and then opening the discord settings and ensuring the new fields are populated with defaults that continue to maintain old behavior where possible. In the case of this new feature that should be possible, hence this value is in code as well as documentation (README).
If you only put information in documentation and tooltips many people won't read/notice during upgrade and the first time they save the settings after upgrade (which could be months) their connector will stop working like it did before pressing Discord Settings -> Save. Because it will save an empty string value by default, which will change module behaviour.
How many support issues might this cause, due to not reading documentation around the uprade ?
The
@@everyroleis an illegal role name on Discord, so was picked for its special meaning.That role names in discord can contain spaces.
That the use of
*it also a valid role name in discord.Discord has a
@everyonewith special meaning, so it wasn't a far stretch to use@@everyroleI'm trying to convey to someone not reading the seat-discord-connector documentation and making a guess how to configure seat-discord-connector with 2 or more roles and when roles contain spaces all at the same time. i.e. space is not the role name delimiter, space is allowed and considered part of the role name. So ideally the SeAT administrators best guess on what to do will be correct first time.