Skip to content

Conversation

@teeohhem
Copy link
Contributor

Fixes: HDX-3116

@changeset-bot
Copy link

changeset-bot bot commented Dec 29, 2025

🦋 Changeset detected

Latest commit: d6b2ff2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Dec 29, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview, Comment Dec 29, 2025 9:28pm

@claude
Copy link

claude bot commented Dec 29, 2025

Code Review

No critical issues found.

Summary

This PR fixes default values in SourceForm by adding defaultValue to all useWatch calls, preventing form fields from being undefined before data loads. The fix follows react-hook-form best practices.

Positive Changes

  • ✅ Consistent pattern applied across all useWatch calls
  • ✅ Comprehensive E2E test coverage added validating form fields for all source types
  • ✅ Test ID consistency fix (edit-sources-menu-item)
  • ✅ Unused parameter cleanup in SessionTableModelForm

Minor Observations (non-blocking)

  • The defaultValue pattern is repeated many times - consider if a wrapper hook could reduce duplication in future refactoring
  • Tests skip fullstack mode (line 99-102) - acceptable workaround documented inline

Recommendation: ✅ Ready to merge

@github-actions
Copy link
Contributor

github-actions bot commented Dec 29, 2025

E2E Test Results

All tests passed • 51 passed • 5 skipped • 705s

Status Count
✅ Passed 51
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 5

Tests ran across 4 shards in parallel.

View full report →

const watchedConnection = useWatch({
control,
name: 'connection',
defaultValue: source?.connection,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was the key fix (setting defaults for each of these)

Copy link
Member

@brandon-pereira brandon-pereira left a comment

Choose a reason for hiding this comment

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

LGTM

@kodiakhq kodiakhq bot merged commit 8584b4a into main Dec 29, 2025
12 checks passed
@kodiakhq kodiakhq bot deleted the tom/fix-source-dialog branch December 29, 2025 21:29
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.

3 participants