Code-Poets / sheetstorm

Web working hours sheet for Code-Poets employees
https://www.sheetstorm.codepoets.it
2 stars 0 forks source link

Bugfix managers field in project admin form #472

Closed Szymiks closed 5 years ago

Szymiks commented 5 years ago

Resolves: https://github.com/Code-Poets/sheetstorm/issues/462

Should have done

rwrzesien commented 5 years ago

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 commented 5 years ago

@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

rwrzesien commented 5 years ago

@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 https://github.com/Code-Poets/sheetstorm/pull/472#discussion_r310101727 so it is clear now (that project creator can be removed from managers in update, right?)

Szymiks commented 5 years ago

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.