Team refactor

ballot/election/doctype/election_team/election_team.json

@@ -26,7 +26,7 @@
  ],
  "index_web_pages_for_search": 1,
  "links": [],
- "modified": "2024-10-14 00:57:30.719301",
+ "modified": "2025-02-26 18:52:20.239819",
  "modified_by": "Administrator",
  "module": "Election",
  "name": "Election Team",
@@ -47,14 +47,7 @@
   },
   {
    "create": 1,
-   "role": "All",
-   "write": 1
-  },
-  {
-   "create": 1,
-   "read": 1,
-   "role": "Election Team Member",
-   "write": 1
+   "role": "All"
   }
  ],
  "show_title_field_in_link": 1,

ballot/election/doctype/election_team/election_team.py

@@ -6,52 +6,106 @@
 
 
 class ElectionTeam(Document):
+    ROLE = "Election Team Member"
+
     def before_insert(self):
-        self.append(
-            "members",
-            {
-                "user": self.owner,
-            },
-        )
+        """Automatically add the creator as a member."""
+        self.append("members", {"user": self.owner})
 
     def before_save(self):
+        """Handle member role assignment and removal."""
         self.add_team_member_role()
-        self.handle_member_removal()
+        self.remove_team_member_role()
+        self.manage_docshare_on_save()
 
-    def add_team_member_role(self):
-        ROLE = "Election Team Member"
+    def after_insert(self):
+        self.manage_docshare_on_insert()
 
+    def add_team_member_role(self):
+        """Ensure all team members have the required role."""
         for member in self.members:
-            # Add the role to the user
             user = frappe.get_doc("User", member.user)
-            existing_roles = {d.role: d for d in user.get("roles")}
-
-            if ROLE in existing_roles:
-                continue
+            user_roles = {role.role for role in user.get("roles")}
 
-            user.append("roles", {"role": ROLE, "doctype": "Has Role"})
-            user.save(ignore_permissions=True)
+            if self.ROLE not in user_roles:
+                user.append("roles", {"role": self.ROLE, "doctype": "Has Role"})
+                user.save(ignore_permissions=True)
 
-    def handle_member_removal(self):
+    def remove_team_member_role(self):
+        """Remove the role from users who are no longer in the team."""
         prev_doc = self.get_doc_before_save()
         if not prev_doc:
             return
 
-        ROLE = "Election Team Member"
+        current_users = {member.user for member in self.members}
 
         for member in prev_doc.members:
-            if member.user not in [m.user for m in self.members]:
-                has_other_teams = frappe.db.exists(
-                    "Election Team Member", {"user": member.user, "parent": ("!=", self.name)}
-                )
-
-                if has_other_teams:
+            if member.user not in current_users:
+                if self.user_in_other_teams(member.user):
                     continue
 
-                # Remove the role from the user
-                user = frappe.get_doc("User", member.user)
-                existing_roles = {d.role: d for d in user.get("roles")}
+                self.remove_role_from_user(member.user)
+
+    def user_in_other_teams(self, user):
+        """Check if the user is part of other Election Teams."""
+        return frappe.db.exists(self.ROLE, {"user": user, "parent": ("!=", self.name)})
+
+    def remove_role_from_user(self, user):
+        """Remove the role from a user if they are not part of any other teams."""
+        user_doc = frappe.get_doc("User", user)
+        role_map = {role.role: role for role in user_doc.get("roles")}
 
