search

mit-teaching-systems-lab / discourse-edx-lti

Discourse plugin for using Discourse as a discussion forum in EdX courses
http://tsl.mit.edu
MIT License
22 stars 6 forks source link

Update user lookup to match code in 1.9.0 related to multiple email a… #17

Closed kevinrobinson closed 7 years ago

kevinrobinson commented 7 years ago

This leads to the LTI user lookup always failing (which is what should happen for the initial LTI login, but not for subsequent logins). The error in Discourse looks like this to users:

screen shot 2017-09-06 at 11 41 15 am

And stack trace pinpointing that this is coming from the call to User.find_or_initialize_bylooks like:

var/www/discourse/vendor/bundle/ruby/2.3.0/gems/logster-1.2.7/lib/logster/logger.rb:93:in `add_with_opts'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/logster-1.2.7/lib/logster/logger.rb:50:in `add'
/usr/local/lib/ruby/2.3.0/logger.rb:507:in `error'
/var/www/discourse/plugins/discourse-edx-lti/lti_strategy.rb:61:in `log'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:475:in `fail!'
/var/www/discourse/plugins/discourse-edx-lti/lti_strategy.rb:55:in `rescue in callback_phase'
/var/www/discourse/plugins/discourse-edx-lti/lti_strategy.rb:31:in `callback_phase'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:230:in `callback_call'

Tracking this to the Discourse User model, it seems the changes to support multiple emails mean that past code using Rails find methods to look for email won't work anymore. From looking at git blame, this appears related to changes in https://github.com/discourse/discourse/pull/4977 to enable multiple email addresses.

This PR works around by using the monkey-patched find methods on User related to email and username lookups, and then manually checks that both find the same record.

kevinrobinson commented 7 years ago

selfie