Yicong-Huang commented on code in PR #7055:
URL: https://github.com/apache/texera/pull/7055#discussion_r3708335853


##########
common/auth/src/main/scala/org/apache/texera/auth/JwtAuth.scala:
##########
@@ -55,10 +55,9 @@ object JwtAuth {
     val claims = new JwtClaims
     claims.setSubject(user.getName)
     claims.setClaim("userId", user.getUid)
-    claims.setClaim("googleId", user.getGoogleId)
     claims.setClaim("email", user.getEmail)
     claims.setClaim("role", user.getRole)
-    claims.setClaim("googleAvatar", user.getGoogleAvatar)
+    claims.setClaim("avatar", user.getAvatar)

Review Comment:
   Do it in this PR or not, you will need to handle the migration either way. 
You need to make sure after any PR is merged, the main branch is not broken 
intentionally. 
   
   if you want to do it separately, then it's better to migrate with three 
steps:
   - in this PR only add a new fields, new code, without changing the old code 
path, old fields. both new and old code path will be live: i.e., it will set 
both `avatar` and `googleAvatar`. (add only, Coexist state)
   - in a second PR to let frontend read from the new code path (i.e., 
`avatar`). (switch only. if anything goes wrong, you can revert easily). 
   - in a third PR to remove the old code path. (clean up/deletion only.)
   
   So after reach PR, the main is still valid.
   
   However, you could also combine those three steps in one PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to