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.

Sign up for GitHub

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

Jump to bottom

[MARXAN-353] [MARXAN-482] [MARXAN-483] geo features: POST and PUT interface #288

Merged
merged 58 commits into from
Jul 20, 2021
Merged

[MARXAN-353] [MARXAN-482] [MARXAN-483] geo features: POST and PUT interface #288

merged 58 commits into from
Jul 20, 2021

Conversation

hotzevzl
Copy link
Member

@hotzevzl hotzevzl commented Jun 22, 2021 •
edited
Loading

@vercel
Copy link

vercel bot commented Jun 22, 2021 •
edited
Loading

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/7HXvWyLhHDQinC2XdoGwdKaHtJKt
✅ Preview: https://marxan-git-feature-apimarxan-182geo-feature-5dd470-vizzuality1.vercel.app

marxan-storybook – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/E88kicEDfLfmfktFtV74jyUatS5A
✅ Preview: https://marxan-storybook-git-feature-apimarxan-182ge-4e02f3-vizzuality1.vercel.app

@hotzevzl hotzevzl marked this pull request as draft June 22, 2021 18:59
@hotzevzl hotzevzl added API Everything related the api WIP Work in progress labels Jun 22, 2021
hotzevzl
hotzevzl commented Jun 23, 2021
View reviewed changes
api/apps/api/src/modules/scenarios/scenarios.controller.ts
@ApiCreatedResponse({ type: ScenarioResult })
@Post(':id/features/specification')
async createFeatureSetFor(
@Body(new ValidationPipe()) dto: CreateGeoFeatureSetDTO,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dyostiq I've seen you giving advanced suggestions for Swagger documentation - this is a WIP, but your feedback would be appreciated: here and in the following endpoint we use DTOs that themselves use union types with class-transformer discriminators for nested validation, which from what I understand and see don't play well with the kind of introspectioni the Swagger plugin could do... short of not using union types and adding some duplication, would you have suggestions for the Swagger documentation to reflect the full shape of DTOs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, for unions/polymorphic type you may use oneOf (along with getSchemaPath) and discriminator property of ApiProperty

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and ApiExtraModels in order to have the model referenced by getSchemaPath included in the open api doc)

@hotzevzl hotzevzl force-pushed the feature/api/MARXAN-182_geo-feature-recipe-for-scenario-take2 branch from d9c34ee to 7b4cb98 Compare June 28, 2021 12:02
@hotzevzl hotzevzl force-pushed the feature/api/MARXAN-182_geo-feature-recipe-for-scenario-take2 branch from 7b4cb98 to 717ec95 Compare July 5, 2021 14:55
@hotzevzl
update post/put endpoints; add get endpoint
793aed6
@hotzevzl
temporary workaround: use hardcoded values
a0017b8
@hotzevzl
add missing type hint for nested validation
7904059
@hotzevzl
update properties
7d0890e
@hotzevzl
make splits optional in stratification v1 ops
e7c5171
@hotzevzl
add function docs
fd79487
@hotzevzl
extend geofeatures recipe response with geofeatures metadata
8573394
@hotzevzl
extend GET responses for feature set specifications
15710e2
@hotzevzl
return geofeature set specification as object rather than a one-eleme…
497debe 
…nt array
@hotzevzl
keep discriminator property
d1d70c7 
It turns out to be quite handy when processing geofeature
specifications.
@hotzevzl
(wip) persist geofeatures used as plain ones
3657a62
@hotzevzl
(wip) add sync setup for geofeature calculation from specification
4f2d950
@hotzevzl
return empty, draft specification if none has been defined yet
f3176ac
@hotzevzl
(refactor): break down lookup and linkage of properties into simpler …
e335ff8 
…functions
@hotzevzl
look up properties of features directly
666da21
@hotzevzl
(wip): remove call to wip code
5a41e3d
@hotzevzl
add filtering of geofeatures by project bbox
feb4f9f
@hotzevzl
properly filter geofeatures by project bbox
f6d46a6
@hotzevzl
clean up names
ece5a92
@hotzevzl
apply lint fixes
a22ed5a
@hotzevzl
clean up typing
1fcd6de
@hotzevzl
fetch metadata for base features used in stratification/v1 ops
0c272bc
@hotzevzl
split concerns off of single service
7aeeb75
@hotzevzl hotzevzl force-pushed the feature/api/MARXAN-182_geo-feature-recipe-for-scenario-take2 branch from 595a208 to 7aeeb75 Compare July 20, 2021 10:48
@hotzevzl hotzevzl marked this pull request as ready for review July 20, 2021 10:50
@hotzevzl hotzevzl merged commit d262ef4 into develop Jul 20, 2021
@hotzevzl hotzevzl deleted the feature/api/MARXAN-182_geo-feature-recipe-for-scenario-take2 branch July 20, 2021 10:50
@hotzevzl hotzevzl changed the title [MARXAN-482, MARXAN-483] geo features: POST and PUT interface [MARXAN-353] [MARXAN-482] [MARXAN-483] geo features: POST and PUT interface May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dyostiq Dyostiq Dyostiq left review comments

Assignees
No one assigned
Labels
API Everything related the api WIP Work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hotzevzl @Dyostiq