Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ticket/12380] Sort Remember Me keys by last login time in UCP #2281

Merged
merged 2 commits into from Apr 11, 2014

Conversation

iMattPro
Copy link
Member

@@ -672,7 +672,8 @@ function main($id, $mode)

$sql = 'SELECT key_id, last_ip, last_login
FROM ' . SESSIONS_KEYS_TABLE . '
WHERE user_id = ' . (int) $user->data['user_id'];
WHERE user_id = ' . (int) $user->data['user_id'] . '
ORDER BY last_login';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please explicitly include sorting direction

@iMattPro
Copy link
Member Author

Updated

@@ -672,7 +672,8 @@ function main($id, $mode)

$sql = 'SELECT key_id, last_ip, last_login
FROM ' . SESSIONS_KEYS_TABLE . '
WHERE user_id = ' . (int) $user->data['user_id'];
WHERE user_id = ' . (int) $user->data['user_id'] . '
ORDER BY last_login ASC';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you pick the sorting direction arbitrarily? I am wondering whether DESC makes more sense, "last used first".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I figured oldest at the top of the list made most sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also make the column names clickable and allow sorting by all 3 values in both directions.

Otherwise I would prefer DESC aswell, then you can start checking the "mark" box starting at a certain date rather then stop :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discussed that on IRC. I think making columns sortable is overkill. loginkeys is not an important list. there usually won't be many loginkeys listed. and its not like a user NEEDS to have them sorted either upwards or downwards for any reason.

I figured oldest at top made sense, because those would be the first ones a user would delete… so they can work their way down the list until they reach their most recent login keys.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah just read it. Fine by me.

@nickvergessen nickvergessen merged commit eb6b877 into phpbb:develop-ascraeus Apr 11, 2014
@iMattPro iMattPro deleted the ticket/12380 branch April 11, 2014 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants