Skip to content

Conversation

@vung-labiso
Copy link

What does this change do?

This PR implements a validation mechanism that prevents the deletion of user roles that are still assigned to active users.
The implementation adds a pre-deletion check to the res.users.role model's unlink() method by searching for any res.users.role.line records that link users to the roles being deleted.
If such records are found, a UserError is raised with a descriptive message indicating which roles cannot be deleted because they are still in use.

Technical Details:

  • Override the unlink() method in the ResUsersRole model to intercept deletion attempts
  • Query the res.users.role.line intermediary table before executing the deletion
  • If role assignments exist, raise a UserError with role names for better user feedback
  • The error message is translatable and includes the names of roles that are currently assigned to users
  • If no role assignments exist, the deletion proceeds normally via super().unlink()

Why is this change being introduced?

Business Context:
Previously, user roles could be deleted even when they were actively assigned to users, leading to inconsistent states and confusion.

Compliance & Stability:

  • Enhances system stability by preventing orphaned role assignments
  • Ensures data consistency by blocking deletion of roles with active user assignments
  • Provides clear, actionable error messages to guide users in the proper deletion workflow
  • Meets OCA contribution guidelines for production-stable modules

Which modules are affected?

  • base_user_role (primary): Core implementation of the deletion validation
    • models/role.py: Added unlink() method override to ResUsersRole class
    • i18n/de.po: Added German translation for the error message

@OCA-git-bot
Copy link
Contributor

Hi @novawish, @jcdrubay, @sebalix,
some modules you are maintaining are being modified, check this out!

@vung-labiso vung-labiso force-pushed the 18.0-extend-oca-base_user_role branch from bac9724 to dcf16d3 Compare December 10, 2025 06:37
- Raises an error when attempting to delete a role that is still assigned to users.
- Provides a user-friendly error message indicating the roles in use, improving data integrity and preventing accidental data loss.
@vung-labiso vung-labiso force-pushed the 18.0-extend-oca-base_user_role branch from dcf16d3 to f2b1370 Compare December 10, 2025 06:48
@vung-labiso vung-labiso changed the title [UPD] Prevents deletion of roles in use [18.0][UPD] base_user_role: prevents deletion of roles in use Dec 10, 2025
@legalsylvain
Copy link
Contributor

Hi. I see the deletion of roles linked to users a valid use case.

@llabusch93
Copy link

@legalsylvain

Let's consider where this use case applies. When we rely on defined roles to control access rights, we have to be careful. Imagine a scenario where another admin—perhaps just having a bit of an off day—accidentally deletes an entire role. Suddenly, access rights for a large group of people are broken. We certainly want to avoid that situation.

To keep things safe, the best practice is to ensure all users are unassigned from a role before it can be deleted. This acts as a safety net. So, rather than seeing this requirement as a problem, we can view it as a helpful improvement that protects our users and keeps the system stable.

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Hi @llabusch93. Thanks for your answer. Are you the guy behind this AI's PR ?

Imagine a scenario where another admin—perhaps just having a bit of an off day—accidentally deletes an entire role.

Well, administrators who have just a bit of an off day should not click on the “settings” button. ;-)

As an administrator, it is possible to uninstall modules, delete views, and nothing will prevent you from doing so.
The consequences of the latter actions are much more serious and irreversible than having users with incorrect settings.

So, rather than seeing this requirement as a problem

Hi. That's not my point. I just say that unlinking a role with users is a valid use case. So it can not be just forbidden. I can understand that for any reason, you don't want to unlink roles with related users, but it can be done in a specific optional extra module.

Proposal : As a first step, you could add a smart button with the number of users on the role form view.

Note: those lines doesn't makes sense with your patch https://github.com/OCA/server-backend/pull/404/files#diff-03b6aef45293bf6e2c5dc2bcf95a52197abec640de7bf4acb804dd662b9de623R117-R119

kind regards,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants