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

feat(api): projects: deny uploading shapefile for non-existing project #218

Merged
merged 7 commits into from
Jun 1, 2021
Merged

feat(api): projects: deny uploading shapefile for non-existing project #218

merged 7 commits into from
Jun 1, 2021

Conversation

kgajowy
Copy link
Contributor

@kgajowy kgajowy commented May 28, 2021 •
edited
Loading

Diff is messed up. Recommended reading: commit via commit

  • 88845bb - moves current project.service to project-crud & creates project-root (later renamed) which will contain more suitable logic (like ACL/resource verification)
  • 96494e9 - check projectId

@vercel
Copy link

vercel bot commented May 28, 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/HRTJNDaqHAYg1ptuHgz3qhjM4raC
✅ Preview: https://marxan-git-api-project-service-responsibilities-vizzuality1.vercel.app

marxan-storybook – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/6TJocNwFKBgZHCc279Nds4usceMN
✅ Preview: https://marxan-storybo-git-api-project-service-responsibilities-151780.vercel.app

@kgajowy kgajowy force-pushed the api/project-service-responsibilities branch from 98080d2 to b1f0603 Compare May 28, 2021 12:11
@kgajowy kgajowy changed the title chore(api): projects: make place for acl/resource logic before crud feat(api): projects: deny uploading shapefile for non-existing project May 28, 2021
@kgajowy kgajowy marked this pull request as ready for review May 28, 2021 13:17
@kgajowy kgajowy requested review from hotzevzl, Dyostiq and alexeh and removed request for hotzevzl May 28, 2021 13:17
hotzevzl
hotzevzl reviewed May 28, 2021
View reviewed changes
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.

thanks @kgajowy - I agree with clearing up responsibilities in our existing crud-ish services (as we go, I wouldn't do a stop-the-world source code garbage collection run to refactor them all at once 😅) - I am mostly not sure about the suggested naming.

api/apps/api/src/modules/projects/dto/geo-feature.mapper.ts Outdated Show resolved Hide resolved
api/apps/api/src/modules/projects/projects-crud.ts Outdated Show resolved Hide resolved
api/apps/api/src/modules/projects/projects-root.service.ts Outdated Show resolved Hide resolved
@kgajowy kgajowy requested a review from hotzevzl May 31, 2021 05:39
@kgajowy kgajowy force-pushed the api/project-service-responsibilities branch from 153e8e2 to 875c655 Compare May 31, 2021 05:59
@kgajowy kgajowy merged commit 54af9ef into develop Jun 1, 2021
@kgajowy kgajowy deleted the api/project-service-responsibilities branch June 1, 2021 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hotzevzl hotzevzl hotzevzl approved these changes

@Dyostiq Dyostiq Awaiting requested review from Dyostiq

@alexeh alexeh Awaiting requested review from alexeh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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