rill icon indicating copy to clipboard operation
rill copied to clipboard
verified publisher icon rilldata

feat: login / signup in rill developer

Open AdityaHegde opened this issue 1 year ago • 3 comments

  • [x] Add GetCurrentUser endpoint to get info similar to cloud.
  • [x] Add /auth/logout endpoint for logout and to clear the token stored locally. Also logs out of cloud ui.
  • [x] Add LocalAvatarButton that calls the above endpoints and is shown in dashboard (will be moved to use global header in a future PR)

This PR doesnt add pylon to rill developer, this will be in a separate PR

AdityaHegde avatar Jun 19 '24 14:06 AdityaHegde

Intent discussed with @jkhwu was for the logged out state to also appear on non-preview screens in Rill dev (see the last variant of the component). Just wanted to clarify that with you in case it was not obvious: https://www.notion.so/rilldata/Logged-out-state-in-Rill-Developer-4add186ff5e649c1b526ed4fdc788608?pvs=4

Also, can we put this behind a feature flag? Given it doesn't provide much value to our users without the rest of the deploy flow, it's best to hide

ericokuma avatar Jun 24 '24 23:06 ericokuma

@ericokuma @ericpgreen2 Updated to add the avatar button in metrics view editor. Also updated logout to logout from cloud ui as well.

AdityaHegde avatar Jun 27 '24 09:06 AdityaHegde

Looks great! Here are some items I found:

  1. When logging out, there's a loading state but on the "Preview" button. @jkhwu, what should be the appropriate behavior here? On the avatar? https://www.loom.com/share/ae865da6814d495b8eb04555416a30c9?sid=ae06b0d1-5b62-452a-b42c-1c48f69ccaa6
  • There's also no loading state when viewing a dashboard.

  1. No logged-in avatar in this view. @jkhwu, can you add the desired location here? Screenshot 2024-06-27 at 8 28 38 AM

  2. No logged-in avatar in other views: rill.yaml, README.md, .gitignore, sources, models Screenshot 2024-06-27 at 8 30 23 AM

ericokuma avatar Jun 27 '24 15:06 ericokuma

@ericokuma Fix for 1 is in a separate PR (it was unrelated to this): #5166 Shall we take the follow ups in a new PR?

AdityaHegde avatar Jul 01 '24 14:07 AdityaHegde

I'm having trouble testing this in dev mode.

First attempt

Per your instructions, I did:

  1. make cli
  2. ./rill devtool start cloud
  3. ./rill start <project>

When trying to log-in via the browser, I hit this 500 error: image

Second attempt

I also tried:

  1. make cli
  2. ./rill devtool start cloud
  3. In web-local/package.json, I edited vite dev to vite dev --port 3001
  4. ./rill devtool start local

Then when logging in via the browser, I hit this 404 error: image

In this^ case, I wonder if the trailing slash after auth is causing the issue?

ericpgreen2 avatar Jul 02 '24 01:07 ericpgreen2

Double-checking, did you see Okuma's comment? I'm not yet seeing the feature flag in the code.

Also, can we put this behind a feature flag? Given it doesn't provide much value to our users without the rest of the deploy flow, it's best to hide

ericpgreen2 avatar Jul 02 '24 01:07 ericpgreen2

Since we are not merging for the current release (~3rd july), doesnt make sense to add the feature flag.

AdityaHegde avatar Jul 02 '24 05:07 AdityaHegde

I'm having trouble testing this in dev mode.

Fixed by https://github.com/rilldata/rill/pull/5233

ericpgreen2 avatar Jul 09 '24 19:07 ericpgreen2

Code looks good. My main UX observation is that the avatar is not shown on all pages. I see that you'll switch to a global header in a follow-on PR. That works – just noting that IMO the avatar needs to be shown on all pages before this is released.

ericpgreen2 avatar Jul 09 '24 19:07 ericpgreen2

Tagging @ericokuma for final UX review & approval

ericpgreen2 avatar Jul 09 '24 19:07 ericpgreen2

Screenshot 2024-07-09 at 1 04 04 PM

I'm getting a 404 when I click on the Login/Signup option under the avatar

ericokuma avatar Jul 09 '24 20:07 ericokuma

Also agree with @ericpgreen2 that the avatar should be present in the top right of every page in Rill Developer.

Looks like it's currently only on the following pages:

  • metric .yamls
  • dashboards

ericokuma avatar Jul 09 '24 20:07 ericokuma

  1. We'll need the avatar to also appear on this page. @jkhwu, for now it's just a floating avatar in the top right?

Screenshot 2024-07-15 at 9 04 59 AM

Also:

  1. the avatar disappears for a a couple of secs after I login

  2. we probably need a loading state for when user logs out to indicate confirmation that of user clicking on logout

https://www.loom.com/share/a519da4a60044144a213ed6d868ca224?sid=d7827a0e-c966-49cd-87f4-ebe4cae487fa

ericokuma avatar Jul 15 '24 16:07 ericokuma

Funky behavior between Rill Developer and Rill Cloud:

  1. If I'm logged into Rill Cloud, then refresh on my Rill Developer, it doesn't detect my logged in state. Only when I click on login/signup in Rill Developer does it pull my Rill Cloud auth
  2. If I log out of Rill Cloud, then refresh on my Rill Developer, it gets into a weird state that says "avatar". When I refresh, it displays my profile image and remains logged in.

https://www.loom.com/share/e2be5e94d56d4fc19348bde09d33a337?sid=4602f307-3c99-4a5b-b9d5-efaba61c1656

  1. When I am logged into both Rill Cloud and Rill Developer and then log out of Rill Developer, then log out of Rill Cloud. When I try logging back into Rill Cloud, it ends up taking me to a weird admin.rilldata.com page

https://www.loom.com/share/6da4f8dc86af419babb5a8aa46d28890?sid=aca4b07d-6772-4151-a4d2-e258f67945bb

UXQA:

  1. Loading state functionally is good! I was thinking of a different loading spinner. The one I'm thinking of is not the cube spinner but the circle spinner inside of buttons. But this works for now. We can come back to it after!
  2. Looks like the avatar is on every page except for Connector tables Screenshot 2024-07-16 at 8 59 20 AM
  3. Loading icon and avatar display after login auth is a bit jumpy https://www.loom.com/share/9311077101394a80b6682afa9ec54b34?sid=dfea1a15-ccb8-4e15-bd43-9781c7e32b64

ericokuma avatar Jul 16 '24 15:07 ericokuma