-                if ROLE in existing_roles:
-                    user.get("roles").remove(existing_roles[ROLE])
-                    user.save(ignore_permissions=True)
+        if self.ROLE in role_map:
+            user_doc.get("roles").remove(role_map[self.ROLE])
+            user_doc.save(ignore_permissions=True)
+
+    def manage_docshare_on_save(self):
+        """Manage DocShare for team members, adding and removing as needed."""
+        if self.is_new():
+            return
+
+        prev_doc = self.get_doc_before_save()
+        current_users = {member.user for member in self.members}
+        prev_users = {member.user for member in prev_doc.members} if prev_doc else set()
+
+        # Add DocShare for new members
+        new_users = current_users - prev_users
+        for user in new_users:
+            self.add_docshare(user)
+
+        # Remove DocShare for removed members
+        removed_users = prev_users - current_users
+        for user in removed_users:
+            self.remove_docshare(user)
+
+    def manage_docshare_on_insert(self):
+        """Manage DocShare creation for members at time of Team creation"""
+        current_users = {member.user for member in self.members}
+
+        for user in current_users:
+            self.add_docshare(user)
+
+    def add_docshare(self, user):
+        """Create a DocShare entry if it doesn't already exist."""
+        if not frappe.db.exists(
+            "DocShare", {"share_doctype": "Election Team", "share_name": self.name, "user": user}
+        ):
+            docshare = frappe.get_doc(
+                {
+                    "doctype": "DocShare",
+                    "share_doctype": self.doctype,
+                    "share_name": self.name,
+                    "user": user,
+                    "read": 1,
+                    "write": 1,
+                    "share": 1,
+                }
+            )
+            docshare.flags.ignore_share_permission = 1
+            docshare.insert(ignore_permissions=True)
+
+    def remove_docshare(self, user):
+        """Delete the DocShare entry for the removed member."""
+        frappe.db.delete(
+            "DocShare", {"share_doctype": "Election Team", "share_name": self.name, "user": user}
+        )

ballot/election/doctype/election_team/test_election_team.py

@@ -5,43 +5,49 @@
 from faker import Faker
 from frappe.tests import IntegrationTestCase
 
+from ballot.id.roles import TEAM_MEMBER as ROLE
+from ballot.id.doctypes import TEAM
+from ballot.tests.utils import create_election_team
+
 fake = Faker()
 
-ROLE = "Election Team Member"
 TEAM_OWNER = "test1@example.com"
 NEW_MEMBER = "test2@example.com"
 
 
 class TestElectionTeam(IntegrationTestCase):
     def setUp(self):
         frappe.set_user(TEAM_OWNER)
-
-        # Create a new Election Team
-        election_team = frappe.get_doc(
-            {
-                "doctype": "Election Team",
-                "team_name": fake.name(),
-            }
-        )
-        election_team.insert()
-        self.election_team = election_team
+        self.election_team = create_election_team()
         self.team_owner = TEAM_OWNER
-
         frappe.set_user("Administrator")
 
     def tearDown(self):
-        frappe.delete_doc("Election Team", self.election_team.name)
+        frappe.delete_doc(TEAM, self.election_team.name, force=True)
 
     def test_owner_added_to_team(self):
         # Check if the owner is added to the team
         self.assertEqual(self.election_team.members[0].user, self.team_owner)
 
+    def role_exists(self, user: str) -> bool:
+        """Check if the `Election Team Member` role exists for a user."""
+        return bool(frappe.db.exists("Has Role", {"role": ROLE, "parent": user}))
+
+    def docshare_exists(self, team, user: str) -> bool:
+        """Check if the `team` is shared with user"""
+        return bool(
+            frappe.db.exists(
+                "DocShare",
+                {"share_doctype": TEAM, "share_name": team, "user": user},
+            )
+        )
+
     def test_team_member_role_added(self):
         # Given an Election Team Member: self.election_team.members[0]
         # When the Election Team Member is created
         # Then the Election Team Member should have the role "Election Team Member"
 
-        self.assertTrue(frappe.db.exists("Has Role", {"role": ROLE, "parent": self.team_owner}))
+        self.assertTrue(self.role_exists(self.team_owner))
 
         self.election_team.append(
             "members",
@@ -51,15 +57,7 @@ def test_team_member_role_added(self):
         )
         self.election_team.save()
 
-        self.assertTrue(
-            frappe.db.exists(
-                "Has Role",
-                {
-                    "role": ROLE,
-                    "parent": NEW_MEMBER,
-                },
-            )
-        )
+        self.assertTrue(self.role_exists(NEW_MEMBER))
 
     def test_team_member_role_removed(self):
         # Given an Election Team Member: self.election_team.members[0]
@@ -74,30 +72,14 @@ def test_team_member_role_removed(self):
         )
         self.election_team.save()
 
-        self.assertTrue(
-            frappe.db.exists(
-                "Has Role",
-                {
-                    "role": ROLE,
-                    "parent": NEW_MEMBER,
-                },
-            )
-        )
+        self.assertTrue(self.role_exists(NEW_MEMBER))
 
         self.election_team.members = [
             m for m in self.election_team.members if m.user != NEW_MEMBER
         ]
         self.election_team.save()
 
