Bugfix add auto assigning manager if he create project#390
Conversation
kbeker
requested review from
MartynaAnnaGottschling,
Szymiks,
dybi,
maciejSamerdak and
rwrzesien
kbeker
changed the title
maciejSamerdak
left a comment
maciejSamerdak
left a comment
There was a problem hiding this comment.
It does it's job alright.
But if not for now, I believe the change in form should ultimately see the daylight. If the user becomes a manager whether he selects himself on list or not, then there's absolutely no reason for him to be on the list at all.
|
|
||
| def form_valid(self, form: ProjectAdminForm) -> HttpRequest: | ||
| if self.request.user not in form.cleaned_data["managers"]: | ||
| assert not self.request.user.is_employee |
There was a problem hiding this comment.
It seems overly cautious to me, since AFAIK we already successfully restrict access for unauthorised personel.
But, you know, personally I don't mind it much, really. 😁
There was a problem hiding this comment.
I gave assert here only for safety-check purposes. It already gave me feed back when I set there assert self.request.user.is_manager and I completely forgot about ADMIN role. So it might be helpful :)
| class ManagerSelectMultiple(SelectMultiple): | ||
| def optgroups(self, name: str, value: List, attrs: Optional[Dict] = None) -> List: | ||
| self.choices.queryset = CustomUser.objects.exclude(user_type=CustomUser.UserType.EMPLOYEE.name) | ||
| return super().optgroups(name, value, attrs) |
There was a problem hiding this comment.
Since the user is assigned either way, the user should be excluded from the field and the field should be marked as optional.
kbeker
force-pushed
the
bugfix-assign-manager-to-project-which-he-create
branch
from
76f4c9c to
91ec78b
Compare
kbeker
force-pushed
the
bugfix-assign-manager-to-project-which-he-create
branch
from
cb6e2fc to
3de7b06
Compare
4 participants
Resolves #118