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.

After discussing it in the latest BTR Bi-weekly meetup we decided to not merge this in Lilac. It was difficult to draw the line. But when looking at issues like the old payment processing flow, it’s easier to justify making a large change because it impacts a large number of users.

For lilac, anyone that needs to wants to use the badgr integration will need to cherry-pick the changes.

@mrtmm Thanks for the fix! I think it’s useful to know that you’re using this feature. It would be great if you could test it and share the results with the BTR group leading up to a major release. That way we can avoid this situation in the future :smiley: .

2 Likes

Alright, thanks folks!

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’d be very interested to know if this is still the plan. If anyone has more information on that, please do share :slight_smile:

2 Likes

I haven’t really been following badge-related developments. I see that Proversity created an XBlock for it, and there’s a writeup by IBL, but I have no idea how supported it is. I did not see any DEPR ticket about Badgr functionality specifically.

IBM offers badges on their edX courses, but I don’t believe they open sourced that. @nedbat may have more insight here.

Badging has certainly been a poster child example of the kind of thing we’d like to have in plugin form, since it represents functionality that talks about content but shouldn’t need to be deeply coupled to things like modulestore. It’s even been used in one or two arch workshops as an example of a simple use case that we’d want to enable via event messaging. But I don’t know of any open, active work on it today.

1 Like