seung-00 commented on code in PR #5006: URL: https://github.com/apache/zeppelin/pull/5006#discussion_r2285664576
########## zeppelin-web-angular/src/app/services/angular-drag-drop.service.ts: ########## @@ -0,0 +1,326 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Injectable } from '@angular/core'; +import * as angular from 'angular'; + +@Injectable({ + providedIn: 'root' +}) +export class AngularDragDropService { + constructor() {} + + addDragDropDirectives(module: angular.IModule): void { + // Drag and drop service to maintain state across directives + // tslint:disable-next-line:no-any + module.service('customDragDropService', [ + '$parse', + // tslint:disable-next-line:no-any + function($parse: any) { Review Comment: Would it be hard to use the types provided by @types/angular here as well? And I'm not fully sure, but if the TS compiler can infer the type in some places without explicitly declaring it, it might be nice to rely on inference. ########## zeppelin-web-angular/src/app/services/table-data-adapter.service.ts: ########## @@ -0,0 +1,115 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Injectable } from '@angular/core'; +import { TableData } from '@zeppelin/visualization'; + +interface ClassicColumn { + name: string; + index: number; + aggr: string; +} + +interface ClassicTableData { + columns: ClassicColumn[]; + // tslint:disable-next-line:no-any + rows: any[][]; + comment: string; +} + +@Injectable({ + providedIn: 'root' +}) +export class TableDataAdapterService { + constructor() {} + + /** + * Convert modern TableData to classic format expected by AngularJS visualizations + */ + convertToClassicFormat(modernTableData: TableData): ClassicTableData { + const classicColumns: ClassicColumn[] = modernTableData.columns.map((columnName, index) => ({ + name: columnName, + index, + aggr: 'sum' // Default aggregation + })); + + // Convert rows from modern format (objects) to classic format (arrays) + // tslint:disable-next-line:no-any + const classicRows: any[][] = []; + + if (modernTableData.rows && modernTableData.rows.length > 0) { + // Check if rows are objects (modern format) or arrays (already classic format) + const firstRow = modernTableData.rows[0]; + + if (Array.isArray(firstRow)) { + // Already in classic format (array of arrays) + for (const row of modernTableData.rows) { + classicRows.push(row); + } + } else if (typeof firstRow === 'object' && firstRow !== null) { + // Modern format (array of objects) - convert to classic format + modernTableData.rows.forEach(rowObj => { + // tslint:disable-next-line:no-any + const rowArray: any[] = []; + modernTableData.columns.forEach(columnName => { + rowArray.push(rowObj[columnName]); + }); + classicRows.push(rowArray); + }); + } + } + + return { + columns: classicColumns, + rows: classicRows, + comment: '' // Modern TableData doesn't have comment field + }; + } + + /** + * Create a classic TableData-like object with the required methods + */ + // tslint:disable-next-line:no-any + createClassicTableDataProxy(modernTableData: TableData): any { + const classicData = this.convertToClassicFormat(modernTableData); + + // Create a proxy object that mimics the classic TableData interface + const proxy = { + columns: classicData.columns, + rows: classicData.rows, + comment: classicData.comment, + + // Add any methods that classic visualizations might expect + // tslint:disable-next-line:no-any + loadParagraphResult: function(paragraphResult: any) { + // Delegate to modern TableData's method + modernTableData.loadParagraphResult(paragraphResult); + + // Update proxy data after loading + const updatedClassicData = this.convertToClassicFormat(modernTableData); + this.columns = updatedClassicData.columns; + this.rows = updatedClassicData.rows; + this.comment = updatedClassicData.comment; + }.bind(this), + + // Refresh data from modern TableData + refresh: function() { + const updatedClassicData = this.convertToClassicFormat(modernTableData); + this.columns = updatedClassicData.columns; + this.rows = updatedClassicData.rows; + this.comment = updatedClassicData.comment; + }.bind(this) Review Comment: How about using an arrow function instead of `bind`? I think it's more readable and more modern way. ########## zeppelin-web-angular/src/app/services/table-data-adapter.service.ts: ########## @@ -0,0 +1,115 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Injectable } from '@angular/core'; +import { TableData } from '@zeppelin/visualization'; + +interface ClassicColumn { + name: string; + index: number; + aggr: string; +} + +interface ClassicTableData { + columns: ClassicColumn[]; + // tslint:disable-next-line:no-any + rows: any[][]; + comment: string; +} + +@Injectable({ + providedIn: 'root' +}) +export class TableDataAdapterService { + constructor() {} + + /** + * Convert modern TableData to classic format expected by AngularJS visualizations + */ + convertToClassicFormat(modernTableData: TableData): ClassicTableData { + const classicColumns: ClassicColumn[] = modernTableData.columns.map((columnName, index) => ({ + name: columnName, + index, + aggr: 'sum' // Default aggregation + })); + + // Convert rows from modern format (objects) to classic format (arrays) + // tslint:disable-next-line:no-any + const classicRows: any[][] = []; + + if (modernTableData.rows && modernTableData.rows.length > 0) { + // Check if rows are objects (modern format) or arrays (already classic format) + const firstRow = modernTableData.rows[0]; + + if (Array.isArray(firstRow)) { + // Already in classic format (array of arrays) + for (const row of modernTableData.rows) { + classicRows.push(row); + } + } else if (typeof firstRow === 'object' && firstRow !== null) { + // Modern format (array of objects) - convert to classic format + modernTableData.rows.forEach(rowObj => { + // tslint:disable-next-line:no-any + const rowArray: any[] = []; + modernTableData.columns.forEach(columnName => { + rowArray.push(rowObj[columnName]); + }); + classicRows.push(rowArray); + }); + } + } + + return { + columns: classicColumns, + rows: classicRows, + comment: '' // Modern TableData doesn't have comment field + }; + } + + /** + * Create a classic TableData-like object with the required methods + */ + // tslint:disable-next-line:no-any + createClassicTableDataProxy(modernTableData: TableData): any { + const classicData = this.convertToClassicFormat(modernTableData); + + // Create a proxy object that mimics the classic TableData interface + const proxy = { + columns: classicData.columns, + rows: classicData.rows, + comment: classicData.comment, + + // Add any methods that classic visualizations might expect + // tslint:disable-next-line:no-any + loadParagraphResult: function(paragraphResult: any) { + // Delegate to modern TableData's method + modernTableData.loadParagraphResult(paragraphResult); + + // Update proxy data after loading + const updatedClassicData = this.convertToClassicFormat(modernTableData); + this.columns = updatedClassicData.columns; + this.rows = updatedClassicData.rows; + this.comment = updatedClassicData.comment; Review Comment: I'm not sure, but it looks like the proxy was supposed to be updated. However, it seems that the `TableDataAdapterService` instance is being updated instead. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@zeppelin.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
