Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #8033 (new defect)

Opened 1 year ago

Last modified 1 year ago

[PATCH] open_id_authentication plugin cleanup/fixes

Reported by: monki Assigned to: david
Priority: normal Milestone: 2.x
Component: Plugins Version: edge
Severity: normal Keywords: plugin
Cc:

Description

I decided to have a go at using the new open_id_authentication plugin and ran into a few issues. So I thought I would try to fix them up so that it will be easier for others moving forward. I'll briefly outline the changes here.

Added attr_reader for the code attribute of Result objects. The example case statement for specific error handling did not work as intended, so this provides access to use the code directly in case statements. For further discussion see: http://groups.google.com/group/rubyonrails-talk/browse_thread/thread/f88ba63d5e5c3555

Fixed a couple typos and tried to generalize (use User instead of @account.users). I also switched to the plural for sessions (as resources generally refer to plural objects). It might be that there is a good reason to name sessions in the singular, but plural for me and seems to better match my understanding of REST best practices.

Added message to Timeout::Error in test cases so that all tests now pass.

Attachments

open_id_cleanup_round_2.diff (1.0 kB) - added by josh on 05/19/07 04:35:40.
open_id_cleanup.diff (4.3 kB) - added by monki on 05/26/07 22:55:13.
Fix/cleanup. (I needed to upgrade rubygems, but it should use the new non-deprecated includes)

Change History

05/04/07 14:13:13 changed by monki

  • owner changed from core to david.

Before it gets buried further, thought I should assign it to david as it would appear to be his project.

05/06/07 21:08:30 changed by monki

  • summary changed from [patch] open_id_authentication plugin cleanup/fixes to [PATCH] open_id_authentication plugin cleanup/fixes.

Capitalize patch, in case it filters on it cap-sensitive.

05/07/07 04:26:34 changed by monki

  • keywords set to plugin.

(follow-up: ↓ 5 ) 05/07/07 04:33:33 changed by david

It's SessionController by design. In my applications, I'm using it as a singular controller as you only have one session per user. Please strip out those changes and the rest looks good.

(in reply to: ↑ 4 ) 05/14/07 20:45:00 changed by monki

Replying to david:

It's SessionController by design. In my applications, I'm using it as a singular controller as you only have one session per user. Please strip out those changes and the rest looks good.

That makes plenty of sense. As per other rails conventions, plurality makes sense but takes a bit to grok. Also I wanted to bump this in case you hadn't noticed that I fixed the patch. Thanks for the handy patch, works nicely.

05/19/07 04:35:40 changed by josh

  • attachment open_id_cleanup_round_2.diff added.

05/24/07 14:20:45 changed by monki

I don't think he intended for the README changes to be removed altogether, just for the plurality to be fixed(the difference between the patches). I suppose this way he has the choice when it comes time to apply.

05/26/07 22:55:13 changed by monki

  • attachment open_id_cleanup.diff added.

Fix/cleanup. (I needed to upgrade rubygems, but it should use the new non-deprecated includes)