Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deactivated users and team assignments #1781

Closed
j0hannesr0th opened this issue Jun 15, 2020 · 9 comments · Fixed by #1841
Closed

deactivated users and team assignments #1781

j0hannesr0th opened this issue Jun 15, 2020 · 9 comments · Fixed by #1841
Milestone

Comments

@j0hannesr0th
Copy link
Contributor

Describe the bug
I don't know if this is a bug or feature but if you deactivate a user the user is still assigned to all teams - and can't be removed from his assigned teams.

To Reproduce
Steps to reproduce the behavior:

  1. Create a team, add two users
  2. Now deactivate one of the users but not the leader of the team
  3. In overview the deactivated user is still visible as a member of the team but in the detail view of the team it's not possible to unassign him. See the gif at the bottom.

Screenshots
20200615_223545

Additional context

  • Tested on Kimai Cloud
@kevinpapst
Copy link
Member

Nope, that is not a wanted feature.

What happens if you save the team (with the invisible user)? Will it get removed?

@kevinpapst kevinpapst added the bug label Jun 15, 2020
@j0hannesr0th
Copy link
Contributor Author

Yes, after saving the user get's removed.

20200615_225647

@kbancerz
Copy link

kbancerz commented Jul 24, 2020

If I may add my take on the case - I believe, that not only should deactivated users stay assigned to the teams (maybe hidden in UI, but they should not get unassigned), but also it should be possible to assign inactive users to new teams. At this moment this is not possible, and if you try to do it with API, a 400 is returned.

To me having a deactivated user assigned to a team is perfectly reasonable state, and blocking an account IMHO should be treated as an entirely separate action from removing it from a team. It might be useful to have such deactivated users hidden in UI, and shown only on demand.

However, if anything should be reported as a bug, to me, it is the 400 returned when trying to assign an inactive user to a team. At this moment, due to the way assignments are handled in API (you need to provide an entire list of the accounts), trying to manipulate them - even when a single assigned user is inactive - becomes problematic.

Consider this situation:

  1. we have a team with accounts A, B, C assigned
  2. account C get blocked, and it stays assigned to the team

Now, due to the fact, that API requires you to provide the entire list each time, you cannot add or remove any users without removing C from the list. This requires additional logic on the external tool's side, which may be an issue, because it might not be aware of C being deactivated.

@j0hannesr0th
Copy link
Contributor Author

@kbancerz it makes perfect sense for me to unassign the user from all teams. Imagine a user quits his job and leaves the comapny - why should he stay assigned in all teams?

@kbancerz
Copy link

@hmr-it-jr if the user quits his/her job, it might be still advisable to keep information on the previous assignments. I agree, that it may be a good idea to hide them in UI. In that case you lose no information, and you do not clutter teams with inactive accounts.

It all depends on the way you treat deactivation. If you delete assignments, you have to reassign users after you reactivate them - which can easily happen (fixing a mistake, employee returning from a maternal leave etc.).

@kevinpapst
Copy link
Member

Regarding the team assignments when deactivating a user: I will not change the current flow.
Unassigning a user would be magic/unexpected behavior from my perspective.
It doesn't do any harm that they stay in the team, they can't login anyway.

A simple usecase why he should stay in the teams: after the user left the company, his previous teamlead still needs to see his booked times. That would not be possible, if the user is removed from all teams.

We could add another checkbox "remove from all teams" but then you can already unassign them manually via the "Team" tab on the profile.

I do agree though, that the behavior on the Team screens (and the API) should be improved.

  1. The Team screen shows the user in the overview listing, but not in the detail/edit screen.
    Once you update the team/save the team members, the user will be removed, because the user ID wasn't submitted.
    Thats a bug. Solution: The user should be shown when editing the list of team members with a checked-checkbox.

  2. The API issues raised by @kbancerz above should be addressed (add and remove for single users).

@kevinpapst kevinpapst added the API label Jul 24, 2020
@kevinpapst
Copy link
Member

kevinpapst commented Jul 26, 2020

@kbancerz

If I may add my take on the case - I believe, that not only should deactivated users stay assigned to the teams (maybe hidden in UI, but they should not get unassigned), but also it should be possible to assign inactive users to new teams. At this moment this is not possible, and if you try to do it with API, a 400 is returned.

This will be fixed for the next release.

To me having a deactivated user assigned to a team is perfectly reasonable state, and blocking an account IMHO should be treated as an entirely separate action from removing it from a team. It might be useful to have such deactivated users hidden in UI, and shown only on demand.

The users will be shown in the member list UI when editing a team in 1.10. They were hidden previously (see above comment) and therefor accidentally removed from the team after updating it.

Edit for clarification
Disabled users are ONLY shown in the edit screen, if they are already assigned to the team. All other disabled users are still hidden (they could still be assigned to a team via their user profile teams tab).

However, if anything should be reported as a bug, to me, it is the 400 returned when trying to assign an inactive user to a team.

See above, fixed.

At this moment, due to the way assignments are handled in API (you need to provide an entire list of the accounts), trying to manipulate them - even when a single assigned user is inactive - becomes problematic.

Regarding "you need to provide an entire list of the accounts": what do you mean? There are explicit methods for adding and removing one user from a team. Don't use the patch, but one of the POST/DELETE endpoints?!

Bildschirmfoto 2020-07-26 um 12 14 14

Or did I misunderstand you?

Consider this situation:

  1. we have a team with accounts A, B, C assigned
  2. account C get blocked, and it stays assigned to the team

Now, due to the fact, that API requires you to provide the entire list each time, you cannot add or remove any users without removing C from the list. This requires additional logic on the external tool's side, which may be an issue, because it might not be aware of C being deactivated.

Not sure what you mean. Can you explain that? Same misunderstanding then above?

@kevinpapst kevinpapst added this to the 1.10 milestone Jul 26, 2020
@kevinpapst kevinpapst changed the title deactivated users don't get removed from assinged teams deactivated users and team assignments Jul 26, 2020
@kevinpapst kevinpapst mentioned this issue Jul 26, 2020
6 tasks
@kbancerz
Copy link

Or did I misunderstand you?

Honestly, I can't recall why at the moment, but I didn't use /member/ methods in our apps - it was either because they were just added in 1.10, or I totally missed them for some reason, when I was developing first versions of our internal integration engine. Either way, a fix to the way 'users' field is validated in PATCH method is great news.

@github-actions
Copy link

github-actions bot commented Oct 8, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. If you use Kimai on a daily basis, please consider donating to support further development of Kimai.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants