Skip to content

[ui-importer] destination setting component and hook #4157

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 8 commits into from
Jun 24, 2025

Conversation

ramprasadagarwal
Copy link
Collaborator

@ramprasadagarwal ramprasadagarwal commented Jun 3, 2025

What changes were proposed in this pull request?

  • Added hook for DataCatalog
  • Sued the hook to populate the engine, database and compute

How was this patch tested?

  • Adding more test cases shortly
image

Please review Hue Contributing Guide before opening a pull request.

Copy link

github-actions bot commented Jun 3, 2025

✅ Test files were modified. Ensure that the tests cover all relevant changes. ✅

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new DataCatalog hook and a DestinationSettings component while updating the importer file preview logic to utilize these new components for configuring destination settings. Key changes include:

  • The addition of a useDataCatalog hook to manage engine, database, and compute selections.
  • Integration of the DestinationSettings component in the file preview along with modifications to the finish import logic.
  • Minor style adjustments and type updates related to importer file types and destination configuration.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
desktop/core/src/desktop/js/utils/hooks/useDataCatalog/useDataCatalog.tsx Introduces a new hook for fetching and managing DataCatalog data.
desktop/core/src/desktop/js/apps/newimporter/types.ts Updates importer types by converting a const enum to an enum and adding DestinationConfig.
desktop/core/src/desktop/js/apps/newimporter/ImporterFilePreview/ImporterFilePreview.tsx Updates finish import logic to utilize destinationConfig and integrates DestinationSettings.
desktop/core/src/desktop/js/apps/newimporter/ImporterFilePreview/ImporterFilePreview.scss Removes a fixed height setting from metadata styling.
desktop/core/src/desktop/js/apps/newimporter/ImporterFilePreview/DestinationSettings/DestinationSettings.tsx Adds a new component for destination configuration and integrates useDataCatalog usage.
desktop/core/src/desktop/js/apps/newimporter/ImporterFilePreview/DestinationSettings/DestinationSettings.scss Provides styling for the new DestinationSettings component.

Copy link

github-actions bot commented Jun 3, 2025

UI Code Coverage Report

Lines Statements Branches Functions
Coverage: 32%
39.24% (30638/78075) 31.07% (14293/45989) 24.1% (2156/8944)

Copy link

github-actions bot commented Jun 3, 2025

Coverage

Backend Code Coverage Report •
FileStmtsMissCoverMissing
TOTAL542242706750% 
report-only-changed-files is enabled. No files were changed during this commit :)

Copy link
Collaborator

@bjornalm bjornalm left a comment

Choose a reason for hiding this comment

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

Nice work, but I'd also like to see the following for this PR:

  • a screenshot for the new UI part
  • unit tests on all new components

Copy link
Collaborator

@bjornalm bjornalm left a comment

Choose a reason for hiding this comment

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

Nice work,

just a small UI misalignment I saw in the screenshot. The rightmost textbox does not have the same height as the dropdowns.

@ramprasadagarwal ramprasadagarwal enabled auto-merge (squash) June 24, 2025 13:09
@ramprasadagarwal ramprasadagarwal merged commit 277a48c into master Jun 24, 2025
9 checks passed
@ramprasadagarwal ramprasadagarwal deleted the feat/importer-7 branch June 24, 2025 13:30
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