search

a-marenkov / gsheets

A Dart library for working with Google Sheets API.
Other
80 stars 31 forks source link

Skip chart type sheets when loading/parsing the spreadsheet #17

Closed balazsbonis closed 4 years ago

balazsbonis commented 4 years ago

If you have a sheet that is made up of a chart, it will have a different set of parameters than what the Worksheet factory expects and then fail on line 667 with a null reference exception: sheetJson['properties']['gridProperties']['rowCount'], Chart type sheets will not have gridProperties at all. I don't know if this is the best solution for the problem - we might want to load those sheets into the system, but for the fundamental features to work I think it's safe to skip those sheets from loading for the time being.

a-marenkov commented 4 years ago

If you have a sheet that is made up of a chart, it will have a different set of parameters than what the Worksheet factory expects and then fail on line 667 with a null reference exception: sheetJson['properties']['gridProperties']['rowCount'], Chart type sheets will not have gridProperties at all. I don't know if this is the best solution for the problem - we might want to load those sheets into the system, but for the fundamental features to work I think it's safe to skip those sheets from loading for the time being.

Hi! Thanks for contributing to gsheets.

I agree with you, those sheets should be skipped. I didn't think of this case.

There is one thing i'd like to pick your brain on - you say that there will be no gridProperties at all, maybe it's better to filter sheets by checking if gridProperties not null rather than containsKey('charts')? What do you say?

Thanks!

balazsbonis commented 4 years ago

Hi! Thanks for contributing to gsheets.

I agree with you, those sheets should be skipped. I didn't think of this case.

There is one thing i'd like to pick your brain on - you say that there will be no gridProperties at all, maybe it's better to filter sheets by checking if gridProperties not null rather than containsKey('charts')? What do you say?

Thanks!

Hi!

Yeah, that does make a lot of sense. I did look up the official Sheets API documentation for what values are possible in that particular property: https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#SheetType According to this, we should only be dealing with GRIDs, so I added a commit in there to filter down to those. I reckon this might be the safest/nicest way to get it done, but let me know what you think!