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

client: remove deprecated authenticatedUser.email #46183

Merged

Conversation

bobheadxi
Copy link
Member

@bobheadxi bobheadxi commented Jan 6, 2023

The authenticatedUsers.email field is deprecated - the equivalent can now be found with authenticatedUsers.emails.find(email => email.isPrimary). This gives more details about the email as well, such as whether it is verified or not, which is required to implement verified email checks (e.g. #46184)

https://github.com/sourcegraph/sourcegraph/blob/fb5a412a9c66c4e08a2fb4dab859a765cdedc1ff/cmd/frontend/graphqlbackend/schema.graphql#L5538-L5542

Test plan

Tests pass still

App preview:

Check out the client app preview documentation to learn more.

@cla-bot cla-bot bot added the cla-signed label Jan 6, 2023
@bobheadxi
Copy link
Member Author

bobheadxi commented Jan 6, 2023

@sg-e2e-regression-test-bob
Copy link

Bundle size report 📦

Initial size Total size Async size Modules
0.01% (+0.15 kb) 0.00% (+0.26 kb) 0.00% (+0.11 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits 85d7ee6 and 179254b or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

@bobheadxi bobheadxi force-pushed the 01-05-client_remove_deprecated_authenticatedUser.email branch 4 times, most recently from fb5a412 to 863f3b3 Compare January 6, 2023 04:57
@bobheadxi bobheadxi marked this pull request as ready for review January 6, 2023 06:38
@bobheadxi bobheadxi requested a review from a team January 6, 2023 06:39
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Jan 6, 2023

Not notifying subscribers because the number of notifying subscribers (11) has exceeded the threshold (10).

@bobheadxi bobheadxi force-pushed the 01-05-client_remove_deprecated_authenticatedUser.email branch from 863f3b3 to c9913ac Compare January 6, 2023 07:05
Copy link
Contributor

@vdavid vdavid left a comment

Choose a reason for hiding this comment

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

LGTM! If the backend is indeed in place then this should work right. Left two very small nits.

client/web/src/Layout.tsx Outdated Show resolved Hide resolved
client/web/src/Layout.tsx Outdated Show resolved Hide resolved
bobheadxi and others added 3 commits January 6, 2023 09:41
Co-authored-by: David Veszelovszki <veszelovszki@gmail.com>
Co-authored-by: David Veszelovszki <veszelovszki@gmail.com>
@bobheadxi bobheadxi force-pushed the 01-05-client_remove_deprecated_authenticatedUser.email branch from 470cc9e to 85d7ee6 Compare January 6, 2023 17:42
@sourcegraph-bot
Copy link
Contributor

Codenotify: Notifying subscribers in OWNERS files for diff 7a6dc29...85d7ee6.

Notify File(s)
@mrnugget dev/sg/internal/check/category.go
dev/sg/internal/check/runner.go
dev/sg/linters/go_checks.go
@sourcegraph/dev-experience dev/ci/go-test.sh
dev/sg/internal/check/category.go
dev/sg/internal/check/runner.go
dev/sg/linters/go_checks.go
enterprise/dev/ci/internal/buildkite/buildkite.go
enterprise/dev/ci/internal/ci/operations.go
enterprise/dev/ci/scripts/annotated-command.sh

@bobheadxi
Copy link
Member Author

Percy change seems unrelated 🤔

image

Can reproduce in S2:

image

@bobheadxi bobheadxi merged commit 7ff8676 into main Jan 6, 2023
@bobheadxi bobheadxi deleted the 01-05-client_remove_deprecated_authenticatedUser.email branch January 6, 2023 18:46
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.

None yet

6 participants