stats4sd / aec_portfolio

A proof of concept for the AEC Consortium Project Management / Assessment System
GNU General Public License v3.0
0 stars 0 forks source link

Project importer #61

Closed dave-mills closed 1 year ago

dave-mills commented 1 year ago

Updates the project importer (fixes #28 , fixes #29 ).

The Feature + Updates

Users can upload an Excel file of project/initiative information. The file data is validated against the same set of rules compared to adding records through the platform.

Technical Notes

The template includes a row of instructions and an exmaple row. The ProjectImport toModel function ignores those rows, but we need the validation to ignore them too. We can use SkipsEmptyRows to ensure the validation is not run against empty rows.

There is also a isEmptyWhen feature that lets us add custom logic to tell the importer when a row should be considered empty - e.g. if $row['code'] === 'example'. However, that feature is not yet published for toModel() importers. See https://github.com/SpartnerNL/Laravel-Excel/pull/3828. Once that PR is merged and published, this will be much easier.

For now, I have added a hack into the prepareForValidation function, which replaces the instruction and example rows with fake data that will pass validation. (It will still be ignored by the toModel function).

Eugh. But it works!


Screenshots:

User must choose which portfolio to import data into: CleanShot 2023-05-25 at 17 14 33

Validation errors are shown, along with the row numbers from the Excel file: CleanShot 2023-05-25 at 17 15 30

dave-mills commented 1 year ago

For reference - here is the upload template. To enable the download from the Projects Crud page, put it inside storage/app/. AE Marker - Project Import Template.xlsx

dan-tang-ssd commented 1 year ago

Thank you Dave.

I performed testing, with some minor comments as below:

  1. geographic_reach seems not imported.

  2. Should we add validation to avoid negative budget?

  3. In data import excel file template, should we indicate which columns are compulsory? We may consider to highlight them in different color.

  4. It would be better if we can indicate row 3 example is just an example. It will not be imported into data platform.

  5. Should we zoom out the excel file a little bit to show all columns at once? Just my personal opinion, I feel more "under control" if no scrolling is required.

dave-mills commented 1 year ago

Thanks Dan,

  1. geographic_reach seems not imported.

Good spot. I've fixed that in the importer.

  1. Should we add validation to avoid negative budget?

Good idea. I've done that in the ProjectRequest.

On your Excel template comments:

What do you think about the attached? AE.Marker.-.Project.Import.Template.xlsx

dan-tang-ssd commented 1 year ago

Thank you Dave.

It is working fine now.

The excel file template looks good, except cell E4 example project budget not showing actual budget value...

image

dave-mills commented 1 year ago

Easy fix. Here's the proper template without my messy test: AE.Marker.-.Project.Import.Template.xlsx