search

AmericanRedCross / damage-assessment-bot

Chatbot for immediate damage and needs assessments
BSD 3-Clause "New" or "Revised" License
7 stars 3 forks source link

CSV Upload #149

Closed blueelvis closed 5 years ago

blueelvis commented 5 years ago

Hey Max!

Following has been taken care of in the following PR -

  1. Validate Header Names. If header name not recognized, fail. Right now, this uses a static file to check all the headers first and then each validation function uses its own set.

  2. If column value not recognized, fail, indicate row number. In this, I provide a generic message like below for each header. I wonder on how to provide the range as well properly... image

  3. Validate the townshipId with the townshipCode in damage-assessment-bot\common\src\system\countries\myanmar\myanmarTownships.json

  4. Check all rows before failing. List out all the rows and their issues once you fail. Please find the CSV which I used for testing and validation over here (Github doesn't allow CSV so unzip it please first) - Format-1.zip

  5. This PR is based on top of your localization PR so there is a merge commit. I have changed a couple of files of yours (Especially the enums). I removed const in some enums (where they had it) as it was not allowing me to use them.

  6. The code for this CSV upload is located at -- damage-assessment-bot\api\src\functions\root\api-upload-csv

Now, a couple of comments requiring your views -

  1. Right now, the searching is case sensitive. I think the header names should be case sensitive but what about the values? Dan mentioned that we could use an Excel Macro so not sure if values should be searched case sensitively or not.
  2. I am working on the Cosmos DB Report Typed interface.
  3. I know there would be comments/code changes required. Please comment so that I can take care of them ASAP :)
  4. I am not sure on how to integrate this with the authentication so I would need some working session.
maxnorth commented 5 years ago

Hey @blueelvis , added a partial review. Let's get these changes addressed before moving onto the admin stack selection story.