Skip to content

Remove @JsMethod from JsTable and JsTreeTable features that do not currently work. #931

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.

Sign up for GitHub

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
devinrsmith merged 5 commits into from
Jul 30, 2021
Merged

Remove @JsMethod from JsTable and JsTreeTable features that do not currently work. #931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5603cba
Comment out JsTable and JsTreeTable features that do not currently work.
mofojed Jul 29, 2021
8e742c2
Remove JsMethod instead of commenting out code
mofojed Jul 30, 2021
57e6d49
Add back the @JsMethod annotations, but comment out and add a TODO;
mofojed Jul 30, 2021
8ce56dc
Clean up based on Colin's review
mofojed Jul 30, 2021
35c5dd9
Missed one thing in JsTreeTable
mofojed Jul 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions web/client-api/src/main/java/io/deephaven/ide/shared/IdeSession.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ public Promise<JsTable> getTable(String name) {
return table;
}

// TODO: #37: Need SmartKey support for this functionality
@JsIgnore
public Promise<JsTreeTable> getTreeTable(String name) {
return connection.getTreeTable(name, result);
}
Expand Down
47 changes: 26 additions & 21 deletions web/client-api/src/main/java/io/deephaven/web/client/api/JsTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,19 +209,6 @@ public boolean hasInputTable() {
return hasInputTable;
}

@JsProperty
public JsTotalsTableConfig getTotalsTableConfig() {
// we want to communicate to the JS dev that there is no default config, so we allow
// returning null here, rather than a default config. They can then easily build a
// default config, but without this ability, there is no way to indicate that the
// config omitted a totals table
String config = lastVisibleState().getTableDef().getAttributes().getTotalsTableConfig();
if (config == null) {
return null;
}
return JsTotalsTableConfig.parse(config);
}

@JsMethod
public Promise<JsInputTable> inputTable() {
if (!hasInputTable) {
Expand Down Expand Up @@ -572,15 +559,29 @@ public Promise<JsTable> copy(boolean resolved) {
}
return Promise.resolve(new JsTable(this));
}

@JsMethod
public Promise<JsTotalsTable> getTotalsTable(@JsOptional Object config) {
// TODO: #37: Need SmartKey support for this functionality
// @JsMethod
public Promise<JsTotalsTable> getTotalsTable(/* @JsOptional */Object config) {
// fetch the handle and wrap it in a new jstable. listen for changes
// on the parent table, and re-fetch each time.

return fetchTotals(config, this::lastVisibleState);
}

// TODO: #37: Need SmartKey support for this functionality
// @JsMethod
public JsTotalsTableConfig getTotalsTableConfig() {
// we want to communicate to the JS dev that there is no default config, so we allow
// returning null here, rather than a default config. They can then easily build a
// default config, but without this ability, there is no way to indicate that the
// config omitted a totals table
String config = lastVisibleState().getTableDef().getAttributes().getTotalsTableConfig();
if (config == null) {
return null;
}
return JsTotalsTableConfig.parse(config);
}

private Promise<JsTotalsTable> fetchTotals(Object config, JsProvider<ClientTableState> state) {

JsTotalsTableConfig directive = getTotalsDirectiveFromOptionalConfig(config);
Expand Down Expand Up @@ -688,8 +689,9 @@ private JsTotalsTableConfig getTotalsDirectiveFromOptionalConfig(Object config)
}
}

@JsMethod
public Promise<JsTotalsTable> getGrandTotalsTable(@JsOptional Object config) {
// TODO: #37: Need SmartKey support for this functionality
// @JsMethod
public Promise<JsTotalsTable> getGrandTotalsTable(/* @JsOptional */Object config) {
// As in getTotalsTable, but this time we want to skip any filters - this could mean use the
// most-derived table which has no filter, or the least-derived table which has all custom columns.
// Currently, these two mean the same thing.
Expand All @@ -703,7 +705,8 @@ public Promise<JsTotalsTable> getGrandTotalsTable(@JsOptional Object config) {
});
}

@JsMethod
// TODO: #37: Need SmartKey support for this functionality
// @JsMethod
public Promise<JsTreeTable> rollup(Object configObject) {
Objects.requireNonNull(configObject, "Table.rollup configuration");
final JsRollupConfig config;
Expand All @@ -721,7 +724,8 @@ public Promise<JsTreeTable> rollup(Object configObject) {
}, "rollup " + Global.JSON.stringify(config)).refetch(this, workerConnection.metadata()).then(state -> new JsTreeTable(state, workerConnection).finishFetch());
}

@JsMethod
// TODO: #37: Need SmartKey support for this functionality
// @JsMethod
public Promise<JsTreeTable> treeTable(Object configObject) {
Objects.requireNonNull(configObject, "Table.treeTable configuration");
final JsTreeTableConfig config;
Expand Down Expand Up @@ -841,7 +845,8 @@ public Promise<TableMap> byExternal(Object keys, @JsOptional Boolean dropKeys) {
}).refetch();
}

@JsMethod
// TODO: #697: Column statistic support
// @JsMethod
public Promise<JsColumnStatistics> getColumnStatistics(Column column) {
return Callbacks.<ColumnStatistics, String>promise(null, c -> {
// workerConnection.getServer().getColumnStatisticsForTable(state().getHandle(), column.getName(), c);
Expand Down