Skip to content

Conversation

Copy link

Copilot AI commented Aug 27, 2025

This PR addresses a critical data consistency issue where deleting a Professor or Student linked to a User would leave the User in an inconsistent state with orphaned roles.

Problem

Currently, when a Professor or Student is deleted, any linked User retains their Professor/Student role even though the corresponding entity no longer exists. This violates the system's constraint that users with Professor or Student roles must be linked to actual Professor/Student records.

For example, a user like this:
User with Professor role

Would be left with the Professor role even after their associated Professor record is deleted, creating an inconsistent state.

Solution

This implementation adds before_destroy callbacks to both Professor and Student models that automatically handle role cleanup when these entities are deleted:

Key Features:

  • Automatic role removal: When a Professor/Student is deleted, the corresponding role is removed from the linked user
  • Graceful degradation: If the user has only the Professor/Student role, they automatically receive the "Desconhecido" (Unknown) role
  • Multi-role support: Users with multiple roles keep their other roles and get their actual_role updated appropriately
  • Edge case handling: No errors occur when deleting Professor/Student records without linked users

Code Changes:

# In Professor model
before_destroy :handle_user_role_removal

private
def handle_user_role_removal
  return unless user.present?
  
  # Remove professor role and handle role reassignment
  professor_role = Role.find_by(id: Role::ROLE_PROFESSOR)
  if professor_role && user.roles.include?(professor_role)
    user.roles.delete(professor_role)
    
    # Assign unknown role if no roles left
    if user.roles.empty?
      unknown_role = Role.find_by(id: Role::ROLE_DESCONHECIDO)
      user.roles << unknown_role if unknown_role
    end
    
    # Update actual_role appropriately
    if user.actual_role == Role::ROLE_PROFESSOR
      user.actual_role = user.user_max_role || Role::ROLE_DESCONHECIDO
    end
    
    user.save!
  end
  
  self.user = nil
end

Test Coverage:

  • ✅ Professor with only Professor role → Gets "Desconhecido" role
  • ✅ Professor with multiple roles → Keeps other roles, updates actual_role
  • ✅ Student with only Student role → Gets "Desconhecido" role
  • ✅ Student with multiple roles → Keeps other roles, updates actual_role
  • ✅ Professor/Student without linked user → Normal deletion, no errors

Benefits

  • Data consistency: Users always have valid role associations
  • Administrative flexibility: Allows safe cleanup of professor/student records
  • No breaking changes: Existing functionality is preserved
  • Backward compatible: Works seamlessly with current user management

Fixes #574.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copy link
Author

Copilot AI commented Aug 27, 2025

@leomurta 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Copilot AI changed the title [WIP] Tratamento de exclusão de aluno e professor ligados ao usuário Implement role cleanup on professor/student deletion to prevent orphaned user roles Aug 27, 2025
Copilot AI requested a review from leomurta August 27, 2025 02:05
Copilot finished work on behalf of leomurta August 27, 2025 02:05
@SAASVUUV
Copy link
Contributor

SAASVUUV commented Aug 28, 2025

Julguei que tinham testes desnecessários para validar a nova função. Além disso, na função handle_user_role_removal haviam validações para verificar se os roles de professor, aluno e desconhecido existiam, o que pra mim não fazia muito sentido, visto que são roles padrão do sistema (roles.rb). Por fim, ao final dessa função, ele limpava o atributo user de aluno/professor, sendo que de qualquer jeito a entidade vai ser destruída, além de que isso não é reproduzido em nenhuma função para :before_save.

Obs. A função em student.rb e professor.rb são semelhantes, mudando somente de aluno para professor, portanto as mudanças foram as mesmas em ambos.

Concordam com as alterações? Fariam mais alguma mudança?

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.

Tratamento de exclusão de aluno e professor ligados ao usuário

3 participants