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

MARXAN-1380-unpublish-public-project #913

Merged
merged 5 commits into from
Mar 23, 2022

Conversation

rubvalave
Copy link
Contributor

-Adds endpoint for project_owners to unpublish a public project.
-Adds conditions to when clearing a project from under moderation to be able to outright unpublish the project straightaway.
-Adds tests for the new endpoint and changes in the workflow.

Depends on #896 being closed beforehand.

@rubvalave rubvalave added the API Everything related the api label Mar 17, 2022
@rubvalave rubvalave requested a review from hotzevzl March 17, 2022 13:57
@rubvalave rubvalave self-assigned this Mar 17, 2022
@vercel
Copy link

vercel bot commented Mar 17, 2022

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/4Y8LRTLXbmqvfWQVPm3VFwkqvqMd
✅ Preview: https://marxan-git-marxan-1380-unpublish-endpoint-pr-c22c48-vizzuality1.vercel.app

marxan-storybook – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/HvuL5FDQyZxJAuRUjDDWRfx2zXQX
✅ Preview: https://marxan-storybook-git-marxan-1380-unpublish-e-9b16b6-vizzuality1.vercel.app

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! IIRC you asked for naming suggestions on a different PR but I ended up suggesting a couple of naming changes here too... feel free to decide.

all the rest looks good!

authenticated users who are not admin, projects under moderation
will be hiding from the listing.
*/
if (!id || !(await this.usersService.isPlatformAdmin(id))) {
Copy link
Member

Choose a reason for hiding this comment

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

while we are revisiting this, let's make the name more explicit

Suggested change
if (!id || !(await this.usersService.isPlatformAdmin(id))) {
if (!userId || !(await this.usersService.isPlatformAdmin(userId))) {

async changeModerationStatus(
id: string,
requestingUserId: string,
status: boolean,
withUnpublish?: boolean,
Copy link
Member

Choose a reason for hiding this comment

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

what about

Suggested change
withUnpublish?: boolean,
alsoUnpublish?: boolean,

@@ -91,6 +96,43 @@ export class PublishProjectController {
return;
}

@Delete(':id/unpublish')
Copy link
Member

Choose a reason for hiding this comment

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

mh, this is quite down to preference, but I suggest to treat this as a kind-of-RPC-thingy-even-if-behind-the-scenes-we-are-deleting-a-resource and use POST instead

Suggested change
@Delete(':id/unpublish')
@Post(':id/unpublish')

@rubvalave rubvalave force-pushed the MARXAN-1380-unpublish-endpoint-project-owners branch from 4990ab8 to 1e76b53 Compare March 22, 2022 18:50
@rubvalave rubvalave closed this Mar 23, 2022
@rubvalave rubvalave reopened this Mar 23, 2022
@rubvalave rubvalave merged commit 419cbf6 into develop Mar 23, 2022
@rubvalave rubvalave deleted the MARXAN-1380-unpublish-endpoint-project-owners branch March 23, 2022 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Everything related the api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants