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

remove user lastActiveTimestamp property #365

Merged
merged 6 commits into from
Feb 20, 2025
Merged

Conversation

vincanger
Copy link
Collaborator

@vincanger vincanger commented Feb 14, 2025

Description

removes user.lastActiveTimestamp property entirely since we're not really using it in a meaningful way.

Renames updateCurrentUser to a more generic function name in the authorization docs.

Contributor Checklist

Make sure to do the following steps if they are applicable to your PR:

@vincanger vincanger changed the title return user id and timestamp only from updateUserTimestamp remove user lastActiveTimestamp property Feb 14, 2025
Copy link
Collaborator

@sodic sodic left a comment

Choose a reason for hiding this comment

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

Seems simple enough, approved!

@vincanger You probably did this, but search for lastActiveTimestamp across the entire repo to ensure there are no leftovers.

@@ -79,7 +79,7 @@ Authorization on the server-side is the core of your access control logic, and d
You can authorize access to server-side operations by adding a check for a logged-in user on the `context.user` object which is passed to all operations in Wasp:

```tsx title="src/server/actions.ts"
export const updateCurrentUser: UpdateCurrentUser<...> = async (args, context) => {
export const someServerAction: SomeServerAction<...> = async (args, context) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How come we changed this?

@sodic
Copy link
Collaborator

sodic commented Feb 19, 2025

@vincanger I double checked.

We seem to be missing a doc change and a migration for this. Here's how I did it:

$ rg lastActiveTimestamp -l
opensaas-sh/app_diff/migrations/20231213174854_init/migration.sql.diff
opensaas-sh/blog/src/content/docs/general/user-overview.md

Copy link
Collaborator

@sodic sodic left a comment

Choose a reason for hiding this comment

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

Changing PR status until we change the docs and add a migration to avoid accidental merging.

Copy link
Collaborator

@sodic sodic left a comment

Choose a reason for hiding this comment

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

Looks good!

@sodic sodic merged commit 40f6b72 into main Feb 20, 2025
3 checks passed
@sodic sodic deleted the update-updateUser-info branch February 20, 2025 14:42
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.

2 participants