ExcelExportService inconsistently does not support getItemMetadata for NoDataRow rows.

arthur-clifford commented 3 years ago

I'm submitting a Bug report

This is a bug in an inconsistent slickgrid standard sense rather than a failing to work as designed sense.

Your Environment

Software	Version(s)
Angular	10.0.14
Angular-Slickgrid	2.25.1
TypeScript	3.9.7

Describe the Bug

Soon, on the main slickgrid repo I will issue a pull request for customizations which, if accepted, will allow for the default dataView to let developers provide additional rows before and after groups or optionally before or after the main table data. These can , or will likely be, extensions of the NoDataRow class. Whether y'all choose to accept that pull request is yet to be seen, but because I'm working toward that I'm seeing that Excel output breaks for custom rows because the ExcelExportService isn't using item metadata providers which I would argue is a consistency-bug/issue that prevents extensibility the rest of slickgrid allows for.

   pushAllGridRowDataToArray(originalDaraArray, columns) {
        const lineCount = this._dataView && this._dataView.getLength && this._dataView.getLength();
        // loop through all the grid rows of data
        for (let rowNumber = 0; rowNumber < lineCount; rowNumber++) {
            const itemObj = this._dataView.getItem(rowNumber);
            if (itemObj) {
                // Normal row (not grouped by anything) would have an ID which was predefined in the Grid Columns definition
                if (itemObj[this.datasetIdName] !== null && itemObj[this.datasetIdName] !== undefined) {
                    // get regular row item data
                    originalDaraArray.push(this.readRegularRowData(columns, rowNumber, itemObj));
                }
                else if (this._hasGroupedItems && itemObj.__groupTotals === undefined) {
                    // get the group row
                    originalDaraArray.push([this.readGroupedTitleRow(itemObj)]);
                }
                else if (itemObj.__groupTotals) {
                    // else if the row is a Group By and we have agreggators, then a property of '__groupTotals' would exist under that object
                    originalDaraArray.push(this.readGroupedTotalRow(columns, itemObj));
                }
            }
        }
        return originalDaraArray;
    }

Note that the code above is deducing functionality based on whether properties are set or not. However, these expetactions are limited to the dataView's two or three row types.

Having been fairly deep in the slick.grid and slick.dataview code I know that internally slick.grid uses getItemMetadataProvider (see slick.grid getFormatter ) for each item when formatting. slick.datavew helps with the gathering of "rows" which are either data rows or objects that extend NoDataRow. But Rows/items are rendered based on their metadata providers providing a formatter.

Here is one of the item metadata providers defined by slickgrid:

function getGroupRowMetadata(item) {
      var groupLevel = item && item.level;
      return {
        selectable: false,
        focusable: options.groupFocusable,
        cssClasses: options.groupCssClass + ' slick-group-level-' + groupLevel,
        formatter: options.includeHeaderTotals && options.totalsFormatter,
        columns: {
             0: {
                colspan: options.includeHeaderTotals?"1":"*",
                formatter: options.groupFormatter,
                editor: null
            }
        }
    };
}

Note the columns section that is either making the colspan 1 or * based on whether includeHeaderTotals is set and then for column 0 there is an additional formatter. Note: I'm sure you know this stuff too, but I am sharing what I've found so we're on a sameish page.

Steps to Reproduce

To reproduce you can see what you get when you output a grid that uses the item detail plugin, you end up with 4 blank rows, however, if the detail view was a NoDataRow and there were an itemMetadataProvider defined for it, it may be possible to provide the developer options for excel export so that the cell is not empty even if it doesn't look the same as the data view output in the grid. (In fairness I haven't checked to see if angular-slickgrid was updated to provide an itemMetadataProvider for the itemdetail plugin, howver, I do know that the Excel output is blank though in the online demo which I assume is not desirable)

Expected Behavior

Custom row types (row classes extending NoDataItem) should be exportable

Either with a custom dataView or if the default dataView is eventually updated to include creation of additional custom row types, the getItemMetadata functionality should drive output even for things like Excel or any other export type one might imagine for later.

Current Behavior

Custom row types (row classes extending NoDataItem) if detected and do not contain a title or __groupedTotals or do not meet other criteria will export to Excel as "undefined" or blank as in the case of the itemDetail content.

Possible Solution

As stated, it will require refactoring the function at the beginning of this report (or a new function it may call) to utilize the dataView's getItemMetadataProvider function with a given row and then use that information provided therein to drive ouput.

