Backporting Badgr integration updates to Lilac

Hi Folks!

The Badgr integration has been broken for quite a while. The long-lived tokens are no longer supported by Badgr and the support for v1 API is coming to an end soon as well. I’ve been working on the updates to fix the integration and these changes have now landed in master:

I’d love to see these changes in Lilac as well, here’s the PR:

It is not a small patch so I do understand the concern. However, I wouldn’t call it a new feature as well since it is actually fixing something that has been broken for a long time. I believe adding this change would only affect folks that have badging enabled and I am not sure how many there are. I didn’t get any responses about it in slack.

I’d love to get your thoughts on adding this to Lilac, I’m here for any questions and concerns :slight_smile:

Thanks!

4 Likes

Yes it really has… thank you so much for fixing it! I’m sure it will be used more now that it’s working again :smiley:

I’m pretty sure OpenCraft doesn’t have any clients that use Badgr anymore (we did many years back), so I’d vote for including this fix in Lilac.

4 Likes

@mrtmm, thanks! I’m convinced. :slight_smile:

Let’s just wait a bit for the rest of the group’s opinions.

I was about to answer “fine with me let’s merge” when I realized that this patch introduces a new database migration. From my perspective, the schema change pushes the patch across the (admittedly blurry) border that separates bug fixes from new features. If someone needs this feature badly then they can always cherry pick the patch.
I will not fight the rest of the community over this, though.

1 Like

If we can get others to test it then I’m personally happy to have it on Lilac.

Is that something you can have a look at @jhony_avella?

That’s an interesting feature which I think has been broken since the Ironwood release.
However as far as I know there were some plans about its deprecation/removal to moving it out in a third-party module/plugin which does the same job. I’m not pretty sure where I’ve read this but maybe @dave or @Diana_Huang could confirm.