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

Last login TimeStamp #1517

Merged
merged 4 commits into from
Feb 6, 2023
Merged

Last login TimeStamp #1517

merged 4 commits into from
Feb 6, 2023

Conversation

FatihKoz
Copy link
Contributor

Add a carbon timestamp for last login instead of using updated_at. Update login controller to save last login details along with user ip (both tied to the same setting due to GDPR)

Closes #1516

Possible Error/Exception Warning

If the admin is already logged off during update, when he/she tries to login after update there will be an exception about missing database field.

Workaround For Admins

  • Disable IP recording before update (re-enable it after completing migration and cleaning cache) or,
  • Visit /update manually after the error (technically authentication is completed at that moment and process fails during model update only)

FatihKoz and others added 2 commits January 28, 2023 21:20
Add a proper timestamp for last login instead of using update timestamp.
@nabeelio
Copy link
Owner

Where's the missing DB field message coming from?

@FatihKoz
Copy link
Contributor Author

Where's the missing DB field message coming from?

LoginController, line 117 of my pr... As expected it is trying to save the login timestamp before running the migration (maybe a try and catch ?)

@nabeelio nabeelio merged commit 0197ed8 into nabeelio:dev Feb 6, 2023
@nabeelio
Copy link
Owner

nabeelio commented Feb 6, 2023

Let's see how it goes

@FatihKoz FatihKoz deleted the LastLogin branch February 6, 2023 18:04
@FatihKoz
Copy link
Contributor Author

FatihKoz commented Feb 6, 2023

Probably admins will get 500 server error if they are already logged out (deleted cache before update, session lifetime etc).
And if they visit /update or admin dashboard after that error all will be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Last Login Date/Time
2 participants