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

Feature/api/marxan 333 vl migrate from proxy controller #256

Merged
merged 21 commits into from
Jun 10, 2021

Conversation

aagm
Copy link
Member

@aagm aagm commented Jun 9, 2021

Overview

  • Moved Vector tile endpoints to their respective modules (Planning units, admin regions, geofeatures, and protected areas)
  • Fixed some issues in how the swagger documentation was generated.
  • Added the possibility to store the swagger JSON spec in the repo so we can link it to Postman collections and Apis.
  • Fixed Issues with PU preview vector tiles that were pulling data from the wrong place
  • Reworked and fixed how bbox filter works
  • Fixed how in admin regions guid filter works based on parent region worked.
  • New properties added to PA Tiles: IUCN categories
  • New id properties provieded for admin regions

Testing instructions

make start-api and inside test, in geoprocessing there is an index.html file that will allow you to test the vector layers
There is one thing that I will want to finish in another pr that is the capability of testing that the mbtile response JSON has produced the response required. But as I said this is for a future pr
Many other things probably can be improved with the wisdom of @hotzevzl, @kgajowy, @alexeh and @Dyostiq but if possible can we leave them for a new pr?

Feature relevant tickets


Checklist before submitting

  • Meaningful commits and code rebased on develop.
  • If this PR adds feature that should be tested for regressions when
    deploying to staging/production, please add brief testing instructions
    to the deploy checklist (docs/deployment-checklist.md)
  • Update CHANGELOG file

@aagm aagm added API Everything related the api Ready to review PR ready to review labels Jun 9, 2021
@aagm aagm self-assigned this Jun 9, 2021
@vercel
Copy link

vercel bot commented Jun 9, 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/CJk3WJF42mJd8WWAzrUw2tscEPJB
✅ Preview: https://marxan-git-feature-apimarxan-333-vl-migrate-a3dd1c-vizzuality1.vercel.app

marxan-storybook – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/Cb2jCXgLBegY52ZhJ71QUsdvkxUx
✅ Preview: https://marxan-storybook-git-feature-apimarxan-333-v-698196-vizzuality1.vercel.app

@aagm aagm requested a review from kgajowy June 9, 2021 14:05
@Dyostiq
Copy link
Contributor

Dyostiq commented Jun 9, 2021

I'd rather have a command to generate swagger.json on demand from the app instead of checking the entire swagger.json in the repo, it's a generated artifact of the code that may get out of sync

api/apps/api/src/main.ts Outdated Show resolved Hide resolved
api/apps/api/src/main.ts Outdated Show resolved Hide resolved
api/apps/api/test/proxy.vector-tiles.e2e-spec.ts Outdated Show resolved Hide resolved
api/package.json Show resolved Hide resolved
api/apps/geoprocessing/src/utils/bbox.utils.ts Outdated Show resolved Hide resolved
api/apps/geoprocessing/src/modules/tile/tile.service.ts Outdated Show resolved Hide resolved
Copy link
Member

@hotzevzl hotzevzl left a comment

Choose a reason for hiding this comment

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

@aagm thanks! with the caveat that I could give this PR only a very quick look and that I didn't run it locally, it overall makes sense to me and I'd be happy for this to be merged to unblock frontend work, and we can then revisit any quirk, if any. Have a look at my inline comments and decide what to do - most are minor anyway.

api/apps/api/src/main.ts Outdated Show resolved Hide resolved
api/apps/geoprocessing/src/modules/tile/tile.service.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Everything related the api Ready to review PR ready to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants