Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions managers/forms.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,38 @@
from typing import Any
from typing import Dict
from typing import List
from typing import Optional

from bootstrap_datepicker_plus import DatePickerInput
from django import forms
from django.forms import SelectMultiple

from common.constants import CORRECT_DATE_FORMAT
from managers.models import Project
from users.models import CustomUser


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).exclude(
pk=self.user_pk if hasattr(self, "user_pk") else None
)
return super().optgroups(name, value, attrs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the user is assigned either way, the user should be excluded from the field and the field should be marked as optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done



class ProjectAdminForm(forms.ModelForm):
def __init__(self, *args: Any, **kwargs: Any) -> None:
super().__init__(*args, **kwargs)
if hasattr(self, "user_pk"):
ManagerSelectMultiple.user_pk = self.user_pk

class Meta:
model = Project
fields = "__all__"
widgets = {
"start_date": DatePickerInput(options={"format": CORRECT_DATE_FORMAT}),
"stop_date": DatePickerInput(options={"format": CORRECT_DATE_FORMAT}),
"managers": ManagerSelectMultiple(),
}


Expand Down
7 changes: 7 additions & 0 deletions managers/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ class ProjectCreateView(CreateView):

def get_context_data(self, **kwargs: Any) -> dict:
logger.info(f"User with id: {self.request.user.pk} is in project create view")
ProjectAdminForm.user_pk = self.request.user.pk
context_data = super().get_context_data(**kwargs)
context_data["back_url"] = self.get_success_url()
return context_data
Expand All @@ -102,6 +103,12 @@ def get_success_url(self) -> str: # pylint: disable=no-self-use
return reverse("custom-projects-list")

def form_valid(self, form: ProjectAdminForm) -> HttpRequest:
if self.request.user not in form.cleaned_data["managers"]:
assert not self.request.user.is_employee
Copy link
Collaborator

Choose a reason for hiding this comment

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

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. 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 :)

managers_pk_list = [manager.pk for manager in form.cleaned_data["managers"]]
managers_pk_list.append(self.request.user.pk)
managers_queryset = CustomUser.objects.filter(pk__in=managers_pk_list)
form.cleaned_data["managers"] = managers_queryset
project = form.save()
logger.info(f"New project with id: {project.pk} has been created")
return super(ModelFormMixin, self).form_valid(form) # pylint: disable=bad-super-call
Expand Down