Skip to content

fix: dynamo db ui query issues fix #206

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 3 commits into from
May 25, 2025
Merged

fix: dynamo db ui query issues fix #206

merged 3 commits into from
May 25, 2025

Conversation

Blankll
Copy link
Member

@Blankll Blankll commented May 24, 2025

fix: dynamo db ui query issues fix

  • set index name to null when the user selects the main table
  • fix scrollbar issue in UI query
  • fix table/index selection issue when contains index name same with table name

Refs: #142

Signed-off-by: seven <zilisheng1996@gmail.com>
Copy link

codecov bot commented May 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.77%. Comparing base (1ef32a9) to head (021ec1f).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #206   +/-   ##
=======================================
  Coverage   52.77%   52.77%           
=======================================
  Files          10       10           
  Lines         108      108           
  Branches       14       14           
=======================================
  Hits           57       57           
  Misses         49       49           
  Partials        2        2           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Blankll added 2 commits May 25, 2025 00:25
Signed-off-by: seven <zilisheng1996@gmail.com>
…th table name

Signed-off-by: seven <zilisheng1996@gmail.com>
@Blankll Blankll marked this pull request as ready for review May 25, 2025 08:40
@Blankll Blankll requested a review from Copilot May 25, 2025 08:47
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 addresses several UI and query logic issues in the DynamoDB editor, including handling the main-table vs. index selection, fixing scrollbars for long forms and results, and preventing conflicts when table and index names overlap.

  • Introduce a vertical split layout with infinite scroll wrappers for query form and results
  • Change form defaults and types to use null for unset keys and index
  • Update selection logic to set indexName to null for the main table and improve error messaging

Reviewed Changes

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

File Description
src/views/editor/dynamo-editor/components/ui-editor.vue Restructure UI into split panels; add infinite scroll containers; update form model and query logic
src/views/editor/dynamo-editor/components/create-item.vue Remove leftover console.log
src/datasources/dynamoApi.ts Extend QueryParams to allow nullable index and key values
src-tauri/src/dynamo/scan_table.rs Extract detailed error metadata and improve formatting
Comments suppressed due to low confidence (3)

src/views/editor/dynamo-editor/components/ui-editor.vue:422

  • [nitpick] The class name uses “infinity” but the component is called n-infinite-scroll. Consider renaming to infinite-scroll-outer-container for consistency and clarity.
.infinity-scroll-outer-container {

src/views/editor/dynamo-editor/components/ui-editor.vue:68

  • New infinite scroll containers introduce custom scrolling behavior. Consider adding a UI or integration test to verify that dynamic filter items and result rows load correctly when the content exceeds visible height.
<n-infinite-scroll style="height: 100%">

src/datasources/dynamoApi.ts:71

  • Allowing partitionKey.value to be null may lead to unexpected runtime errors downstream if the API consumer expects a string. Ensure the backend or callers can handle null, or provide a default empty string instead.
value: string | null;

try {
loadingRef.value.queryResult = true;
queryResult.value = { columns: [], data: undefined };
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

Restoring queryResult only in the success path may leave stale data visible on errors. Consider resetting or clearing queryResult.value in the catch block (or a finally) so the UI doesn’t display old results when a query fails.

Copilot uses AI. Check for mistakes.

const indices = getDynamoIndexOrTableOption.value(activeConnection.value as DynamoDBConnection);
selectedIndexOrTable.value = indices.find(item => item.value === value);
dynamoQueryForm.value.index = selectedIndexOrTable.value!.value;
selectedIndexOrTable.value = indices.find(({ label }) => label === options.label);
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

[nitpick] Relying on label for matching can be brittle if labels change or collide. It’s safer to match on a unique identifier or value field to ensure the correct option is selected.

Suggested change
selectedIndexOrTable.value = indices.find(({ label }) => label === options.label);
selectedIndexOrTable.value = indices.find(({ key }) => key === options.key);

Copilot uses AI. Check for mistakes.

@Blankll Blankll merged commit c4a2e25 into master May 25, 2025
6 checks passed
@Blankll Blankll deleted the fix/fix-query-issues branch May 25, 2025 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant