docs(custom-pu-grid): initial hld #526

kgajowy · 2021-09-07T12:11:46Z

https://vizzuality.atlassian.net/browse/MARXAN-252

Next steps:

confirm HLD is correct
agree on front <-> back communication (ideally: curls/openApi) - see LLD document

vercel · 2021-09-07T12:11:50Z

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-storybook – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/FVf8FPN1mxc4rL9fHwF9qwJwJPCx
✅ Preview: https://marxan-storybook-git-docs-custom-planning-un-fd7d45-vizzuality1.vercel.app

marxan – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan/3fkFF3FU9f3rzxvnpyunMFxzebBQ
✅ Preview: https://marxan-git-docs-custom-planning-unit-geometr-9f79f9-vizzuality1.vercel.app

kgajowy · 2021-09-07T12:13:14Z

@mbarrenechea would you want to declare how frontend team wants to process with uploading / fetching status? (don't mind current setup if you want)

Feel free to commit to LLD.md

aagm · 2021-09-09T07:43:21Z

docs/custom-planning-unit-grid-geometry.hld.md

+* once set, shape cannot be changed or removed from project
+* only `FeatureCollection` with `Polygon` features can be uploaded
+* custom planning unit grid geometry belongs to given project
+* custom planning unit grid geometry has a type of `irregular`


Irregular is not for user-uploaded data, is for another kind of PU grid that will not follow a regular shape (i.e. River basins), and we will provide it in a future phase of the project. (Instead, please provide the options to the user)

@aagm what should then be the type then? if this is a new type, what possible implications it has?

The user should provide the type and the 3 we have should be kept (we should also expand it with from_shapefile: [square, hexagon, irregular, from_shapefile]) and match the enum in the API planning_unit_grid_shape ( [square, hexagon, irregular, from_shapefile]),

What do you think?

added in 99dbd8f

kgajowy · 2021-09-09T10:31:04Z

@mbarrenechea
Regarding LLD (api contract), it could be:

POST projects/:id/grid with similar to other shapefile-uploads arguments (file attachment), returning request ID to track
GET projects/:id/grid/:requestId to get the status

the last one is just a proposition how we could implement it for such cases, however we can keep using GET projects/:id/scenarios/status to acquire all pending asynchronous jobs.

kgajowy · 2021-09-10T07:20:57Z

@mbarrenechea having no response, implemented it with the second option (add to .../scenario/status) for now to keep the old style.

Dyostiq · 2021-09-20T07:35:10Z

docs/custom-planning-unit-grid-geometry.lld.md

+ID returned from request can be ignored. In the future, it may be used to 
+fetch status of the job directly.
+
+# Getting status of the processing


it may get out of sync with the code

aagm

Other than that and ensuring jobs and api events work similar within all processing bits i think it could be merge.

aagm · 2021-10-13T11:17:07Z

docs/custom-planning-unit-grid-geometry.hld.md

+* project with custom planning unit grid geometry cannot use other shape types
+* uploaded shapefile will be processed asynchronously
+* until processed, any action on project/underlying scenarios shouldn't be
+  allowed


Suggested change

allowed

allowed

## Processing requirements after a successful user upload of a Custom planning grid:

* From the boundaries of it, we need to generate a custom planning area.

* When a scenario is created we need to attach the custom grid with the scenario (need to expand current flow).

as a corollary (ish) of the first of the suggested additions here, we also set the bbox for the project - worth making it explicit here, I think

aagm · 2021-10-13T11:20:31Z

docs/custom-planning-unit-grid-geometry.hld.md

+# Key assumptions
+
+* shape can be re-uploaded as long as there are no scenarios created yet
+* only `FeatureCollection` with `Polygon` features can be uploaded


I am very concerned if they end up with irregular shapes that need multipolygons. Also, we need to ensure this feature collection really represents a grid or a surface.

I agree, thats why we have the spike-task for everything related to real validation of the shape being "greed grid"

Suggested change

* only `FeatureCollection` with `Polygon` features can be uploaded

* only `FeatureCollection` with `Polygon` features can be uploaded, and this must be validated on upload

* shapefiles will be rejected if they do not comply with the condition above, and possibly other conditions to be identified before implementation

* conditions that may not be expediently validated should be clearly documented (as part of user documentation)

would the above allow us to move forward?

@hotzevzl not sure if this should really be validated there (would rather pin-point to geoprocessing and it may be heavy)

kgajowy · 2021-10-13T13:44:41Z

@hotzevzl @aagm
9c103e9
please double check what I captured from discussions 🙇

… has scenario

vercel bot deployed to Preview – marxan-storybook September 7, 2021 12:11 View deployment

vercel bot deployed to Preview – marxan September 7, 2021 12:12 View deployment

kgajowy requested review from hotzevzl and mbarrenechea September 7, 2021 12:12

kgajowy requested a review from aagm September 9, 2021 06:26

aagm requested changes Sep 9, 2021

View reviewed changes

kgajowy mentioned this pull request Sep 10, 2021

feat(custom-pu-grid): custom pu grid geometry #525

Merged

vercel bot deployed to Preview – marxan September 10, 2021 08:46 View deployment

vercel bot deployed to Preview – marxan-storybook September 10, 2021 08:46 View deployment

kgajowy requested a review from aagm September 10, 2021 08:46

Dyostiq reviewed Sep 20, 2021

View reviewed changes

vercel bot deployed to Preview – marxan October 13, 2021 05:32 View deployment

vercel bot deployed to Preview – marxan-storybook October 13, 2021 05:32 View deployment

aagm requested changes Oct 13, 2021

View reviewed changes

aagm reviewed Oct 13, 2021

View reviewed changes

hotzevzl approved these changes Oct 13, 2021

View reviewed changes

vercel bot deployed to Preview – marxan-storybook October 13, 2021 13:44 View deployment

vercel bot deployed to Preview – marxan October 13, 2021 13:44 View deployment

kgajowy requested a review from aagm October 13, 2021 13:56

kgajowy added 4 commits October 21, 2021 12:33

docs(custom-pu-grid): initial hld

cfed8c8

docs(custom-pu-grid): change geometry type; add requests to lld

67fc621

docs(custom-pu-grid): ensure that grid cannot be changed once project…

235c33b

… has scenario

docs(custom-pu-grid): add validation and post-processing effects

9b7c731

kgajowy force-pushed the docs/custom-planning-unit-geometry-grid-1 branch from 9c103e9 to 9b7c731 Compare October 21, 2021 10:37

vercel bot deployed to Preview – marxan October 21, 2021 10:37 View deployment

vercel bot deployed to Preview – marxan-storybook October 21, 2021 10:37 View deployment

kgajowy merged commit 1258b88 into develop Oct 21, 2021

kgajowy deleted the docs/custom-planning-unit-geometry-grid-1 branch October 21, 2021 10:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(custom-pu-grid): initial hld #526

docs(custom-pu-grid): initial hld #526

kgajowy commented Sep 7, 2021

vercel bot commented Sep 7, 2021 •

edited

Loading

kgajowy commented Sep 7, 2021 •

edited

Loading

aagm Sep 9, 2021

kgajowy Sep 9, 2021

aagm Sep 10, 2021 •

edited

Loading

kgajowy Sep 10, 2021

kgajowy commented Sep 9, 2021

kgajowy commented Sep 10, 2021

Dyostiq Sep 20, 2021

aagm left a comment

aagm Oct 13, 2021

hotzevzl Oct 13, 2021

aagm Oct 13, 2021

kgajowy Oct 13, 2021

hotzevzl Oct 13, 2021

kgajowy Oct 13, 2021

kgajowy commented Oct 13, 2021

-  allowed
+  allowed
+  ## Processing requirements after a successful user upload of a Custom planning grid:
+  * From the boundaries of it, we need to generate a custom planning area.
+  * When a scenario is created we need to attach the custom grid with the scenario (need to expand current flow).

-* only `FeatureCollection` with `Polygon` features can be uploaded
+* only `FeatureCollection` with `Polygon` features can be uploaded, and this must be validated on upload
+* shapefiles will be rejected if they do not comply with the condition above, and possibly other conditions to be identified before implementation
+* conditions that may not be expediently validated should be clearly documented (as part of user documentation)

docs(custom-pu-grid): initial hld #526

docs(custom-pu-grid): initial hld #526

Conversation

kgajowy commented Sep 7, 2021

vercel bot commented Sep 7, 2021 • edited Loading

marxan-storybook – ./app

marxan – ./app

kgajowy commented Sep 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aagm Sep 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kgajowy commented Sep 9, 2021

kgajowy commented Sep 10, 2021

Choose a reason for hiding this comment

aagm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kgajowy commented Oct 13, 2021

vercel bot commented Sep 7, 2021 •

edited

Loading

kgajowy commented Sep 7, 2021 •

edited

Loading

aagm Sep 10, 2021 •

edited

Loading