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

[Core] Add getLastLogin helper to user class #5899

Merged
merged 4 commits into from
Jan 8, 2020

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Jan 7, 2020

This moves the query to get the last user login from the
dashboard code to the User class, where it more accurately
belongs.

(This was extracted from #5896 where it was necessary because
multiple modules use the last login time to determine if something
is new.)

This moves the query to get the last user login from the
dashboard code to the User class, where it more accurately
belongs.

(This was extracted from aces#5896 where it was necessary because
multiple modules use the last login time to determine if something
is new.)
@johnsaigle johnsaigle added the Cleanup PR or issue introducing/requiring at least one clean-up operation label Jan 7, 2020
@driusan driusan added the State: Needs work PR awaiting additional work by the author to proceed label Jan 7, 2020
@driusan driusan added Blocking PR should be prioritized because it is blocking the progress of another task and removed State: Needs work PR awaiting additional work by the author to proceed labels Jan 8, 2020
AND userID=:UserID AND Success='Y'",
array('UserID' => $userID)
);
$last_login = $user->getLastLogin($DB)->format('Y-m-d H:i:s');
Copy link
Contributor

@johnsaigle johnsaigle Jan 8, 2020

Choose a reason for hiding this comment

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

Will this cause an error to the effect of "Can't use function 'format' on null" when the function returns null?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, good catch. I added a hack to handle null and ensure that it's in the same format as the DB was returning for this PR only if it's set, but I'll fix it properly in the modularized widgets PR because work on this one will end up getting lost in the end anyways

@@ -532,5 +532,32 @@ class User extends UserPermissions
(string) $this->userInfo['Password_hash']
);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good pending Travis. I have a suspicion that one of the static analysis tools won't like the two new lines here but maybe it doesn't matter.

@driusan driusan merged commit 109c46a into aces:master Jan 8, 2020
@driusan
Copy link
Collaborator Author

driusan commented Jan 8, 2020

One down, 4 to go..

@ridz1208 ridz1208 added this to the 23.0.0 milestone Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocking PR should be prioritized because it is blocking the progress of another task Cleanup PR or issue introducing/requiring at least one clean-up operation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants