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

[legacy-framework] Add setPublicDataForUser() — useful for updating the role of another user #2473

Merged
merged 4 commits into from
Jun 14, 2021

Conversation

lksnmnn
Copy link

@lksnmnn lksnmnn commented Jun 10, 2021

Closes: blitz-js/legacy-framework#144
Documentation PR: blitz-js/blitzjs.com#500

What are the changes and their implications?

This change adds a new API to update a user's publicData across all of the user's active sessions. Among other things this feature solves the common usecase, where a user's role is changed and the change should be reflected immediatly, i.e., when the user is denoted from "admin" to "user".

Bug Checklist

  • Integration test added (see test docs if needed)

Feature Checklist

@lksnmnn lksnmnn marked this pull request as ready for review June 11, 2021 07:48
Copy link
Member

@flybayer flybayer left a comment

Choose a reason for hiding this comment

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

Thank you!!

@lksnmnn
Copy link
Author

lksnmnn commented Jun 11, 2021

There is one thing I might want to test with this, that is actually being logged in with both users and check what happens to the user, when his publiData is being updated.

In my version of this function, which I posted in the linked issue, it worked, but when I changed browsers to quickly after updating the role, the user got weird invalid session and CSRF errors. Which is probably a bug in the session logic, but using this new API could trigger this.

@flybayer
Copy link
Member

@lksnmnn ok good to know. You are going to test?

@lksnmnn
Copy link
Author

lksnmnn commented Jun 11, 2021

Yes, I will try to reproduce this in the auth example, so I can debug and hopefully find the issue. From outside it “felt” similar to the issue I was having with using gSSP (https://github.com/blitz-js/blitz/issues/2448).

But I wont be able to start this before Sunday I guess.

@lksnmnn
Copy link
Author

lksnmnn commented Jun 13, 2021

I could not reproduce this bug in the auth example. So I guess the issue was my code doing something weird. 🚀

@flybayer
Copy link
Member

Ok thank you, we'll roll with this and address the bug later if it shows up

@flybayer flybayer changed the title feat: set public session data API Add setPublicDataForUser() — useful for updating the role of another user Jun 14, 2021
@flybayer flybayer merged commit f478f83 into blitz-js:canary Jun 14, 2021
@blitzjs-bot
Copy link
Contributor

Added @lksnmnn contributions for code and test

@itsdillon itsdillon changed the title Add setPublicDataForUser() — useful for updating the role of another user [legacy-framework] Add setPublicDataForUser() — useful for updating the role of another user Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a setPublicData(userId, publicData) API to update active user sessions (i.e. update a user's role)
3 participants