Core plugins wherein a fork of a merged plugin was being used

A bit late to the game here, but in regard to the migration of popular plugins to core discourse, what does one do if one was using a modified fork of a plugin that was merged?

For reference Set up Discord notifications with the discourse-chat-integration plugin - #71 by skatefriday I had added the ability to pipe flagged posts into Discord as it is important for moderators to immediately get notification that a user flagged a post and Discord is the lowest latency communication medium for this.

The existing discourse chat integration plugin in did not, and still does not, have this feature.

A while back, another member of my team, updated our discourse server and upon noticing that the plugin was now natively bundled with Discourse, because the build failed on update, he simply removed the clone of my fork.

And now I’ve noticed that my feature is gone.

So what’s the best practice for when a self hosted setup was using a modified plugin to restore the features of the modified plugin?

4 לייקים

The best thing to do is write a plugin that overrides the plugin.

Another thing to do is in the line before you clone your fork, you “rm - rf” the merged plugin.

So do just that in your new plugin rather than forking the main one. There spoilt be a hook to do that.

3 לייקים

Write a completely new plugin? That seems excessive.

This is probably the answer.

I don’t really understand this at all. I wanted the functionality of the existing plugin with the addition of being able to tell it to pipe to flagged posts into the chat client, in my case Discord. Are you suggesting again that I write, from scratch, a completely new plugin that duplicates much of the functionality of the existing plugin and adds the new feature that I wanted? Again, seems excessive.

To a certain degree you can replace/extend logic in existing classes. This might be an option to extend the bundled plugin. You would write a new plugin which just add the modified logic. Using module prepend.

enabled_site_setting :myoverridingplugin_enabled

module ::MyOverridingPlugin
	PLUGIN_NAME = "my-overriding-plugin"

	class Engine < ::Rails::Engine
		engine_name MyOverridingPlugin::PLUGIN_NAME
		isolate_namespace MyOverridingPlugin
	end

	module SomeClassOverrides
		def overriding_method(foo, bar)
			if foo == "something"
				# do something custom
			else
				# this will call the original logic
				super(f00, bar)
			end
		end
	end
end

after_initialize do
	SomeClass.prepend(MyOverridingPlugin::SomeClassOverrides)
end

I used this constructor to limit some controllers under certain conditions.

לייק 1

I feel you, and I have found this as one of the most impactful technical complications of the “bundling plugins with core” as well. We had a few forked plugins and it was very complicated to get them to work without removing the bundled plugin.

I don’t think Jay is suggesting that. A plugin can also override very specific parts of another plugin.

The best approach would be to convince the team that your code is worth being merged into the official plugin. That will work if your modification is generic or flexible enough. I see you already have made a fork and your changes/additions are pretty clean. Maybe the hardcoded “Flagged” string could be in a translation file and if you make :flagged default to false then you don’t have to modify the original event handler with an extra parameter but apart from that, it looks worthy. If I were you I would bring it up to date, open a PR and discuss this in the plugin topic.

If that route fails, you could simply build a plugin that overrides those three functions that you changed, and adds the on(:reviewable_created) handler.

לייק 1

The point of plugins is so that you don’t have to fork discourse. It’s not excessive, it’s why plugins exist.

No. Your plugin will simply add the new feature to send flagged posts to discord, and call the existing code to do it. It’s probably ten lines of code and won’t require you to merge upstream changes into your plugin.