diff --git a/erpnext/projects/doctype/project_update/project_update.py b/erpnext/projects/doctype/project_update/project_update.py index 2153f1b39e1..97d5652eae7 100644 --- a/erpnext/projects/doctype/project_update/project_update.py +++ b/erpnext/projects/doctype/project_update/project_update.py @@ -4,6 +4,7 @@ import frappe from frappe.model.document import Document +from frappe.utils import add_days, today class ProjectUpdate(Document): @@ -31,36 +32,71 @@ class ProjectUpdate(Document): @frappe.whitelist() def daily_reminder(): - project = frappe.db.sql( - """SELECT `tabProject`.project_name,`tabProject`.frequency,`tabProject`.expected_start_date,`tabProject`.expected_end_date,`tabProject`.percent_complete FROM `tabProject`;""" + # This endpoint emails every Project User across every Project, so restrict it to managers. + frappe.only_for("Projects Manager") + + # Same for every project this run, so check once instead of once per project. + holiday_today = frappe.db.exists("Holiday", {"holiday_date": today()}) + + projects = frappe.get_all( + "Project", + fields=[ + "name", + "project_name", + "frequency", + "expected_start_date", + "expected_end_date", + "percent_complete", + ], + limit_page_length=0, # intentionally unbounded: every project is reminded ) - for projects in project: - project_name = projects[0] - frequency = projects[1] - date_start = projects[2] - date_end = projects[3] - progress = projects[4] - draft = frappe.db.sql( - """SELECT count(docstatus) from `tabProject Update` WHERE `tabProject Update`.project = %s AND `tabProject Update`.docstatus = 0;""", - project_name, + for project in projects: + # project.name is the document key (e.g. PROJ-0001); Project Update.project and the + # Project User child rows store it, NOT the project_name display value. + project_id = project.name + frequency = project.frequency + date_start = project.expected_start_date + date_end = project.expected_end_date + progress = project.percent_complete + number_of_drafts = frappe.db.count("Project Update", {"project": project_id, "docstatus": 0}) + # "progress"/"progress_details" are not fields on Project Update (selecting them errored on + # both engines); report the columns that actually exist. + update = frappe.get_all( + "Project Update", + filters={"project": project_id, "date": add_days(today(), -1)}, + fields=["name", "date", "time"], + as_list=True, ) - for drafts in draft: - number_of_drafts = drafts[0] - update = frappe.db.sql( - """SELECT name,date,time,progress,progress_details FROM `tabProject Update` WHERE `tabProject Update`.project = %s AND date = DATE_ADD(CURRENT_DATE, INTERVAL -1 DAY);""", - project_name, + email_sending( + project_id, + project.project_name, + frequency, + date_start, + date_end, + progress, + number_of_drafts, + update, + holiday_today, ) - email_sending(project_name, frequency, date_start, date_end, progress, number_of_drafts, update) -def email_sending(project_name, frequency, date_start, date_end, progress, number_of_drafts, update): - holiday = frappe.db.sql("""SELECT holiday_date FROM `tabHoliday` where holiday_date = CURRENT_DATE;""") +def email_sending( + project_id, + project_name, + frequency, + date_start, + date_end, + progress, + number_of_drafts, + update, + holiday_today, +): msg = ( "

Project Name: " + project_name + "

Frequency: " + " " - + frequency + + str(frequency) + "

Update Reminder:" + " " + str(date_start) @@ -80,7 +116,7 @@ def email_sending(project_name, frequency, date_start, date_end, progress, numbe + "

" ) msg += """

- """ + """ for updates in update: msg += ( "" - + "" ) msg += "
Project IDDate UpdatedTime UpdatedProject StatusNotesProject IDDate UpdatedTime Updated
" @@ -89,18 +125,18 @@ def email_sending(project_name, frequency, date_start, date_end, progress, numbe + str(updates[1]) + "" + str(updates[2]) - + "" - + str(updates[3]) - + "" - + str(updates[4]) + "
" - if len(holiday) == 0: - email = frappe.db.sql("""SELECT user from `tabProject User` WHERE parent = %s;""", project_name) - for emails in email: - frappe.sendmail(recipients=emails, subject=frappe._(project_name + " " + "Summary"), message=msg) + if not holiday_today: + recipients = frappe.get_all( + "Project User", + filters={"parent": project_id}, + pluck="user", + limit_page_length=0, # every project member must be reminded, not just the first page + ) + for user in recipients: + frappe.sendmail(recipients=[user], subject=frappe._(project_name + " " + "Summary"), message=msg) else: pass diff --git a/erpnext/projects/doctype/project_update/test_project_update.py b/erpnext/projects/doctype/project_update/test_project_update.py index 6756965a71a..2f83e26fdfe 100644 --- a/erpnext/projects/doctype/project_update/test_project_update.py +++ b/erpnext/projects/doctype/project_update/test_project_update.py @@ -1,8 +1,67 @@ # Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt +from unittest.mock import patch + +import frappe +from frappe.utils import add_days, today + from erpnext.tests.utils import ERPNextTestSuite class TestProjectUpdate(ERPNextTestSuite): - pass + def test_daily_reminder_runs_and_finds_yesterdays_update(self): + # daily_reminder previously selected non-existent Project Update columns (progress / + # progress_details), raising on both engines. Verify the converted query finds yesterday's + # update and that the whole reminder flow runs without error. + from erpnext.projects.doctype.project.test_project import make_project + from erpnext.projects.doctype.project_update.project_update import daily_reminder + + project = make_project({"project_name": "_Test Project Update Reminder", "company": "_Test Company"}) + project.db_set("frequency", "Daily") + + # Project autonames by naming series, so project.name (PROJ-xxxx) differs from project_name. + # The reminder must filter on project.name, not the display name. + self.assertNotEqual(project.name, project.project_name) + + user = "_test_project_reminder@example.com" + if not frappe.db.exists("User", user): + frappe.get_doc( + {"doctype": "User", "email": user, "first_name": "PR", "send_welcome_email": 0} + ).insert(ignore_permissions=True) + if user not in [u.user for u in project.users]: + # welcome_email_sent=1 so saving doesn't try to send a collaboration invite (no SMTP in tests) + project.append("users", {"user": user, "welcome_email_sent": 1}) + project.save() + + pu = frappe.get_doc( + { + "doctype": "Project Update", + "project": project.name, + "date": add_days(today(), -1), + "time": "10:00:00", + } + ).insert() + self.addCleanup(frappe.delete_doc, "Project Update", pu.name, force=1) + + # The converted update query (no longer referencing progress/progress_details) must find + # yesterday's Project Update, keyed on project.name, on both engines. + updates = frappe.get_all( + "Project Update", + filters={"project": project.name, "date": add_days(today(), -1)}, + fields=["name", "date", "time"], + as_list=True, + ) + self.assertIn(pu.name, [u[0] for u in updates]) + + # Project Users are stored under project.name, not project_name: the reminder must use the + # document key to resolve recipients (the display name matches nothing). + self.assertIn(user, frappe.get_all("Project User", filters={"parent": project.name}, pluck="user")) + self.assertEqual( + frappe.get_all("Project User", filters={"parent": project.project_name}, pluck="user"), [] + ) + + # The full reminder flow runs without error (Project / Project Update / Holiday / Project + # User lookups all execute). sendmail is mocked so no SMTP account is required. + with patch("frappe.sendmail"): + daily_reminder()