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

Elgg 3: Hide river activity for disabled plugin #12514

Open
rohit1290 opened this issue Apr 7, 2019 · 13 comments
Open

Elgg 3: Hide river activity for disabled plugin #12514

rohit1290 opened this issue Apr 7, 2019 · 13 comments

Comments

@rohit1290
Copy link
Contributor

Let's say I have the blog plugin enabled and I created a blog post and the river activity looks like this.
image


Now I disabled the blog plugin and the river activity is shown like this.
image

@rohit1290
Copy link
Contributor Author

I also found that if the user is banned the river activity of banned user is show as below:

image

@rohit1290
Copy link
Contributor Author

I have tested the following code to show/hide river activity on plugin activation/deactivation:

// Call on plugin deactivation
elgg_register_event_handler('deactivate', 'plugin', function($event, $object_type, $object){

	$dbprefix = elgg_get_config('dbprefix');
	$plugin = $object['plugin_id'];
		
	$query = <<<QUERY
	UPDATE {$dbprefix}river AS rv
	LEFT JOIN {$dbprefix}entities AS se ON se.guid = rv.subject_guid
	LEFT JOIN {$dbprefix}entities AS oe ON oe.guid = rv.object_guid
	LEFT JOIN {$dbprefix}entities AS te ON te.guid = rv.target_guid
	SET rv.enabled = 'no'
	WHERE (
			(se.enabled = 'yes' OR se.guid IS NULL) AND
			(oe.enabled = 'yes' OR oe.guid IS NULL) AND
			(te.enabled = 'yes' OR te.guid IS NULL)
		)
		AND (
			(se.subtype = '$plugin') OR
			(oe.subtype = '$plugin') OR
			(te.subtype = '$plugin')
		);
QUERY;
		
	elgg()->db->updateData($query);

});



// Call on plugin activation
elgg_register_event_handler('activate', 'plugin', function($event, $object_type, $object){

	$dbprefix = elgg_get_config('dbprefix');
	$plugin = $object['plugin_id'];
		
	$query = <<<QUERY
	UPDATE {$dbprefix}river AS rv
	LEFT JOIN {$dbprefix}entities AS se ON se.guid = rv.subject_guid
	LEFT JOIN {$dbprefix}entities AS oe ON oe.guid = rv.object_guid
	LEFT JOIN {$dbprefix}entities AS te ON te.guid = rv.target_guid
	SET rv.enabled = 'yes'
	WHERE (
			(se.enabled = 'yes' OR se.guid IS NULL) AND
			(oe.enabled = 'yes' OR oe.guid IS NULL) AND
			(te.enabled = 'yes' OR te.guid IS NULL)
		)
		AND (
			(se.subtype = '$plugin') OR
			(oe.subtype = '$plugin') OR
			(te.subtype = '$plugin')
		);
QUERY;
		
	elgg()->db->updateData($query);

});

What's you view? should I make a PR for this?

@jeabakker
Copy link
Member

If i read this correct this would enable/disable all river activity for a plugin_id. How will that help? activtity mostly isn't created for the plugin id by for content types the plugin provides.
Yes for some core plugins this would work for example blog, but that is more coincident that wisdom

@rohit1290
Copy link
Contributor Author

aaah.. i see what you mean, the subtype of a plugin might not be necessarily be same as the plugin id.
True.. I didn't thought of that.. All i saw was on calling 'deactivate', 'plugin' or 'activate', 'plugin', only the plugin_id is available.

I will close one of the above PR. However, you can look into the user ban/unban PR it basically calling the 'disable:after' and 'enable:after' trigger. That can be merged.

@jdalsem
Copy link
Member

jdalsem commented Apr 11, 2019

i disagree that we should disable river activity for banned users... we still show user and its content, so why not show the activity... however we still need to fix the missing river activity text (as shown in your screenshot). Did you investigate why there is not text showing?

@rohit1290
Copy link
Contributor Author

i disagree that we should disable river activity for banned users... we still show user and its content, so why not show the activity..

Well, I beg to differ here.. If an admin is banning a user then there must be a reason behind it (like the user is spamming the website). In such scenario the admin would also want to remove all the activity of the user from the river and don't show them to other user. (Atleast I would not want my user to see those spamming content). In such cases removing the river item manually would be a hectic task.

however we still need to fix the missing river activity text (as shown in your screenshot). Did you investigate why there is not text showing?

