Adds the possibility to configure the record's primary_key. #338

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
@julianalucena

No description provided.

@binarylogic

This comment has been minimized.

Show comment Hide comment
@binarylogic

binarylogic Dec 7, 2012

Owner

Why would this deviate from the klass primary_key?

Owner

binarylogic commented Dec 7, 2012

Why would this deviate from the klass primary_key?

@julianalucena

This comment has been minimized.

Show comment Hide comment
@julianalucena

julianalucena Dec 7, 2012

I have two different Rails app and when the user logs in one app, he will be logged in both apps. To do so, both apps should interpret the cookie equally, but the same user has different primary_keys (databse auto increment column) for each app. So I needed to configure another column to be saved on the cookie, that is the user's id on the main app.

I have two different Rails app and when the user logs in one app, he will be logged in both apps. To do so, both apps should interpret the cookie equally, but the same user has different primary_keys (databse auto increment column) for each app. So I needed to configure another column to be saved on the cookie, that is the user's id on the main app.

@tiegz

This comment has been minimized.

Show comment Hide comment
@tiegz

tiegz Dec 27, 2012

Collaborator

I might be misunderstanding, but you might be able to use the id feature for this. For example:

us1 = UserSession.new(params)
us1.id = :session1
us1.save

us2 = UserSession.new(params)
us2.id = :session2
us2.save

and then later:

us1 = UserSession.find(:session1)
us2 = UserSession.find(:session2)
Collaborator

tiegz commented Dec 27, 2012

I might be misunderstanding, but you might be able to use the id feature for this. For example:

us1 = UserSession.new(params)
us1.id = :session1
us1.save

us2 = UserSession.new(params)
us2.id = :session2
us2.save

and then later:

us1 = UserSession.find(:session1)
us2 = UserSession.find(:session2)
@julianalucena

This comment has been minimized.

Show comment Hide comment
@julianalucena

julianalucena Jan 2, 2013

@tiegz, thanks for the help.

Yes, maybe this could solve. But this seems conceptually wrong to me because this change should be a configuration, do you know? And solve it locally when creating and finding sessions keeps things in the wrong place.

@tiegz, thanks for the help.

Yes, maybe this could solve. But this seems conceptually wrong to me because this change should be a configuration, do you know? And solve it locally when creating and finding sessions keeps things in the wrong place.

@tiegz

This comment has been minimized.

Show comment Hide comment
@tiegz

tiegz Jan 2, 2013

Collaborator

Ah, I see.

For your instance, tho, won't each app be using separate classes each with different logic? So if they're using different column names for their primary key, can't you set the primary_key specifically in each app's class?

Collaborator

tiegz commented Jan 2, 2013

Ah, I see.

For your instance, tho, won't each app be using separate classes each with different logic? So if they're using different column names for their primary key, can't you set the primary_key specifically in each app's class?

@julianalucena

This comment has been minimized.

Show comment Hide comment
@julianalucena

julianalucena Jan 4, 2013

Actually, @tiegz, I want to maintain the primary_key as it is in the apps, I just need the session to consider another column as key to identify the user.

Got it?

Actually, @tiegz, I want to maintain the primary_key as it is in the apps, I just need the session to consider another column as key to identify the user.

Got it?

@tiegz

This comment has been minimized.

Show comment Hide comment
@tiegz

tiegz Jan 4, 2013

Collaborator

Do you mean the session cookie key that identifiers the user, like user_credentials_id?

Collaborator

tiegz commented Jan 4, 2013

Do you mean the session cookie key that identifiers the user, like user_credentials_id?

@julianalucena

This comment has been minimized.

Show comment Hide comment
@julianalucena

julianalucena Jan 7, 2013

Yes, because the session cookey key uses the user's id.

Yes, because the session cookey key uses the user's id.

@tiegz

This comment has been minimized.

Show comment Hide comment
@tiegz

tiegz Jan 7, 2013

Collaborator

Ok, sorry, I understand now!

I'd say +1 on this. I think this could also be handy for obfuscating the user's id, for apps that use some sort of guid to identify users.

Collaborator

tiegz commented Jan 7, 2013

Ok, sorry, I understand now!

I'd say +1 on this. I think this could also be handy for obfuscating the user's id, for apps that use some sort of guid to identify users.

@fltiago

This comment has been minimized.

Show comment Hide comment
@fltiago

fltiago Sep 28, 2013

+1

fltiago commented Sep 28, 2013

+1

@binarylogic

This comment has been minimized.

Show comment Hide comment
@binarylogic

binarylogic Feb 28, 2014

Owner

Cool, I'll have to merge in the new changes, unless someone else wants to tackle it. But I'll pull once we can merge without conflicts.

Owner

binarylogic commented Feb 28, 2014

Cool, I'll have to merge in the new changes, unless someone else wants to tackle it. But I'll pull once we can merge without conflicts.

@jaredbeck

This comment has been minimized.

Show comment Hide comment
@jaredbeck

jaredbeck Jul 18, 2016

Collaborator

This seems like a reasonable feature, but it adds some complexity to authlogic. We'd have to remember to use authlogic_record_primary_key everywhere instead of primary_key, right?

Since this feature was suggested nearly four years ago, only one other person has expressed any interest in it (AFAIK). So, I'm hesitant to add complexity for only two people. Maybe this could be a plugin gem instead?

Given no activity on this PR in over two years, I'm going to close it.

Collaborator

jaredbeck commented Jul 18, 2016

This seems like a reasonable feature, but it adds some complexity to authlogic. We'd have to remember to use authlogic_record_primary_key everywhere instead of primary_key, right?

Since this feature was suggested nearly four years ago, only one other person has expressed any interest in it (AFAIK). So, I'm hesitant to add complexity for only two people. Maybe this could be a plugin gem instead?

Given no activity on this PR in over two years, I'm going to close it.

@jaredbeck jaredbeck closed this Jul 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment