We want more than one administrator per Circle

changed milestone to %Projects part I

By Alexandre on 2019-11-05T14:02:30 (imported from GitLab project)

mentioned in issue #296 (closed)

Some proposals from #296 (closed)

Hold Administrator permission on the CircleMember relation
Newbie group is set when there are no other groups
roles property on CircleMember returning concatenated list of user groups and circle-member groups, with their origin (for colouring blue/yellow)

changed milestone to %10

By Alexandre on 2019-11-18T15:17:18 (imported from GitLab project)

assigned to @calummackervoy and unassigned @jbpasquier

mentioned in issue #308 (closed)

Added custom permissions in the latest commit on circle-member-admins of DjangoLDP_Circle

Couple of questions for Monday @jbpasquier

I've written some unit tests in my Startinblox server instance to test Circle permissions, how should I share these?
My test for GET circle returns 200 when the circle status is Private, I can change Circle permissions to reject this or is this supposed to be handled elsewhere?

self.setUpLoggedInUser()

another_user = LDPUser(...)
another_user.save()

self.setUpCircle('Private', another_user)

response = self.client.get('/circles/1/')

self.assertEqual(response.status_code, 404)

How should I test the permissions inheritance scenario you described earlier? I didn't understand the scenario

Guys, careful that we pushed that issue to a later milestone. Let's focus on Projects and Profile directory for now.

By Alexandre on 2019-11-24T21:25:50 (imported from GitLab project)

Issue #296 (closed) is dependent on this (which is for Projects milestone) .. I'll MR what I've done so far (multiple circle-members can be made admins) and we can leave this issue open, referring to the remaining permissions work?

what do you think @jbpasquier ?

Well, I don't really like deliver this as without permissions it's unusable.

Let's leave your MRs open on WIP, as we'll not touch to Circles until that, we should have no conflict on time.

If #296 (closed) is much work, let's postpone it as well. I thought it was a 2h thing.

By Alexandre on 2019-11-26T08:37:47 (imported from GitLab project)

#296 (closed) is working now it just needs CSS changes, but it also depends on the code in this branch (for having multiple admins)

I hear you. Don't keep on going here if it is more than a couple of hours. This is not a priority.

By Alexandre on 2019-11-26T14:03:30 (imported from GitLab project)

I've written some unit tests in my Startinblox server instance to test Circle permissions, how should I share these?

That's a good idea if it's already done. Feel free to see how tests work on djangoldp, you can implement them the same way on the component & @plup will love you.

My test for GET circle returns 200 when the circle status is Private, I can change Circle permissions to reject this or is this supposed to be handled elsewhere?

Yes, you can make the check on too.

How should I test the permissions inheritance scenario you described earlier? I didn't understand the scenario

I think this should be enought: If you put someone as an admin, he may be allowed to delete another member relation. But if it's not, he can only delete himself.

To make sure you go on the right way:

You need a custom permission class on Circles: It'll check if the user of the request is an admin of the circle or not (beware for circle creation) and return right permissions.
You need another custom permission class on CircleMember: It'll check if the user of the request is an admin of the Circle linked to, in fact after that it'll do the same as above.

Thanks for these, got a bunch more tests passing :) to clarify:

What should the permissions on Circles with status "Archived" be?
I should be able to POST a circle if I am authenticated, and only PATCH, DELETE, PUT circles which I am an admin of?
I should be able to remove myself from a circle if I am authenticated, and only add a new member or remove someone else if I am an admin of the circle?

What should the permissions on Circles with status "Archived" be?

Archive mean an hidden circle from left menu, need to be unarchived from admin before use it again. From permission view, I don't think it change anything from Private/Public.

But a good question here is, if my Circle was Private does it become Public+Archived? Or are every Archived Circles == Private but without the left menu? @alexandre ?

I should be able to POST a circle if I am authenticated, and only PATCH, DELETE, PUT circles which I am an admin of?

Yes

POST: auth only
PATCH/PUT/DELETE: admins

I should be able to remove myself from a circle if I am authenticated, and only add a new member or remove someone else if I am an admin of the circle?

No

Add anyone: Any member of, admin or not
Remove myself: Any member of. Can't if I'm last member.
Remove others: Any admins. As it's not really useful on public circle, it's primary for private.
Remove admins: Nobody, only myself. Can't if I'm the last admin.

nearly there I've got tests passing for all conditions except:

Add anyone: Any member of, admin or not

This is difficult because the front-end makes this request by doing PATCH circle, not POST circle-member, and the permissions for PATCH circle block non-admins. I think the only way around this is to analyse the request data, making sure:

it only alters the members field
the new members value includes all of the previous members

This seems hard, unless I'm missing a better solution?

Possibly we could change the requirements to only allow admins to add new users, or the front-end could POST to CircleMembers instead?

This is difficult because the front-end makes this request by doing PATCH circle, not POST circle-member, and the permissions for PATCH circle block non-admins. I think the only way around this is to analyse the request data, making sure:

I'm pretty sure we made a POST on /circles/X/members/ here because of that, could you check?

I think we can workaround like that:

On your first permission class, on PATCH on a Circle: Only admins of are allowed.

On the second one, on POST on a Circle->CircleMember: Any member of can.

mentioned in issue #299 (closed)

@jbpasquier, @calummackervoy : I suggest we forget about archiving for now. Let's postpone this. I created https://git.happy-dev.fr/startinblox/applications/sib-app/issues/313 so we don't forget about it.

By Alexandre on 2019-11-28T14:23:59 (imported from GitLab project)

@sylvain: You might be able to help here.

By Alexandre on 2019-11-28T14:27:28 (imported from GitLab project)

Hi @jbpasquier, Matthieu explained to me that it does POST to /circles/X/members/, because of the nested-field property (even though the data-src is /circle/X/). Sorry for the confusion! The good news is that this means it's ready for an MR (https://git.happy-dev.fr/startinblox/djangoldp-packages/djangoldp-circle/merge_requests/13)

mentioned in issue #314 (closed)

@calummackervoy djangoldp-circle part is on live

closed

changed milestone to %Circles part I

We want more than one administrator per Circle

Designs

Child items 0

Activity