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

fix(Employee): Delete user permission, if create permission is not checked #37548

Closed
wants to merge 1 commit into from

Conversation

niraj2477
Copy link
Contributor

While creating or updating Employee, if Create User Permission is not checked, delete the user permission for that user.
Screenshot 2023-10-17 at 3 29 24 PM

fixes : frappe/hrms#643

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Oct 17, 2023
@ruchamahabal ruchamahabal self-assigned this Nov 1, 2023
Copy link

stale bot commented Nov 17, 2023

This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Nov 17, 2023
@niraj2477
Copy link
Contributor Author

@ruchamahabal Any updates on this?

@stale stale bot removed the inactive label Nov 17, 2023
Copy link

stale bot commented Dec 3, 2023

This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Dec 3, 2023
@niraj2477
Copy link
Contributor Author

@deepeshgarg007 Can you have a look at this PR?

@stale stale bot removed the inactive label Dec 4, 2023
Copy link
Member

@ruchamahabal ruchamahabal left a comment

Choose a reason for hiding this comment

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

I am not sure if this is how it should be fixed. Deleting existing user permission that was not created because of this checkbox should not be implicitly deleted. Its possible people explicitly went and created the user permissions. This will give the employee access to all the records which also seems like an unexpected behaviour.

UX fix for this needs more thought. Closing the PR for now

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create User Permission on Employee should delete user permission when unticked
2 participants