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

Already on GitHub? Sign in to your account

onGetGroupsByUser; request for new event trigger #1488

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
4 participants

Request to create a new event trigger, to be able to write a plugin that can insert groups from a custom source.

@elinw elinw commented on an outdated diff Aug 24, 2012

libraries/joomla/access/access.php
@@ -333,7 +340,27 @@ public static function getGroupsByUser($userId, $recursive = true)
$db->setQuery($query);
$result = $db->loadColumn();
- // Clean up any NULL or duplicate values, just in case
+ // Execute the query and load the rules from the result.
+ $db->setQuery($query);
+ $result = $db->loadColumn();
+
+ // Add trigger to allow for external group assignment
+ // nesting prevention:
+ if (!self::$loaded) {
@elinw

elinw Aug 24, 2012

Contributor

We put opening curly brackets on new lines, can you please fix this here and elsewhere?

Contributor

elinw commented Aug 24, 2012

http://developer.joomla.org/pulls/pulls/1488.html
You've got a failing unit test and some code style errors (don't worry the first 129 are not yours).

@elinw elinw commented on an outdated diff Aug 24, 2012

libraries/joomla/access/access.php
@@ -333,7 +340,27 @@ public static function getGroupsByUser($userId, $recursive = true)
$db->setQuery($query);
$result = $db->loadColumn();
- // Clean up any NULL or duplicate values, just in case
+ // Execute the query and load the rules from the result.
+ $db->setQuery($query);
+ $result = $db->loadColumn();
+
+ // Add trigger to allow for external group assignment
+ // nesting prevention:
+ if (!self::$loaded) {
+ self::$loaded=true;
+ JPluginHelper::importPlugin('user');
+ }
+ $dispatcher = JDispatcher::getInstance();
@elinw

elinw Aug 24, 2012

Contributor

JEventDispatcher

Contributor

elinw commented Sep 9, 2012

Would you close ths one please since it s the same as the other?

Contributor

LouisLandry commented Oct 9, 2012

This is the same as what other one?

Contributor

elinw commented Oct 25, 2012

#1494 but that's closed instead.

Isn't this repeated?
https://github.com/mdobrinic/joomla-platform/blob/0ed0fd2442d66a2e5e110114fdba29c754605bbd/libraries/joomla/access/access.php#L340

I really like the idea of being able to manipulate what groups a user is part of more easily and based on context. For example that would have let us possibly handle guest differently or let us dynamically create a single-user group for each user. But I'm not sure that I would do it this way because I'd want to know that I'm not going to end up with group id conflicts without a plan for handling them (ignore or replace? throw an exception?). I'm also not sure if that should be happening in JAcccess or JUser.
Maybe there should be a method for adding a group that could manage some of these things.

Contributor

eddieajau commented Nov 12, 2012

Apart from a few code-style issues, I don't like the way the trigger is done. I'm going to close this PR and suggest you raise the issue on the mailing list to get a consensus on how to solve your problem.

@eddieajau eddieajau closed this Nov 12, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment