Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cost-surface): consume uploaded file #537

Merged
merged 3 commits into from
Sep 29, 2021

Conversation

kgajowy
Copy link
Contributor

@kgajowy kgajowy commented Sep 27, 2021

There were a few issues:

  • generated template used different fields than API expected when consuming (cost property name)
  • code assumed that all rows exist , thus would only update the available ones. It proved that rows are never inserted. Instead, implemented re-creating all rows based on upload
  • file was never processed by multer thus was not available within tmp/storage

Note: there may be some issue with mapshaper, as it removed some PU while testing...

@vercel
Copy link

vercel bot commented Sep 27, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

marxan – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan/4A2J6K4bKGDeq1o4pjuQsmcJTJ8Z
✅ Preview: https://marxan-git-fix-marxan-755-cost-surface-upload-vizzuality1.vercel.app

marxan-storybook – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/AWV5vwD6DMq4tgDgGuP6s3FaH2W3
✅ Preview: https://marxan-storybook-git-fix-marxan-755-cost-sur-85298c-vizzuality1.vercel.app

@kgajowy
Copy link
Contributor Author

kgajowy commented Sep 27, 2021

Sadly, Extract doesn't like spaces

marxan-geoprocessing  | [Nest] 1153   - 09/27/2021, 8:11:52 AM   [ShapefileService] Error: 8bbeda10-b90c-45cf-a194-1e599cb0e0a9_with cost and spaces in file.zip could not be extracted: Error: ENOENT: no such file or directory, open '"/tmp/storage/8bbeda10-b90c-45cf-a194-1e599cb0e0a9_with cost and spaces in file.zip"' +118272ms

wrapping it in " does not help. Also just "replacing" spaces within filename within multer options causes another issues, so enough for this PR.

@hotzevzl
Copy link
Member

Sadly, Extract doesn't like spaces

marxan-geoprocessing  | [Nest] 1153   - 09/27/2021, 8:11:52 AM   [ShapefileService] Error: 8bbeda10-b90c-45cf-a194-1e599cb0e0a9_with cost and spaces in file.zip could not be extracted: Error: ENOENT: no such file or directory, open '"/tmp/storage/8bbeda10-b90c-45cf-a194-1e599cb0e0a9_with cost and spaces in file.zip"' +118272ms

wrapping it in " does not help. Also just "replacing" spaces within filename within multer options causes another issues, so enough for this PR.

should we reject (with a clear error message) uploads of files that contain spaces, in the meanwhile?

@hotzevzl
Copy link
Member

Note: there may be some issue with mapshaper, as it removed some PU while testing...

Are you sure? From a quick look at the expected/received differences in Jest's output, could it be that only the order of features is different?

Received:

Screenshot 2021-09-27 at 15-00-01 geojson io

Expected:

Screenshot 2021-09-27 at 15-00-20 geojson io

@kgajowy
Copy link
Contributor Author

kgajowy commented Sep 28, 2021

Note: there may be some issue with mapshaper, as it removed some PU while testing...

Are you sure? From a quick look at the expected/received differences in Jest's output, could it be that only the order of features is different?

Received:

Screenshot 2021-09-27 at 15-00-01 geojson io

Expected:

Screenshot 2021-09-27 at 15-00-20 geojson io

the matcher already ignores the order but indeed, only within 'features' (not their 'geometries') and apparently geometry is transformed. Not sure yet if only its pivoted order.

@kgajowy
Copy link
Contributor Author

kgajowy commented Sep 29, 2021

Note: there may be some issue with mapshaper, as it removed some PU while testing...

Are you sure? From a quick look at the expected/received differences in Jest's output, could it be that only the order of features is different?

Received:

Screenshot 2021-09-27 at 15-00-01 geojson io

Expected:

Screenshot 2021-09-27 at 15-00-20 geojson io

image

apparently, coordinates order is also re-ordered.

@kgajowy
Copy link
Contributor Author

kgajowy commented Sep 29, 2021

for (const geo of underlyingGeoJson.features) {
        expect(
          geoJsonFromGeometries.features.some((geoFromJson) =>
            booleanEqual(geoFromJson, geo),
          ),
        ).toBeTruthy();
      }

@hotzevzl the check is quite slow but I see no other way around now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants