Give moderators access to edit channels #4608

kontrollanten · 2021-12-06T05:29:39Z

Description

Gives moderators access to edit another users video channel. A moderator won't be able to update an admins channel.

Related issues

closes #4598

Has this been tested?

🙋 yes

Screenshots

closes Chocobozzz#4598

kontrollanten · 2021-12-09T04:35:08Z

@Chocobozzz My second test fails due to

  1) Test video channels
       Should have another acccounts video channel updated:
     Error: expected 200 "OK", got 404 "Not Found"
      at buildRequest (shared/extra-utils/requests/requests.ts:175:35)
      at makeGetRequest (shared/extra-utils/requests/requests.ts:39:10)
      at ChannelsCommand.getRequest (shared/extra-utils/shared/abstract-command.ts:83:26)
      at ChannelsCommand.getRequestBody (shared/extra-utils/shared/abstract-command.ts:57:31)
      at ChannelsCommand.get (shared/extra-utils/videos/channels-command.ts:110:17)
      at Context.<anonymous> (server/tests/api/videos/video-channels.ts:242:42)

It seems like the update doesn't get synced to the second server. Do you know what I'm doing wrong here?

kontrollanten · 2021-12-09T11:14:33Z

Thanks. Do you have any comments on the questions in the PR description?

Chocobozzz · 2021-12-09T12:53:16Z

It seems like the update doesn't get synced to the second server. Do you know what I'm doing wrong here?

You need the complete handle and upload a video in the channel so server 2 knows it. I added the commit that fixes tests.

Should we display this Manage channel button, or should we add a dropdown like the account page?

I think the manage button is okay

Should we send a notification and email to the channel owner? I think that would be a great idea.

IMHO I don't think a notification could be interesting for users. It can be confusing to receive a notification that an admin updated one of your channels. I mean, why an admin updated my channel? For what purpose?
I think that most of the time, as an admin, you'll update channels of users for a specific need that should be communicated to the user (by email for example).
But I may miss a use case, so as an admin why do you need this feature?

In the future I think also moderators should be able to edit channels, but that was a bit tricky to solve so I skipped it for now.

I think you can just replace REMOVE_ANY_VIDEO_CHANNEL by MANAGE_VIDEO_CHANNELS. Why is this tricky?

kontrollanten · 2021-12-09T13:36:42Z

Should we send a notification and email to the channel owner? I think that would be a great idea.

IMHO I don't think a notification could be interesting for users. It can be confusing to receive a notification that an admin updated one of your channels. I mean, why an admin updated my channel? For what purpose? I think that most of the time, as an admin, you'll update channels of users for a specific need that should be communicated to the user (by email for example). But I may miss a use case, so as an admin why do you need this feature?

That's my use case as well. The notification idea came from @andrew712-1 (#4598 (comment)) and I thought it may be good to be transparent. But on the other hand I think as a user you don't expect to get a notification if an admin is changing anything in your channel. Since we don't send notifications for similar admin actions I think we can skip it.

In the future I think also moderators should be able to edit channels, but that was a bit tricky to solve so I skipped it for now.

I think you can just replace REMOVE_ANY_VIDEO_CHANNEL by MANAGE_VIDEO_CHANNELS. Why is this tricky?

I don't remember by heart, I just didn't manage to get the role permissions to work when I tried, but I'll try again.

Since the channel owner isn't necessary the auth user we need to check the right account whether it's the last video or not.

Merge REMOVE_ANY_VIDEO_CHANNEL and MANY_VIDEO_CHANNELS to MANAGE_ANY_VIDEO_CHANNEL.

kontrollanten · 2021-12-09T23:21:43Z

The tricky part was how to handle when a moderator tries to update an admins channel. I found the following code and made same logic here.

PeerTube/server/middlewares/validators/users.ts

Lines 516 to 519 in c4c0c31

    
           return res.fail({ 
        
             status: HttpStatusCode.FORBIDDEN_403, 
        
             message: 'A moderator can only manager users.' 
        
           })

client/src/app/+video-channels/video-channel-edit/video-channel-update.component.ts

client/src/app/+my-library/+my-video-channels/my-video-channels-routing.module.ts

client/src/app/+video-channels/video-channel-edit/video-channel-create.component.ts

client/src/app/+video-channels/video-channels-routing.module.ts

server/middlewares/user-right.ts

Chocobozzz · 2021-12-10T08:37:53Z

server/middlewares/validators/videos/video-channels.ts

@@ -151,31 +149,8 @@ export {

 // ---------------------------------------------------------------------------

-function checkUserCanDeleteVideoChannel (user: MUser, videoChannel: MChannelAccountDefault, res: express.Response) {
-  if (videoChannel.Actor.isOwned() === false) {


I think this if should be used to check all routes except /:nameWithHost/followers

You could put this part in a dedicated middleware, and call it in controllers

Should we really use this check on regular GET routes?

There's middleware called localVideoChannelValidator but it returns a 404 if the video channel isn't found on the local server. It's only used by activity pub routes, should I move that middleware to validators/activitypub/video-channel.ts and then create a new middleware in validators/videos/video-channel.ts?

I think I'll remove localVideoChannelValidator
Thanks!

This reverts commit 2c627c1.

It's not async anymore.

ensureAuthUserOwnsChannelValidator & ensureUserCanManageChannel gets merged into one middleware.

Chocobozzz · 2021-12-13T14:32:39Z

Thanks!

* give admins access to edit all channels closes Chocobozzz#4598 * test(channels): +admin update another users channel * Fix tests * fix(server): delete another users channel Since the channel owner isn't necessary the auth user we need to check the right account whether it's the last video or not. * REMOVE_ANY_VIDEO_CHANNEL > MANAGE_ANY_VIDEO_CHANNEL Merge REMOVE_ANY_VIDEO_CHANNEL and MANY_VIDEO_CHANNELS to MANAGE_ANY_VIDEO_CHANNEL. * user-right: moderator can't manage admins channel * client: MyVideoChannelCreateComponent > VideoChannelCreateComponent * client: MyVideoChannelEdit > VideoChannelEdit * Revert "user-right: moderator can't manage admins channel" This reverts commit 2c627c1. * server: clean dupl validator functionality * fix ensureUserCanManageChannel usage It's not async anymore. * server: merge channel validator middleares ensureAuthUserOwnsChannelValidator & ensureUserCanManageChannel gets merged into one middleware. * client(VideoChannelEdit): redirect to prev route * fix(VideoChannels): handle anon users * client: new routes for create/update channel * Refactor channel validators Co-authored-by: Chocobozzz <me@florianbigard.com>

give admins access to edit all channels

43df00a

closes Chocobozzz#4598

kontrollanten force-pushed the feat-4596-edit-channel branch from 4aca1c7 to 43df00a Compare December 6, 2021 05:43

kontrollanten added 2 commits December 8, 2021 07:46

Merge remote-tracking branch 'origin' into feat-4596-edit-channel

2a8b94b

test(channels): +admin update another users channel

3a8fe5a

Fix tests

952881a

kontrollanten added 3 commits December 9, 2021 23:18

fix(server): delete another users channel

416a7f4

Since the channel owner isn't necessary the auth user we need to check the right account whether it's the last video or not.

REMOVE_ANY_VIDEO_CHANNEL > MANAGE_ANY_VIDEO_CHANNEL

5e7d46e

Merge REMOVE_ANY_VIDEO_CHANNEL and MANY_VIDEO_CHANNELS to MANAGE_ANY_VIDEO_CHANNEL.

user-right: moderator can't manage admins channel

2c627c1

kontrollanten force-pushed the feat-4596-edit-channel branch from 946731e to 2c627c1 Compare December 9, 2021 23:04

kontrollanten changed the title ~~WIP: Give admins access to edit all channels~~ Give admins access to edit all channels Dec 9, 2021

kontrollanten changed the title ~~Give admins access to edit all channels~~ Give moderators access to edit channels Dec 9, 2021

Chocobozzz reviewed Dec 10, 2021

View reviewed changes

kontrollanten added 8 commits December 12, 2021 20:17

client: MyVideoChannelCreateComponent > VideoChannelCreateComponent

dd07af0

client: MyVideoChannelEdit > VideoChannelEdit

638ef08

Revert "user-right: moderator can't manage admins channel"

3b6796a

This reverts commit 2c627c1.

server: clean dupl validator functionality

67a07d9

fix ensureUserCanManageChannel usage

0618c85

It's not async anymore.

server: merge channel validator middleares

683caf7

ensureAuthUserOwnsChannelValidator & ensureUserCanManageChannel gets merged into one middleware.

client(VideoChannelEdit): redirect to prev route

5592bea

fix(VideoChannels): handle anon users

740d1da

kontrollanten force-pushed the feat-4596-edit-channel branch from fc007fb to 1ff6b64 Compare December 12, 2021 21:30

client: new routes for create/update channel

f24a4c1

kontrollanten force-pushed the feat-4596-edit-channel branch from 9c04bd1 to f24a4c1 Compare December 12, 2021 21:50

kontrollanten requested a review from Chocobozzz December 13, 2021 05:11

Refactor channel validators

1317422

Chocobozzz force-pushed the feat-4596-edit-channel branch from 75715a4 to 1317422 Compare December 13, 2021 13:45

Chocobozzz merged commit a37e9e7 into Chocobozzz:develop Dec 13, 2021

kontrollanten deleted the feat-4596-edit-channel branch December 13, 2021 14:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give moderators access to edit channels #4608

Give moderators access to edit channels #4608

kontrollanten commented Dec 6, 2021 •

edited

kontrollanten commented Dec 9, 2021

kontrollanten commented Dec 9, 2021

Chocobozzz commented Dec 9, 2021

kontrollanten commented Dec 9, 2021

kontrollanten commented Dec 9, 2021

Chocobozzz Dec 10, 2021

kontrollanten Dec 12, 2021

kontrollanten Dec 12, 2021

Chocobozzz Dec 13, 2021

Chocobozzz commented Dec 13, 2021 •

edited

Give moderators access to edit channels #4608

Give moderators access to edit channels #4608

Conversation

kontrollanten commented Dec 6, 2021 • edited

Description

Related issues

Has this been tested?

Screenshots

kontrollanten commented Dec 9, 2021

kontrollanten commented Dec 9, 2021

Chocobozzz commented Dec 9, 2021

kontrollanten commented Dec 9, 2021

kontrollanten commented Dec 9, 2021

Chocobozzz Dec 10, 2021

Choose a reason for hiding this comment

kontrollanten Dec 12, 2021

Choose a reason for hiding this comment

kontrollanten Dec 12, 2021

Choose a reason for hiding this comment

Chocobozzz Dec 13, 2021

Choose a reason for hiding this comment

Chocobozzz commented Dec 13, 2021 • edited

kontrollanten commented Dec 6, 2021 •

edited

Chocobozzz commented Dec 13, 2021 •

edited