You would have at least two options: 1) les desirable as it is still deducy, for any formatter ( item-level or column level), you could provide 'excel' as the grid parameter. That would mean we treat grid as a context parameter and check to see if it is a grid object or switch on that to determine how we want to proceed.

2) probably better and I think you do it elsewhere, you can specify excelExportOptions and allow for at least a formatter and any other parameters you wish to make available.

    function getItemDetailMetadata(item) {
      var groupLevel = item && item.group && item.group.level;      
      return {
        selectable: false,
        focusable: options.totalsFocusable,
        cssClasses: options.totalsCssClass + ' slick-group-level-' + groupLevel,
        formatter: options.totalsFormatter,
        excelFormatter: {
          rowsCount: 1,
          columns: {
            0: {
               colspan: "*",
               cellFormatter:  MyCoolDetailViewFormatter
            }
          }
       }
        editor: null
      };
    }

Of course what is included in excelFormatter would be up to you. The rowsCount was basically to suggest that we may want to provide a hint as to how many rows to allow for in the output which may or not be as many rows as are needed by the grid to accommodate content. So, where itemDetail defaults to 4 rows, this provider would reduce that to 1 row and the MyCoolDetailViewFormatter would know how to provide single row output for the item assuming it is cached or is programatically available in the export flow to the cellFormatter function.

That also suggests that rowsCount could be 0 as a way to exclude an "item"/row type from output to excel and at least get rid of the itemDetail view 4-row gap in Excel output. When rowsCount=0 it would of course be silly to have a formatter too.

Code Sample

When I am able to commit stuff and make a pull request on the main slickgrid repo I will be better able to demonstrate. In the meantime you can confirm what is or isn't being done by reviewing the code at the beginning of this report and by comparing how the itemMetadataProvider tech works with outputting cells. https://github.com/6pac/SlickGrid/blob/master/slick.grid.js https://github.com/6pac/SlickGrid/blob/master/slick.dataview.js https://github.com/6pac/SlickGrid/blob/master/slick.groupitemmetadataprovider.js

ghiscoding commented 3 years ago

PR are welcome, especially so if you know what you want and I don't have much time to invest on that. I created the file export and then the Excel export from scratch and I included regular & grouped rows and that is where I stopped. I helped on creating the Row Detail plugin in SlickGrid but I never used it since we never really had use case for it but I did try the export on it and I see your point and if you want to submit a PR then I'll be more than happy to review/merge it. The way both Export Services work are simply to run through each row and parse whatever it has to in which ever way it has to. It was also purposely built to be run as WYIWYG (What You See is What You Get). If you think it's missing something then go ahead with a PR, I don't have the time to do the code change but I can bring guidance.

On another note, if you want to provide custom formatter, you should use the properties that I created for that purpose, which are shown below. It simply goes by order of priority (it first check if exportWithFormatter is enabled, then goes to which Formatter(s) to use, the custom one being the first or else regular Formatter, ...). I use the exportCustomFormatter quite often. https://github.com/ghiscoding/Angular-Slickgrid/blob/e50a060103cd7a48d03e257ffebae37fe311e3d7/src/app/modules/angular-slickgrid/models/column.interface.ts#L67-L77

A side note, I think that the Example 15 ColSpan also uses that metadata provider and is not currently exporting as what is shown in the UI, so I assume that your PR will also fix that which could be another grid to test with... and again that is not a feature that I've ever used so I never care to look at the export. https://github.com/ghiscoding/Angular-Slickgrid/blob/e50a060103cd7a48d03e257ffebae37fe311e3d7/src/app/examples/grid-colspan.component.ts#L121-L139

ghiscoding commented 3 years ago

I fixed the export with colspan in latest PR #730 by reading the item metadata so that one is done. However please that I also created a new ItemMetadata interface that was missing, so if you want to add something to the metadata that you want to read in the export then you will need to update the interface as well.

https://github.com/ghiscoding/Angular-Slickgrid/blob/003738c49dcc460521b28141fd821e2b4f613ee7/src/app/modules/angular-slickgrid/models/itemMetadata.interface.ts#L8-L43

ghiscoding commented 3 years ago

Some update, the colspan Export is now fixed and released, it reads the item metadata and I also created ItemMetadata interface since it was missing. If you still looking at adding something to the item metadata, you'll have to modify that interface.

See latest version 2.28.1 release

ghiscoding commented 3 years ago

I'm converting this into a Discussion since half of the issue was fixed while the other half needs more discussions

ghiscoding / Angular-Slickgrid