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

feat: Add global shortcut to export logs #2336

Conversation

ericlln
Copy link
Contributor

@ericlln ericlln commented Jan 9, 2025

  • Added Ctrl+Alt+Shift+L/Cmd-Option-Shift-L as a global shortcut for exporting support logs
  • When pressed in code-studio, it will download logs with the same contents as logs produced by pressing the 'Export Logs' button in the settings menu
  • When pressed in other contexts: when not logged in, on a reconnect error, or in embed-widgets, the logs are exported without the plugin and server info
  • Tested by E2E tests that navigate to the various contexts, presses the shortcut, and checks if a file was downloaded. Can further inspect the file downloaded in the tests but unsure if that's necessary

Closes #1963

@ericlln ericlln requested a review from mofojed January 9, 2025 15:03
@ericlln ericlln self-assigned this Jan 9, 2025
Copy link
Contributor

github-actions bot commented Jan 9, 2025

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@ericlln
Copy link
Contributor Author

ericlln commented Jan 9, 2025

I have read the CLA Document and I hereby sign the CLA

deephaven-internal added a commit to deephaven/cla that referenced this pull request Jan 9, 2025
Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

I think the e2e test is failing in CI because the timeout may not be long enough - it may take a while for the UI to load in a resource constrained environment. You can increase the timeout for this call specifically by passing in the option: https://playwright.dev/docs/test-timeouts

packages/embed-widget/vite.config.ts Show resolved Hide resolved
packages/code-studio/src/main/App.tsx Show resolved Hide resolved
packages/app-utils/src/components/AppBootstrap.tsx Outdated Show resolved Hide resolved
packages/app-utils/src/components/AppBootstrap.tsx Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.

Project coverage is 46.70%. Comparing base (6a08198) to head (e8c092f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...p-utils/src/utils/createExportLogsContextAction.ts 40.00% 3 Missing ⚠️
packages/code-studio/src/index.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #2336    +/-   ##
========================================
  Coverage   46.70%   46.70%            
========================================
  Files         704      705     +1     
  Lines       39044    39051     +7     
  Branches     9757     9943   +186     
========================================
+ Hits        18234    18240     +6     
- Misses      20799    20800     +1     
  Partials       11       11            
Flag Coverage Δ
unit 46.70% <55.55%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ericlln ericlln requested a review from mofojed January 14, 2025 14:46
mofojed
mofojed previously approved these changes Jan 14, 2025
Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Looks great and resolves the ticket as specified! Going forward we can improve these error situations by having a visible button so the user doesn't need to remember a keyboard shortcut.

tests/shortcuts.spec.ts Outdated Show resolved Hide resolved
tests/shortcuts.spec.ts Outdated Show resolved Hide resolved
Co-authored-by: Mike Bender <mikebender@deephaven.io>
Co-authored-by: Mike Bender <mikebender@deephaven.io>
@ericlln ericlln requested a review from mofojed January 14, 2025 23:40
@ericlln ericlln merged commit 6e813fd into deephaven:main Jan 17, 2025
11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No way to export support logs before logging in or after there's a connection error
2 participants