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
WIP: feature(river): Adds hook-based permissions for river item delete action #8942
Conversation
7950b9e
to
605be9a
Compare
* @return \ElggUser|false | ||
* @access private | ||
*/ | ||
public function resolveUser($guid = 0) { |
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.
Better suggestions for naming this?
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.
Can we make this more generic?
I think it would be useful to have something like resolve($guid = 0, $type = null, $subtype = null)
. We could then use elgg_entity_exists()
beforehand, or elgg_instanceof()
afterwards.
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.
Strawman: move the functionality to get(), add $subtype (= null) and $ignore_condition (= false). For this use we'd set $type to "user" and $ignore_condition to true.
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.
SGTM. I think it will be much cleaner.
605be9a
to
f9fb4a5
Compare
…ents This also deprecates elgg_delete_river(). Fixes Elgg#8936
f9fb4a5
to
e799a02
Compare
@@ -1035,21 +1035,8 @@ public function canEdit($user_guid = 0) { | |||
* @see elgg_set_ignore_access() | |||
*/ | |||
public function canDelete($user_guid = 0) { | |||
$user_guid = (int) $user_guid; | |||
$user = _elgg_services()->entityTable->resolveUser($user_guid); |
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.
Waiting for #8944 to offer a standard API for this.
Replaced by #9964 |
Fixes #8936