This is what is happening, when I am deactivating a plugin the enabled column of elgg_river table is not getting updated to "no". As the river item is still active (enabled == yes), those item are getting populated in elgg_list_river. Now when I am deactivating the plugin the river view i.e. river/object/plugin/create and the language string river:object:plugin:create is no longer available. When generating the elgg_list_river, due to the non availability of the view, it is falling back to a default view, however, as the the language string is also not available, hence it showing blank.

Solution:
I tried setting enabled as no using elgg_register_event_handler for 'deactivate', 'plugin' but what I found was that the event handler only has plugin_id and I was not able to find any solution that map the plugin_id with the subtype of the plugin. (Like jeabakker said it is not necessary that subtype of a plugin will be same as the plugin id.)

  1. I think adding a plugin_id column in the river table will be helpful, In that way at the time of triggering 'deactivate', 'plugin' we can set enabled as no for the plugin_id.
  2. We can defined that fallback in such a way that it does not show if the river/* view is missing.
  3. If there is any way we can map the plugin_id to subtype then the above code shared by me can be tweaked and can be used.

One more point that should also be consider is the comment. If I disable blog plugin, the comments for those blog entity should also not be shown on river.

@jdalsem
Copy link
Member

jdalsem commented Apr 11, 2019

however we still need to fix the missing river activity text (as shown in your screenshot). Did you investigate why there is not text showing?

You have shown a screenshot where you state the content is missing after you banned a user... just banning a user should not cause river content to disappear... can you reproduce missing translations in the river when you just ban a user... or was it also related to the deactivation of a plugin

@rohit1290
Copy link
Contributor Author

No no no.. there is some confusion.. When I deactivate a plugin, the translations are missing for those plugin's river activity like blogs (Referring to the first screenshot).

When I ban a user, the activity is not removed. (There is no text because i was testing something, sorry if that created a confusion. There is no translations problem for banning user. I was just saying that we need to hide the river activity for ban user.)

@jdalsem
Copy link
Member

jdalsem commented Apr 12, 2019

ok, so a banned user does not cause issues with the activity. That is good. As said, the banned user logic will not remove any activity/content. That logic will not be changed in core. Deleting/disable a user will remove/hide all content/activity. Banning will just limit the use of the site for the banned user. Of course you can change that logic yourself with a plugin. Core will not change that logic as we think a banned users actity/content/profile should still be visible.

@rohit1290
Copy link
Contributor Author

No problem, I will add the ban/unban logic in my plugin.

Now that leaves us with the issue of hiding the river activity for disabled plugin.

@jeabakker
Copy link
Member

Still an interesting problem with lots of (potential) racing conditions.

Example
Blog and Groups enabled.

  • disable Groups, this could disable all river activity in groups
  • disable Blog, this could disable all river activity for blogs
  • enable Groups, this could re-enable all river activity in groups (including blogs which isn't present)

Don't know a good way to have support for this all. Yes we could add a function which offers an easy api to enable/disable river activity. But than the problems start.

Like your feedback

@jdalsem
Copy link
Member

jdalsem commented Apr 17, 2019

this can only be done by plugins if the river knows which activity is created/provided by that plugin. We could add a extra plugin id column which controls if river content is enabled/disabled on activate/deactivate of a plugin.... alternatively plugins should/could do something on the (de)activate events but that requires way more coding by the plugin dev and also makes it still very hard to pinpoint specific river activity

@rohit1290
Copy link
Contributor Author

I have solved the issue and I have created a plugin for the same.
https://github.com/rohit1290/activity_tool

@jdalsem

this can only be done by plugins if the river knows which activity is created/provided by that plugin.

plugins should/could do something on the (de)activate events but that requires way more coding by the plugin dev

Well I won't completely rely on the developer and expect him/her to play by the books and do the extra work to maintain the code for activation and deactivation of river activity. On the other hand, the site administrator will do everything in his power to keep his community clean and that is the whole idea of my plugin. The site admin needs to manually set the type, subtype of plugin to a plugin id which is stored in a newly created table within the elgg database.

it still very hard to pinpoint specific river activity

Yes, true. It is complex but I have written a query that can solve the problem by correctly identifying which river item to activate and which one to leave out using the mapping maintained by the site administrator. (Here the sql query: https://github.com/rohit1290/activity_tool/blob/master/start.php#L108,L137)
This query has also taken into consideration of the scenarios that @jeabakker has highlighted in his comment above. Additionally, the comments for the entities are also taken into consideration on plugin activation and deactivation.

Do let me know your views.

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

Successfully merging a pull request may close this issue.

3 participants