-        self.assertFalse(
-            frappe.db.exists(
-                "Has Role",
-                {
-                    "role": ROLE,
-                    "parent": NEW_MEMBER,
-                },
-            )
-        )
+        self.assertFalse(self.role_exists(NEW_MEMBER))
 
     def test_team_member_role_not_removed(self):
         # Given an Election Team Member: NEW_MEMBER
@@ -110,61 +92,63 @@ def test_team_member_role_not_removed(self):
 
         _election_team = frappe.get_doc(
             {
-                "doctype": "Election Team",
+                "doctype": TEAM,
                 "team_name": fake.name(),
             }
         )
         _election_team.insert()
 
+        # Append New Member to this new team
         _election_team.append(
             "members",
             {
                 "user": NEW_MEMBER,
             },
         )
         _election_team.save()
+        self.assertTrue(self.role_exists(NEW_MEMBER))
 
-        self.assertTrue(
-            frappe.db.exists(
-                "Has Role",
-                {
-                    "role": ROLE,
-                    "parent": NEW_MEMBER,
-                },
-            )
-        )
-
+        # Append new member to the original team
         self.election_team.append(
             "members",
             {
                 "user": NEW_MEMBER,
             },
         )
         self.election_team.save()
+        self.assertTrue(self.role_exists(NEW_MEMBER))
 
-        self.assertTrue(
-            frappe.db.exists(
-                "Has Role",
-                {
-                    "role": ROLE,
-                    "parent": NEW_MEMBER,
-                },
-            )
-        )
-
-        frappe.set_user("Administrator")
-
+        # Remove new member from the new team
         _election_team.members = [m for m in _election_team.members if m.user != NEW_MEMBER]
         _election_team.save()
 
-        self.assertTrue(
-            frappe.db.exists(
-                "Has Role",
-                {
-                    "role": ROLE,
-                    "parent": NEW_MEMBER,
-                },
-            )
-        )
+        # The role should retain
+        self.assertTrue(self.role_exists(NEW_MEMBER))
 
+        frappe.set_user("Administrator")
         _election_team.delete(force=1)
+
+    def test_docshare_created_on_member_addition(self):
+        """Test that a DocShare entry is created when a member is added."""
+        self.assertFalse(self.docshare_exists(self.election_team.name, NEW_MEMBER))
+
+        self.election_team.append("members", {"user": NEW_MEMBER})
+        self.election_team.save()
+
+        self.assertTrue(self.docshare_exists(self.election_team.name, NEW_MEMBER))
+
+    def test_docshare_removed_on_member_removal(self):
+        """Test that the DocShare entry is removed when a member is removed."""
+        # Add the member first
+        self.election_team.append("members", {"user": NEW_MEMBER})
+        self.election_team.save()
+
+        self.assertTrue(self.docshare_exists(self.election_team.name, NEW_MEMBER))
+
+        # Remove the Member
+        self.election_team.members = [
+            m for m in self.election_team.members if m.user != NEW_MEMBER
+        ]
+        self.election_team.save()
+
+        self.assertFalse(self.docshare_exists(self.election_team.name, NEW_MEMBER))

ballot/id/doctypes.py

@@ -0,0 +1,11 @@
+# Constants for all the doctype names for Ballot
+
+ELECTION = "Election"
+TEAM = "Election Team"
+TEAM_MEMBER = "Election Team Member"
+
+CANDIDATE = "Election Candidate"
+VOTE = "Candidate Vote"
+
+NOMINATION_FORM = "Election Nomination Form"
+CANDIDATE_APPLICATION = "Election Candidate Application"

ballot/id/roles.py

@@ -0,0 +1,3 @@
+# Constant for all the roles defined for Ballot
+
+TEAM_MEMBER = "Election Team Member"

ballot/patches.txt

@@ -3,4 +3,5 @@
 # Read docs to understand patches: https://frappeframework.com/docs/v14/user/en/database-migrations
 
 [post_model_sync]
-# Patches added in this section will be executed after doctypes are migrated
+# Patches added in this section will be executed after doctypes are migrated
+ballot.patches.v_0.share_doc_with_team_members