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

Validate custom column names #1013

Merged
merged 7 commits into from
Jan 27, 2023

Conversation

mattrunyon
Copy link
Collaborator

Fixes #939

Does not validate that the names aren't duplicates of existing column names, only of existing custom columns. The API seems to allow custom columns that overwrite an existing column name. For example, table with coiumns A, B, C. Create a custom column A=C and B=A. All of the columns will display column C. If you swap the order of the custom columns, then B will show the original A and A will show C.

The API does not allow invalid custom column names (table column validation) or duplicate custom column names.

@mattrunyon mattrunyon added the bug Something isn't working label Jan 12, 2023
@mattrunyon mattrunyon requested a review from mofojed January 12, 2023 07:42
@mattrunyon mattrunyon self-assigned this Jan 12, 2023
@codecov
Copy link

codecov bot commented Jan 12, 2023

Codecov Report

Merging #1013 (f249e57) into main (06581c1) will increase coverage by 0.74%.
The diff coverage is 95.55%.

@@            Coverage Diff             @@
##             main    #1013      +/-   ##
==========================================
+ Coverage   41.40%   42.15%   +0.74%     
==========================================
  Files         430      430              
  Lines       32104    32115      +11     
  Branches     8040     8053      +13     
==========================================
+ Hits        13293    13538     +245     
+ Misses      18758    18524     -234     
  Partials       53       53              
Flag Coverage Δ
unit 42.15% <95.55%> (+0.74%) ⬆️

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

Impacted Files Coverage Δ
packages/iris-grid/src/IrisGrid.tsx 28.60% <0.00%> (-0.03%) ⬇️
...ages/iris-grid/src/sidebar/CustomColumnBuilder.tsx 92.12% <100.00%> (+90.82%) ⬆️
...ckages/iris-grid/src/sidebar/CustomColumnInput.tsx 100.00% <100.00%> (+90.47%) ⬆️
packages/iris-grid/src/sidebar/InputEditor.tsx 93.44% <100.00%> (+91.77%) ⬆️
...ckages/iris-grid/src/IrisGridTableModelTemplate.ts 46.45% <0.00%> (+0.92%) ⬆️
packages/iris-grid/src/IrisGridProxyModel.ts 68.20% <0.00%> (+2.16%) ⬆️
packages/iris-grid/src/IrisGridModel.ts 31.57% <0.00%> (+10.52%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

mofojed
mofojed previously approved these changes Jan 12, 2023
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.

Would be nice to add some CustomColumnBuilder tests as well.

@mattrunyon
Copy link
Collaborator Author

Would be nice to add some CustomColumnBuilder tests as well.

Ask and you shall receive

The tests found a bug in Tab handling within the monaco editor (monaco was eating the tab). I have the fix for that behavior in this PR. I can split it out if you prefer

@mattrunyon mattrunyon requested a review from mofojed January 12, 2023 22:35
@mattrunyon mattrunyon requested a review from mofojed January 25, 2023 07:20
@mattrunyon mattrunyon force-pushed the custom-column-validation branch from 1018abc to f249e57 Compare January 25, 2023 22:33
@mattrunyon mattrunyon requested a review from dsmmcken January 25, 2023 23:30
@mattrunyon mattrunyon merged commit 1ae6aed into deephaven:main Jan 27, 2023
@mattrunyon mattrunyon deleted the custom-column-validation branch January 27, 2023 18:03
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating a custom column with invalid name errors panel
3 participants