WikiEducationFoundation / WikiEduDashboard

Wiki Education Foundation's Wikipedia course dashboard system
https://dashboard.wikiedu.org
MIT License
391 stars 630 forks source link

Edits by new users that introduce external links will fail because of CAPTCHA #237

Open ragesoss opened 9 years ago

ragesoss commented 9 years ago

At least, in the case that I tried: http://dashboard.wikiedu.org/courses/University_of_Toronto/HMB436H_Medical_and_Veterinary_Mycology_%28Fall_2015%29/overview

I updated the link in course description, but that change never propagated to Wikipedia: https://en.wikipedia.org/w/index.php?title=Wikipedia:Wiki_Ed/University_of_Toronto/HMB436H_Medical_and_Veterinary_Mycology_%28Fall_2015%29&action=history

ragesoss commented 9 years ago

Here was another case from today: https://en.wikipedia.org/w/index.php?title=Wikipedia:Wiki_Ed/University_of_San_Francisco/Golden_Gate_Park_%28Fall_2015%29&action=history

The instructor created and submitted the course, and then I added a cohort. It was not until Helaine took another action that it actually created the course on-wiki.

We need to beef up the WikiEdits flow; as a start, maybe we can just find a way to check for success and capture any failure messages?

ntdb commented 9 years ago

I couldn't find any good reason for this yesterday but I did go ahead and add some error handling. Hopefully something helpful comes out of it. :) Please review my work @ragesoss, you're the Sentry mastermind.

ragesoss commented 9 years ago

This looks good. One thing that I wish we were capturing in Sentry logs is the current user. At some point, we should see if we can configure it to globally capture that. Otherwise, we can add it to the explicit capture_exception calls where appropriate.

ragesoss commented 9 years ago

One thing that has been happening is that the course announcements will successfully post a template to the userpage, but they will not successfully create the course page.

I looked through the production.log and found the rails logs for a user who did this today, and there aren't any errors showing up there (nor are any errors getting passed to Sentry).

Maybe this is because of the this line in the .update_course method? return unless current_user.wiki_id? && course.submitted && course.slug?

If the 'submit' update hasn't hit the database yet, then that would make the 'submit' here usually hit that guard clause. We still need that guard clause to prevent posting to wiki before submission, though. Assuming this might be the problem, I'll work on a patch.

ragesoss commented 9 years ago

Okay, I don't think that's the problem, precisely, because in the case of the user from today, they only made one of the three expected edits after pressing submit: https://en.wikipedia.org/wiki/Special:Contributions/KimD_UWT

In other words, WikiEdits.announce_course succeeded with the first edit it tries to make, but not the second. And there are no ways I can see on our side of the code that would let one of those fail, but not the other. Maybe it's just that sometimes, too many edits in immediate succession don't work? In any case, it looks like the new error handling is not catching the edits that don't happen. Maybe there's something more specific that the Wikipedia API spits out to let us know whether an edit succeeded or not.

ragesoss commented 9 years ago

Well, I thought that the new error handling code in WikiEdits would pick up cases where a user's edits fail, but it happened again today: https://en.wikipedia.org/wiki/Special:Contributions/S.C._Kaplan

Upon course submission, only the first of the three edits (to the user's own userpage) happened. The other two did not go through, although there was no error picked up on Sentry. We should inspect the server logs to see if we can learn anything more.

ragesoss commented 9 years ago

Let's see if we can learn more by capturing the API response for successful edits, too: 9e35b8bd53f089863c7e668ee3a24e1130db117a

ragesoss commented 9 years ago

Okay, my new theory is that the second and third edits trigger a CAPTCHA when performed by a new user, because they include external links. I should have thought of this much earlier. It's not a failed edit, but it's also not a successful one (unless the CAPTCHA is returned, which we don't even attempt).

I think the best way around this will be to remove the live links from the output of the second and third messages. This would mean essentially turning off live links for the course output, but that's okay.

ragesoss commented 9 years ago

Sentry events confirm that the CAPTCHA theory is correct. I removed the urls from the second edit message, leaving only the course page creation. I think we can live with that not working for new users who can't yet post external links without a captcha. The course page will get created at the time that it gets approved, anyway, and after that, most edits will not introduce any external links... and it doesn't take that long for users to have enough edits to not trigger the CAPTCHA anyway.

ragesoss commented 8 years ago

We've worked around the issue as much as we can. Passing on the CAPTCHA is definitely not what we want to do, which puts the remaining cases of it into the realm of things we'd need to address at the wiki end, such as by whitelisting the dashboard edits.