Skip to content

dashboard: set correct default setting for telemetry #8373

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
Feb 24, 2022

Conversation

Pothulapati
Copy link
Contributor

Description

Enable Service Ping seems to be set to false by defaut until
the UI is re-loaded. This fixes it by also adding the retrieval
logic into useEffect thereby calling it everytime, even during
initial render.

Signed-off-by: Tarun Pothulapati [email protected]

Related Issue(s)

Fixes #8344

How to test

Open the admin/settings dashboard, and check if its set to true by default without a refresh.

Release Notes

Show correct admin telemetry settings during first visit

Documentation

@Pothulapati Pothulapati requested a review from a team February 22, 2022 07:08
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Feb 22, 2022
@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #8373 (2a7adf2) into main (aa52b3e) will decrease coverage by 1.13%.
The diff coverage is n/a.

❗ Current head 2a7adf2 differs from pull request most recent head 6764e7a. Consider uploading reports for the commit 6764e7a to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8373      +/-   ##
==========================================
- Coverage   12.31%   11.17%   -1.14%     
==========================================
  Files          20       18       -2     
  Lines        1161      993     -168     
==========================================
- Hits          143      111      -32     
+ Misses       1014      880     -134     
+ Partials        4        2       -2     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa52b3e...6764e7a. Read the comment docs.

@Pothulapati Pothulapati added the team: delivery Issue belongs to the self-hosted team label Feb 22, 2022
@Pothulapati Pothulapati added this to the release/2022.02 milestone Feb 24, 2022
@corneliusludmann
Copy link
Contributor

@Pothulapati Could you rebase and squash this?

@Pothulapati Pothulapati force-pushed the tar/incorrect-admin-setting branch from 6764e7a to 7829016 Compare February 24, 2022 11:50
@Pothulapati
Copy link
Contributor Author

@corneliusludmann Done! (Also, Did a mental reminder to myself to always amend commits instead)

@corneliusludmann
Copy link
Contributor

Done! (Also, Did a mental reminder to myself to always amend commits instead)

Yeah! 🚀 Thanks! 🙏

@corneliusludmann
Copy link
Contributor

@Pothulapati Are you sure the rebase on main was successful? I'm missing this change: #8386 🤔

Fixes #8344

`Enable Service Ping` seems to be set to `false` by defaut until
the UI is re-loaded. This fixes it by also adding the retrieval
logic into `useEffect` thereby calling it everytime, even during
initial render.

Signed-off-by: Tarun Pothulapati <[email protected]>
@Pothulapati Pothulapati force-pushed the tar/incorrect-admin-setting branch from 7829016 to 9d89553 Compare February 24, 2022 12:02
@Pothulapati
Copy link
Contributor Author

Pothulapati commented Feb 24, 2022

@corneliusludmann Now, It's up! 👍🏼

Copy link
Member

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

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

code change looks good to me.

@roboquat roboquat merged commit e720c49 into main Feb 24, 2022
@roboquat roboquat deleted the tar/incorrect-admin-setting branch February 24, 2022 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/XS team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[telemetry] Wrong enable service ping state in admin dashboard after login
4 participants