Skip to content

[dashboard] allow editing user information #11023

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

Merged
merged 1 commit into from
Jul 5, 2022

Conversation

svenefftinge
Copy link
Member

@svenefftinge svenefftinge commented Jun 30, 2022

Description

This change allows users to update their name, email and company name.
It also nudges users in the
Screenshot 2022-07-05 at 16 43 40
workspaces view on the second day (or after) and then once a year.

Related Issue(s)

Fixes #10999

How to test

Use the preview env,

  • create a new user
  • verify you don't get nudged (no modal) on /workspaces
  • set system time to yesterday and reload page. You should now see a modal.
  • either update your profile or simply close the modal (cancel or close).
  • reload page, you should not see the modal anymore (only after one year, which you can verify again by setting your system date)
  • Check the settings page

Release Notes

You can now update your profile information (name, email, company)

Documentation

Werft options:

  • /werft with-preview

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-sefftinge-allow-user-profile-updates-10999.1 because the annotations in the pull request description changed
(with .werft/ from main)

@svenefftinge svenefftinge force-pushed the sefftinge/allow-user-profile-updates-10999 branch from 77276cc to eda0f85 Compare June 30, 2022 07:10
@roboquat roboquat added size/L and removed size/XS labels Jun 30, 2022
@svenefftinge svenefftinge force-pushed the sefftinge/allow-user-profile-updates-10999 branch from eda0f85 to 04c5c35 Compare June 30, 2022 12:59
@svenefftinge svenefftinge requested a review from a team June 30, 2022 13:04
@svenefftinge svenefftinge marked this pull request as ready for review June 30, 2022 13:13
@svenefftinge svenefftinge requested a review from a team June 30, 2022 13:13
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jun 30, 2022
@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 30, 2022

Looking at this now! 👀

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

UX LAGTM (Looks Almost Good To Me)! Thanks, @svenefftinge!

Cross-posting a relevant issue in #3957 in case it can be also closed.

Left some minor comments below. We can open a follow-up issue for anything that can be skipped in this iteration.

@svenefftinge svenefftinge force-pushed the sefftinge/allow-user-profile-updates-10999 branch from 04c5c35 to ad017ac Compare June 30, 2022 16:29
Copy link
Contributor

@jldec jldec left a comment

Choose a reason for hiding this comment

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

LGTM - I think this is a good enhancement. A few comments.

  • Users ask to change their avatar almost as often as their name. (followup feature?)

  • Offering to change the name and email during sign-up flow is OK and may be a good opportunity to remind new users of our terms and privacy policy.

  • I was unable to test (in preview) whether this modal is triggered when an additional git integration is added through settings/integrations. Generally adding an integration should not result in modification of this information.

  • Including the hints for how to set env vars to override git email/author/committer is helpful, and I would not remove it.

/hold for more technical engineering review

@jldec
Copy link
Contributor

jldec commented Jul 1, 2022

Followup: I managed to test adding an integration using a non-incognito window, and it did not prompt for profile changes - so all good.

(The error I saw earlier when trying to add an integration was using a preview environment from an incognito window.)

@svenefftinge svenefftinge force-pushed the sefftinge/allow-user-profile-updates-10999 branch from ad017ac to d92f361 Compare July 1, 2022 07:03
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Left one more round of comments, mostly non-blocking. 🥊
Thanks for tackling all these in one step! 🙏

@svenefftinge svenefftinge force-pushed the sefftinge/allow-user-profile-updates-10999 branch 2 times, most recently from a81ea7c to 8229fb5 Compare July 1, 2022 11:23
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Keeping the hold in case we'd like to:
1. Fix one outstanding UX issue mentioned in #11023 (comment).
2. Ask for another pair of eyes to take a closer look at the code changes. Cc @geropl

@svenefftinge svenefftinge requested a review from a team July 1, 2022 14:28
@geropl
Copy link
Member

geropl commented Jul 4, 2022

/werft run

👍 started the job as gitpod-build-sefftinge-allow-user-profile-updates-10999.9
(with .werft/ from main)

@geropl
Copy link
Member

geropl commented Jul 4, 2022

@svenefftinge From the issues I don't understand how urgent this is. I feel we have a good opportunity to nicely solve our handling of email addresses if we delay it until end of next week and ask @AlexTugarev to clean it up beforehand (sth he's keen on for some time already, but ooo this week).
If the temporary gap is fine we can merge as-is, though, and I will ask him to do it anyway afterwards.

@geropl geropl self-assigned this Jul 4, 2022
@svenefftinge
Copy link
Member Author

svenefftinge commented Jul 4, 2022

/werft run

👍 started the job as gitpod-build-sefftinge-allow-user-profile-updates-10999.10
(with .werft/ from main)

@svenefftinge svenefftinge force-pushed the sefftinge/allow-user-profile-updates-10999 branch from 8229fb5 to d2a1075 Compare July 5, 2022 06:31
@svenefftinge
Copy link
Member Author

@geropl I have addressed the two nits and left a comment on the other feedback. I'd like to land this as it unblocks @phimae's work on the lead identification.

Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested and works 👍

@roboquat roboquat merged commit 8bf152b into main Jul 5, 2022
@roboquat roboquat deleted the sefftinge/allow-user-profile-updates-10999 branch July 5, 2022 06:48
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production release-note size/XL team: product-design team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow User Profile Updates
5 participants