Bugfix managers field in project admin form

[Szymiks]

Resolves: #462

Should have done

• managers field is optional, user of request is always set as manager.
• when we go to update the project view, we could see all managers and if we want to update it, we do not remove the manager who created the project.

[rwrzesien]

This check doesn't make much sense, as this field is in form by its definition. If you really want to check this, make it an assert.

[Szymiks]

But manager is able to update project, too. However they don't have rights to update managers field in project and then we are using for themProjectManagerForm which inheritance from ProjectAdminForm but manager field is excluded in ProjectManagerForm

[rwrzesien]

Then inheritance doesn't make much sense here any more. It should be used if parent class can contain logic from subclasses so you don't have to repeat this logic. Here now those two forms have different logic. So please make ProjectManagerForm do not inherit from ProjectAdminForm and you can then remove this if.

[rwrzesien]

user of request is always set as manager.

Is this implemented?

when we go to update the project view, we could see all managers and if we want to update it, we do not remove the manager who created the project.

I don't see this too.

[Szymiks]

@rwrzesien

user of request is always set as manager.

Is this implemented?

It is implemented in create view by someone else before that PR.

    def form_valid(self, form: ProjectAdminForm) -> HttpRequest:
        if self.request.user not in form.cleaned_data["managers"]:
            assert not self.request.user.is_employee
            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

when we go to update the project view, we could see all managers and if we want to update it, we do not remove the manager who created the project.

I don't see this too.

        if "managers" in self.fields:
            managers_to_choose = CustomUser.objects.exclude(user_type=CustomUser.UserType.EMPLOYEE.name)
            if self.instance.pk:
                self.fields["managers"].queryset = managers_to_choose

It is here, before we was excluding all the time user_pk

[dybi]

Please clarify

[rwrzesien]

@Szymiks

It is implemented in create view by someone else before that PR.

Thanks, I thought you will be adding this in this PR and I didn't saw that code in PR that is why I have asked.

It is here, before we was excluding all the time user_pk.

OK, what I have originally understood from we do not remove the manager who created the project. is that we want to block possibility of removing project creatior from managers, but you have explained it in #472 (comment) so it is clear now (that project creator can be removed from managers in update, right?)

[rwrzesien]

Looks good now, thanks for explanation, some small improvements can be applied.

[Szymiks]

OK, what I have originally understood from we do not remove the manager who created the project. is that we want to block possibility of removing project creatior from managers, but you have explained it in #472 (comment) so it is clear now (that project creator can be removed from managers in update, right?)

@rwrzesien, Yes he can be removed, but before he was allways removed during first updating.