search

IDEMSInternational / rapidpro-flow-toolkit

Toolkit for using spreadsheets to create and modify RapidPro flows
GNU Lesser General Public License v2.1
3 stars 1 forks source link

Move multiple sheet reader handling to Content Index Parser #110

Closed istride closed 7 months ago

istride commented 7 months ago

The CompositeSheetReader changed the sheet reader abstraction to accommodate a specific use case - spreadsheet-based authoring of chatbot flows for RapidPro - which has specific requirements that are unlikely to be shared by other use cases.

In order to keep the sheet reader concept general and usable in a broad range of scenarios, the handling of multiple readers that was performed by CompositeSheetReader has been moved to ContentIndexParser.

To be considered a sheet reader it is necessary to implement a single method, get_sheets, which should return a single instance of type Sheet. A Sheet contains a reference to the reader that created it, the name of the sheet and the table data of the sheet. The reader also needs to hold a property, name, that may uniquely identify the reader, within the context it is being used.

geoo89 commented 7 months ago

Upon closer look to the code changes, I do like the changes to the tests, and storing sheets rather than tables within the ContentIndexParser which allows you to give better error messages. This would be nice to keep.