From 8be9bcb95cd4fd8b28c216f3bbf73661cc817b99 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Thu, 7 Nov 2024 20:41:03 +0530 Subject: [PATCH 01/11] refactor: hd team and assignment rules flow --- helpdesk/helpdesk/doctype/hd_team/hd_team.py | 83 ++++++++++++++++++-- 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/helpdesk/helpdesk/doctype/hd_team/hd_team.py b/helpdesk/helpdesk/doctype/hd_team/hd_team.py index 900c80cb1..a40805d70 100644 --- a/helpdesk/helpdesk/doctype/hd_team/hd_team.py +++ b/helpdesk/helpdesk/doctype/hd_team/hd_team.py @@ -2,6 +2,8 @@ # For license information, please see license.txt import frappe +from frappe import _ +from frappe.core.doctype.version.version import get_diff from frappe.exceptions import DoesNotExistError from frappe.model.document import Document from frappe.model.naming import append_number_if_name_exists @@ -22,18 +24,18 @@ def after_rename(self, olddn, newdn, merge=False): rule_doc.assign_condition = f"status == 'Open' and agent_group == '{newdn}'" rule_doc.save(ignore_permissions=True) + def on_update(self): + self.update_support_rotations() + def on_trash(self): # Deletes the assignment rule for this group try: - rule = self.get_assignment_rule() - if rule: - self.assignment_rule = "" - self.save() - frappe.get_doc("Assignment Rule", rule).delete() + frappe.delete_doc("Assignment Rule", self.assignment_rule) except DoesNotExistError: - # TODO: Log this error - pass + frappe.throw( + _("Assignment Rule for HD Team {0} does not exist").format(self.name) + ) def create_assignment_rule(self): """Creates the assignment rule for this group""" @@ -72,3 +74,70 @@ def get_assignment_rule(self): self.create_assignment_rule() return self.assignment_rule + + def update_support_rotations(self): + """ + Update the support rotations for the hd_agent + # If agent removed remove from the support rule of the team + # If agent added add to the support rule of the team + # while adding remove from base Support Rotation + """ + assg_rule_doc = frappe.get_doc("Assignment Rule", self.assignment_rule) + if not assg_rule_doc: + return + + previous_doc = self.get_doc_before_save() + diff = get_diff(previous_doc, self) + if not diff: + return + + if not diff.get("removed") and not diff.get("added"): + return + + for user in diff.get("removed"): + self.update_assignment_rule_users(user, assg_rule_doc, action="remove") + + for user in diff.get("added"): + self.update_assignment_rule_users(user, assg_rule_doc) + + def update_assignment_rule_users(self, user, assignment_rule_doc, action="add"): + _user = user[1].get("user") + if not user: + frappe.throw(_("User Not found")) + return + + if action == "add": + assignment_rule_doc.append("users", {"user": _user}) + if assignment_rule_doc.disabled: + assignment_rule_doc.disabled = False + assignment_rule_doc.save() + + # remove the user from the base assignment rule + base_assignment_rule = frappe.get_value( + "HD Settings", "HD Settings", "base_support_rotation" + ) + base_assignment_rule = frappe.get_doc( + "Assignment Rule", base_assignment_rule + ) + user_id = frappe.get_value( + "Assignment Rule User", + {"user": _user, "parent": base_assignment_rule.name}, + ) + if user_id: + frappe.delete_doc("Assignment Rule User", user_id) + else: + user_id = frappe.get_value( + "Assignment Rule User", + {"user": _user, "parent": assignment_rule_doc.name}, + ) + if not user_id: + return + frappe.delete_doc("Assignment Rule User", user_id) + + # disable the assignment rule if there are no users + total_users_in_assignment_rule = frappe.db.count( + "Assignment Rule User", {"parent": assignment_rule_doc.name} + ) + if total_users_in_assignment_rule == 0: + assignment_rule_doc.disabled = True + assignment_rule_doc.save() From ef3d3e87b61c4820fc97997907ce9018bd764991 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Mon, 11 Nov 2024 16:48:14 +0530 Subject: [PATCH 02/11] fix(patch): add patch for updating HD Team --- helpdesk/helpdesk/doctype/hd_team/hd_team.py | 6 ++---- helpdesk/patches/update_hd_team_users.py | 21 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 helpdesk/patches/update_hd_team_users.py diff --git a/helpdesk/helpdesk/doctype/hd_team/hd_team.py b/helpdesk/helpdesk/doctype/hd_team/hd_team.py index a40805d70..86bcdbd68 100644 --- a/helpdesk/helpdesk/doctype/hd_team/hd_team.py +++ b/helpdesk/helpdesk/doctype/hd_team/hd_team.py @@ -28,7 +28,6 @@ def on_update(self): self.update_support_rotations() def on_trash(self): - # Deletes the assignment rule for this group try: frappe.delete_doc("Assignment Rule", self.assignment_rule) @@ -78,9 +77,8 @@ def get_assignment_rule(self): def update_support_rotations(self): """ Update the support rotations for the hd_agent - # If agent removed remove from the support rule of the team - # If agent added add to the support rule of the team - # while adding remove from base Support Rotation + # If agent removed, remove from the support rule of the team + # If agent added add to the support rule of the team and also, while adding remove from base Support Rotation """ assg_rule_doc = frappe.get_doc("Assignment Rule", self.assignment_rule) if not assg_rule_doc: diff --git a/helpdesk/patches/update_hd_team_users.py b/helpdesk/patches/update_hd_team_users.py new file mode 100644 index 000000000..5d34a1fe9 --- /dev/null +++ b/helpdesk/patches/update_hd_team_users.py @@ -0,0 +1,21 @@ +import frappe + + +def execute(): + teams = frappe.get_all("HD Team", pluck="name") + for team in teams: + existing_agents = frappe.get_all( + "HD Team Item", filters={"team": team}, pluck="parent" + ) + team_users = frappe.get_all( + "HD Team Member", filters={"parent": team}, pluck="user" + ) + + for agent in existing_agents: + if agent not in team_users: + team_doc = ( + frappe.get_doc("HD Team", team) + .append("users", {"user": agent}) + .save() + ) + print("Agent Added") From 3de5672d0d94512164f7ecea96fccc44e30ea7a9 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Mon, 11 Nov 2024 16:49:07 +0530 Subject: [PATCH 03/11] fix: remove groups field from agent doctype --- helpdesk/helpdesk/doctype/hd_agent/hd_agent.json | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/helpdesk/helpdesk/doctype/hd_agent/hd_agent.json b/helpdesk/helpdesk/doctype/hd_agent/hd_agent.json index d5f6d0724..da7fdb69d 100644 --- a/helpdesk/helpdesk/doctype/hd_agent/hd_agent.json +++ b/helpdesk/helpdesk/doctype/hd_agent/hd_agent.json @@ -9,8 +9,7 @@ "user", "agent_name", "user_image", - "is_active", - "groups" + "is_active" ], "fields": [ { @@ -35,12 +34,6 @@ "in_list_view": 1, "label": "Is Active" }, - { - "fieldname": "groups", - "fieldtype": "Table", - "label": "Groups", - "options": "HD Team Item" - }, { "fetch_from": "user.user_image", "fieldname": "user_image", @@ -51,7 +44,7 @@ ], "index_web_pages_for_search": 1, "links": [], - "modified": "2023-07-24 22:31:23.178626", + "modified": "2024-11-11 16:46:38.302701", "modified_by": "Administrator", "module": "Helpdesk", "name": "HD Agent", From 6de7fdbe691539f71a789b8c967018e4034c2e84 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Mon, 11 Nov 2024 16:58:56 +0530 Subject: [PATCH 04/11] fix(patch): consider only active agents --- helpdesk/patches/update_hd_team_users.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/helpdesk/patches/update_hd_team_users.py b/helpdesk/patches/update_hd_team_users.py index 5d34a1fe9..42ed91b06 100644 --- a/helpdesk/patches/update_hd_team_users.py +++ b/helpdesk/patches/update_hd_team_users.py @@ -6,16 +6,21 @@ def execute(): for team in teams: existing_agents = frappe.get_all( "HD Team Item", filters={"team": team}, pluck="parent" - ) + ) # agents in HD Agent doctype team_users = frappe.get_all( "HD Team Member", filters={"parent": team}, pluck="user" - ) + ) # agents in HD Team doctype for agent in existing_agents: - if agent not in team_users: - team_doc = ( - frappe.get_doc("HD Team", team) - .append("users", {"user": agent}) - .save() - ) - print("Agent Added") + is_agent_active = frappe.get_value("HD Agent", agent, "is_active") + if not is_agent_active: + continue + + if is_agent_active: + if agent not in team_users: + team_doc = ( + frappe.get_doc("HD Team", team) + .append("users", {"user": agent}) + .save() + ) + print("Agent Added") From d9d992ef13d7d8e2ea7aac41562c7dffc4d724a7 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Mon, 11 Nov 2024 17:31:07 +0530 Subject: [PATCH 05/11] fix: add new patch path in patch file --- helpdesk/patches.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/helpdesk/patches.txt b/helpdesk/patches.txt index 28ff52d9a..c04b156ed 100644 --- a/helpdesk/patches.txt +++ b/helpdesk/patches.txt @@ -17,3 +17,4 @@ helpdesk.helpdesk.doctype.hd_ticket.patches.replace_overdue_failed helpdesk.patches.create_helpdesk_folder helpdesk.helpdesk.doctype.hd_ticket.patches.feedback_in_master helpdesk.helpdesk.doctype.hd_ticket.patches.first_responded_on +helpdesk.patches.update_hd_team_users From cb252668b300e0090ee1685c108b88dadf3080ad Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Mon, 11 Nov 2024 17:32:00 +0530 Subject: [PATCH 06/11] refactor: remove unnecessary code from hd agent --- .../helpdesk/doctype/hd_agent/hd_agent.py | 150 ------------------ 1 file changed, 150 deletions(-) diff --git a/helpdesk/helpdesk/doctype/hd_agent/hd_agent.py b/helpdesk/helpdesk/doctype/hd_agent/hd_agent.py index 44a9936c0..4ecc6ccd2 100644 --- a/helpdesk/helpdesk/doctype/hd_agent/hd_agent.py +++ b/helpdesk/helpdesk/doctype/hd_agent/hd_agent.py @@ -21,156 +21,6 @@ def set_user_roles(self): user.save() - def on_update(self): - if self.has_value_changed("is_active"): - if not self.is_active: - self.remove_from_support_rotations() - else: - self.add_to_support_rotations() - - if self.has_value_changed("groups") and self.is_active: - previous = self.get_doc_before_save() - if previous: - for group in previous.groups: - if not next( - (g for g in self.groups if g.team == group.team), - None, - ): - self.remove_from_support_rotations(group.team) - - self.add_to_support_rotations() - - def on_trash(self): - self.remove_from_support_rotations() - - def add_to_support_rotations(self, group=None): - """ - Add the hd_agent to the support rotation for the given group or all groups - the hd_agent belongs to if hd_agent already added to the support roatation - for a group, skip - - :param str group: Team name, defaults to None. - """ - rule_docs = [] - if not group: - # Add the hd_agent to the base support rotation - - rule_docs.append( - frappe.get_doc( - "Assignment Rule", - frappe.get_doc("HD Settings").get_base_support_rotation(), - ) - ) - - # Add the hd_agent to the support rotation for each group they belong to - if self.groups: - for group in self.groups: - try: - team_assignment_rule = frappe.get_doc( - "HD Team", group.team - ).get_assignment_rule() - rule_docs.append( - frappe.get_doc( - "Assignment Rule", - team_assignment_rule, - ) - ) - except frappe.DoesNotExistError: - frappe.throw( - frappe._( - "Assignment Rule for HD Team {0} does not exist" - ).format(group.team) - ) - else: - # check if the group is in self.groups - if next( - (group for group in self.groups if group["group_name"] == group), None - ): - rule_docs.append( - frappe.get_doc( - "Assignment Rule", - frappe.get_doc("HD Team", group).get_assignment_rule(), - ) - ) - else: - frappe.throw( - frappe._( - "Agent {0} does not belong to team {1}".format( - self.agent_name, group - ) - ) - ) - - for rule_doc in rule_docs: - skip = False - if rule_doc: - if rule_doc.users and len(rule_doc.users) > 0: - for user in rule_doc.users: - if ( - user.user == self.user - ): # if the user is already in the rule, skip - skip = True - break - if skip: - continue - - user_doc = frappe.get_doc( - {"doctype": "Assignment Rule User", "user": self.user} - ) - rule_doc.append("users", user_doc) - rule_doc.disabled = False # enable the rule if it is disabled - rule_doc.save(ignore_permissions=True) - - def remove_from_support_rotations(self, group=None): - rule_docs = [] - - if group: - # remove the hd_agent from the support rotation for the given group - rule_docs.append( - frappe.get_doc( - "Assignment Rule", - frappe.get_doc("HD Team", group).get_assignment_rule(), - ) - ) - - else: - # Remove the hd_agent from the base support rotation - rule_docs.append( - frappe.get_doc( - "Assignment Rule", - frappe.get_doc("HD Settings").get_base_support_rotation(), - ) - ) - - # Remove the hd_agent from the support rotation for each group they belong to - for group in self.groups: - rule_docs.append( - frappe.get_doc( - "Assignment Rule", - frappe.get_doc("HD Team", group.team).get_assignment_rule(), - ) - ) - - for rule_doc in rule_docs: - if rule_doc.users and len(rule_doc.users) > 0: - for user in rule_doc.users: - if user.user == self.user: - if len(rule_doc.users) == 1: - rule_doc.disabled = ( - True # disable the rule if there are no users left - ) - rule_doc.remove(user) - rule_doc.save() - - def in_group(self, group): - """ - Check if agent is in the given group - """ - if self.groups: - return next((g for g in self.groups if g.team == group), False) - - return False - @frappe.whitelist() def create_hd_agent(first_name, last_name, email, signature, team): From 1f7e76fb404e43a81a9e7c00035e6e8985b62b81 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Mon, 11 Nov 2024 17:32:38 +0530 Subject: [PATCH 07/11] fix: hd_agent doctype should not have track changes enabled --- helpdesk/helpdesk/doctype/hd_agent/hd_agent.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/helpdesk/helpdesk/doctype/hd_agent/hd_agent.json b/helpdesk/helpdesk/doctype/hd_agent/hd_agent.json index da7fdb69d..f3425c6bc 100644 --- a/helpdesk/helpdesk/doctype/hd_agent/hd_agent.json +++ b/helpdesk/helpdesk/doctype/hd_agent/hd_agent.json @@ -44,7 +44,7 @@ ], "index_web_pages_for_search": 1, "links": [], - "modified": "2024-11-11 16:46:38.302701", + "modified": "2024-11-11 17:30:22.859253", "modified_by": "Administrator", "module": "Helpdesk", "name": "HD Agent", @@ -88,6 +88,5 @@ "sort_field": "modified", "sort_order": "DESC", "states": [], - "title_field": "agent_name", - "track_changes": 1 + "title_field": "agent_name" } \ No newline at end of file From 4c5171dcd40d7c07e9103d0dec1b393bd5f300d8 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Wed, 13 Nov 2024 14:03:16 +0530 Subject: [PATCH 08/11] refactor: remove agents if not in team --- .../helpdesk/doctype/hd_ticket/hd_ticket.py | 52 +++++++++++++++---- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/helpdesk/helpdesk/doctype/hd_ticket/hd_ticket.py b/helpdesk/helpdesk/doctype/hd_ticket/hd_ticket.py index da3acb747..985833559 100644 --- a/helpdesk/helpdesk/doctype/hd_ticket/hd_ticket.py +++ b/helpdesk/helpdesk/doctype/hd_ticket/hd_ticket.py @@ -346,14 +346,16 @@ def remove_assignment_if_not_in_team(self): if not self.agent_group or (hasattr(self, "_assign") and not self._assign): return if self.has_value_changed("agent_group") and self.status == "Open": - current_assigned_agent_doc = self.get_assigned_agent() + current_assigned_agent = self.get_assigned_agent() + if not current_assigned_agent: + return + is_agent_in_assigned_team = self.agent_in_assigned_team( + current_assigned_agent, self.agent_group + ) + if ( - current_assigned_agent_doc - and not current_assigned_agent_doc.in_group(self.agent_group) - ) and frappe.get_doc( - "Assignment Rule", - frappe.get_doc("HD Team", self.agent_group).assignment_rule, - ).users: + not is_agent_in_assigned_team + ) and self.users_present_in_team_assignment_rule(): clear_all_assignments("HD Ticket", self.name) frappe.publish_realtime( "helpdesk:update-ticket-assignee", @@ -361,6 +363,39 @@ def remove_assignment_if_not_in_team(self): after_commit=True, ) + def agent_in_assigned_team(self, agent, team): + return frappe.db.exists( + "HD Team Member", + { + "parent": team, + "user": agent, + }, + ) + + def users_present_in_team_assignment_rule(self): + if not self.agent_group: + return False + + assignment_rule = frappe.db.get_value( + "HD Team", self.agent_group, "assignment_rule" + ) + if not assignment_rule: + return False + + is_disabled = frappe.db.get_value( + "Assignment Rule", assignment_rule, "disabled" + ) + if is_disabled: + return False + + users = frappe.get_all( + "Assignment Rule User", filters={"parent": assignment_rule} + ) + if not users: + return False + + return True + @frappe.whitelist() def assign_agent(self, agent): assign({"assign_to": [agent], "doctype": "HD Ticket", "name": self.name}) @@ -386,8 +421,7 @@ def get_assigned_agent(self): # TODO: temporary fix, remove this when only agents can be assigned to ticket exists = frappe.db.exists("HD Agent", assignees[0]) if exists: - agent_doc = frappe.get_doc("HD Agent", assignees[0]) - return agent_doc + return assignees[0] assignees = get_assignees({"doctype": "HD Ticket", "name": self.name}) if len(assignees) > 0: From b669151a0a9c00b0210864d03a9f5cfbce4646bd Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Wed, 13 Nov 2024 15:50:27 +0530 Subject: [PATCH 09/11] refactor: patch --- helpdesk/patches/update_hd_team_users.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/helpdesk/patches/update_hd_team_users.py b/helpdesk/patches/update_hd_team_users.py index 42ed91b06..e162cb065 100644 --- a/helpdesk/patches/update_hd_team_users.py +++ b/helpdesk/patches/update_hd_team_users.py @@ -13,14 +13,10 @@ def execute(): for agent in existing_agents: is_agent_active = frappe.get_value("HD Agent", agent, "is_active") - if not is_agent_active: - continue - - if is_agent_active: - if agent not in team_users: - team_doc = ( - frappe.get_doc("HD Team", team) - .append("users", {"user": agent}) - .save() - ) - print("Agent Added") + if is_agent_active and agent not in team_users: + team_doc = ( + frappe.get_doc("HD Team", team) + .append("users", {"user": agent}) + .save() + ) + print("Agent Added") From fffedda3ad29a710242d7818c9ae852852fcc1ef Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Thu, 14 Nov 2024 00:29:19 +0530 Subject: [PATCH 10/11] fix: remove assignment rule on deletion of team --- helpdesk/helpdesk/doctype/hd_team/hd_team.py | 29 ++++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/helpdesk/helpdesk/doctype/hd_team/hd_team.py b/helpdesk/helpdesk/doctype/hd_team/hd_team.py index 86bcdbd68..7a9d0c40d 100644 --- a/helpdesk/helpdesk/doctype/hd_team/hd_team.py +++ b/helpdesk/helpdesk/doctype/hd_team/hd_team.py @@ -14,8 +14,20 @@ class HDTeam(Document): def rename_self(self, new_name: str): self.rename(new_name) + # nosemgrep: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting def after_insert(self): self.create_assignment_rule() + assignment_rule_doc = frappe.get_doc("Assignment Rule", self.assignment_rule) + + for user in self.users: + _user = user.get("user") + if not _user: + continue + assignment_rule_doc.append("users", {"user": _user}) + + if assignment_rule_doc.disabled and assignment_rule_doc.users: + assignment_rule_doc.disabled = False + assignment_rule_doc.save() def after_rename(self, olddn, newdn, merge=False): # Update the condition for the linked assignment rule @@ -29,11 +41,22 @@ def on_update(self): def on_trash(self): # Deletes the assignment rule for this group + rule = self.assignment_rule + if not rule: + return try: - frappe.delete_doc("Assignment Rule", self.assignment_rule) + frappe.delete_doc( + "Assignment Rule", + rule, + ignore_permissions=True, + force=True, + ignore_on_trash=True, + ) + frappe.db.commit() except DoesNotExistError: - frappe.throw( - _("Assignment Rule for HD Team {0} does not exist").format(self.name) + frappe.log_error( + title="Assignment Rule not found", + message=f"Assignment Rule {rule} not found", ) def create_assignment_rule(self): From 4647ac609b609f2a56a45ac07eee05e3caf610d2 Mon Sep 17 00:00:00 2001 From: RitvikSardana Date: Thu, 14 Nov 2024 00:33:31 +0530 Subject: [PATCH 11/11] fix: semgrep ignore --- helpdesk/helpdesk/doctype/hd_team/hd_team.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helpdesk/helpdesk/doctype/hd_team/hd_team.py b/helpdesk/helpdesk/doctype/hd_team/hd_team.py index 7a9d0c40d..75fb85e7c 100644 --- a/helpdesk/helpdesk/doctype/hd_team/hd_team.py +++ b/helpdesk/helpdesk/doctype/hd_team/hd_team.py @@ -14,7 +14,7 @@ class HDTeam(Document): def rename_self(self, new_name: str): self.rename(new_name) - # nosemgrep: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting + # nosemgrep: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting-other-method def after_insert(self): self.create_assignment_rule() assignment_rule_doc = frappe.get_doc("Assignment Rule", self.assignment_rule)