Conversation
awilfox
approved these changes
Feb 14, 2026
Member
awilfox
left a comment
There was a problem hiding this comment.
This looks great and should help with debugging AP-583 as well. r+ after tiny question.
| # list all keys except duo keys | ||
| def list_auth_extra_keys(auth_extra) | ||
| keys = auth_extra.keys.reject { |k| k.start_with?('duo') }.sort | ||
| Rails.logger.info("CalNet auth_extra keys: #{keys.join(', ')}") |
Member
There was a problem hiding this comment.
Since this value is already being logged above (line 76), does it need to be logged here as well?
awilfox
requested changes
Feb 14, 2026
Member
awilfox
left a comment
There was a problem hiding this comment.
Ah, sorry, I didn't see the test failures. Looks like Rubocop has some suggestions, and we need to fix up the mock user objects in the other tests to have the required attributes. Still, I approve of the overall work - just need to fix those issues before merging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During Rails upgrade code review testing, we discovered users were missing email addresses. Investigation showed that a CalNet schema attribute had been renamed, though we were not notified when the change occurred. This ticket aims to address unexpected attribute changes from CalNet and improve our handling of schema updates.