Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Fix key "id" in result table issue of the workbench #890

Merged
merged 13 commits into from
Dec 7, 2020

Conversation

chloe-zh
Copy link
Member

@chloe-zh chloe-zh commented Dec 4, 2020

Issue #, if available:
#875

Description of changes:

  • Created DataRow struct with properties rowId and data as one data row, replaced the dictionary type with DataRow in result set
  • Changed variable names in function getQueryResultsForTable to more readable and understandable ones
  • Refactored affected files
  • Modified mock data and updated tests

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Dec 4, 2020

Codecov Report

Merging #890 (f1683c4) into develop (a00a73d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #890   +/-   ##
==========================================
  Coverage      99.85%   99.85%           
  Complexity      2146     2146           
==========================================
  Files            216      216           
  Lines           4833     4833           
  Branches         318      318           
==========================================
  Hits            4826     4826           
  Misses             5        5           
  Partials           2        2           

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 a00a73d...f1683c4. Read the comment docs.

@chloe-zh chloe-zh requested a review from penghuo December 4, 2020 02:34
Copy link
Contributor

@penghuo penghuo left a comment

Choose a reason for hiding this comment

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

Do we need a quick sync with UX for UI change approval?

Copy link
Contributor

@joshuali925 joshuali925 left a comment

Choose a reason for hiding this comment

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

thanks

Copy link
Contributor

@davidcui1225 davidcui1225 left a comment

Choose a reason for hiding this comment

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

LGTM- one minor comment

@joshuali925 joshuali925 merged commit 793d92d into opendistro-for-elasticsearch:develop Dec 7, 2020
joshuali925 added a commit that referenced this pull request Dec 7, 2020
* removed key "id" from result table

* removed cache files from PR

* remove unnecessary lines

* fixed search bar

* update

* update

* update

* update

* update

Co-authored-by: Joshua <joshuali925@gmail.com>
chloe-zh added a commit to chloe-zh/sql that referenced this pull request Dec 15, 2020
…lasticsearch#890)

* removed key "id" from result table

* removed cache files from PR

* remove unnecessary lines

* fixed search bar

* update

* update

* update

* update

* update

Co-authored-by: Joshua <joshuali925@gmail.com>
chloe-zh added a commit to chloe-zh/sql that referenced this pull request Dec 15, 2020
…lasticsearch#890)

* removed key "id" from result table

* removed cache files from PR

* remove unnecessary lines

* fixed search bar

* update

* update

* update

* update

* update

Co-authored-by: Joshua <joshuali925@gmail.com>
chloe-zh added a commit to chloe-zh/sql that referenced this pull request Dec 15, 2020
…lasticsearch#890)

* removed key "id" from result table

* removed cache files from PR

* remove unnecessary lines

* fixed search bar

* update

* update

* update

* update

* update

Co-authored-by: Joshua <joshuali925@gmail.com>
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.

None yet

4 participants