WebHookEventType and the solved and assign plugins

bug
1

In this (fantastic) change that added webhooks to the solved plugin, some functionality regarding the webhook event types was moved to core.

However, this has some unwanted side effects.

  1. on instances that do not have the solved or assign plugins installed, the event types are showing but the translation is missing, and (of course) the events are not available although they are being shown.

afbeelding
afbeelding946Ɨ439 12.4 KB

  1. the solved and assign plugins are currently broken on new stable and beta installs because they are missing the seeds

My initial thought was to ask you to backport the seeds to stable to fix #2, but since there is an additional bug (#1) maybe it is better to leave the seeding code in the actual plugin and just reserve the IDā€™s in the code in core.

Long story short, I think it would be better to move the

WebHookEventType.seed do |b|
  b.id = WebHookEventType::SOLVED
  b.name = "solved"
end

code back into the solved and assign plugins.

8 Likes
2

Thanks for bringing this up. I will look into what we can do so that if the plugins arenā€™t installed they donā€™t show up in core.

7 Likes
4

Great, thank you!
Can you please try to address the fact that it is broken on stable as well?

5 Likes
5

A fix has been created for this:

https://github.com/discourse/discourse/commit/919e405c4832c5901a616953e1df947717c37743

Regarding #2 though:

Iā€™m not seeing any issues running stable (2.4.1) locally with the latest versions of the the discourse-assign and discourse-solved plugins.

Could you provide some info on how I can replicate the issues you are running into?

4 Likes
6

A new install on stable with the latest version of the solved plugin will not have the IDā€™s in the web_hook_event_type table, preventing the webhook type from showing.

This is happening because the code that seeds the IDā€™s has been removed from the solved plugin and it is not yet present in the stable / beta release.

Previously installed sites that had an older version of the solved plugin will already have the seed data and they will not have the problem.

So replication step is easy: create a brand new install on stable.

Clean new install on 2.4.1 with solved plugin: no webhook available.

LKqhCxFVDH
LKqhCxFVDH1182Ɨ892 509 KB

Letā€™s perform the seeding manually, then take a look at the webhook screen again and it will appear:

pX9fvAw4i7
pX9fvAw4i71084Ɨ888 1.01 MB

7

Oh okay, so ā€œbrokenā€ means it just doesnā€™t show up.

This is by design. It is a 2.5+ feature.

4 Likes
8

Ok, you are completely right, my apologies for being unclear. I was mixing up ā€˜brokenā€™ and ā€˜not availableā€™, and since this is a new feature there is indeed nothing broken.

It does feel a bit strange how seeding code for a plugin is moved into core though. I can understand that you want to reserve the IDā€™s but I think it would be more clean to leave the seeding in the plugin. But thatā€™s just my own personal opinion :wink:

3 Likes