From 2d9ff7c6f3916fca4f670b6229c4e81bc229ac99 Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Tue, 23 Jun 2026 19:18:51 +0000 Subject: [PATCH 1/6] feat: ignore non-jira configurations --- working_time/jira_utils.py | 16 +++--- .../doctype/working_time/test_working_time.py | 50 ++++++++++++++++++- .../doctype/working_time/working_time.py | 40 ++++++++++++--- 3 files changed, 92 insertions(+), 14 deletions(-) diff --git a/working_time/jira_utils.py b/working_time/jira_utils.py index 9352ee3..37d89f6 100644 --- a/working_time/jira_utils.py +++ b/working_time/jira_utils.py @@ -6,12 +6,14 @@ def get_jira_issue_url(jira_site, key): def get_description(jira_site, key, note): - if key: + if key and jira_site: description = f"{JiraClient(jira_site).get_issue_summary(key)} ({key})" - if note: - description += f":\n\n{note}" - return description.strip() - elif note: - return note + elif key: + description = key else: - return "-" + description = note or "-" + + if key and note: + description += f":\n\n{note}" + + return description.strip() diff --git a/working_time/working_time/doctype/working_time/test_working_time.py b/working_time/working_time/doctype/working_time/test_working_time.py index 5b1be10..d9a052e 100644 --- a/working_time/working_time/doctype/working_time/test_working_time.py +++ b/working_time/working_time/doctype/working_time/test_working_time.py @@ -6,7 +6,12 @@ import frappe from frappe import _dict -from working_time.working_time.doctype.working_time.working_time import aggregate_time_logs +from working_time.jira_utils import get_description +from working_time.working_time.doctype.working_time.working_time import ( + aggregate_time_logs, + billable_row_missing_invoice_reference, + parse_note, +) class TestWorkingTime(unittest.TestCase): @@ -84,6 +89,49 @@ def test_aggregate_time_logs(self): self.assertEqual(project_b["internal_notes"], []) self.assertEqual(project_b["customer_notes"], ["Customer Note 1"]) + def test_aggregate_time_logs_without_jira_site(self): + logs = [ + _dict( + project="Project A", + duration=3600, + billable="100%", + note="Internal note", + ), + ] + + result = aggregate_time_logs(logs) + + project_a = result[("Project A", None, None)] + self.assertEqual(project_a["customer_notes"], []) + self.assertEqual(project_a["internal_notes"], ["Internal note"]) + + def test_parse_note(self): + customer_note, internal_note = parse_note("internal only") + self.assertIsNone(customer_note) + self.assertEqual(internal_note, "internal only") + + customer_note, internal_note = parse_note("+customer") + self.assertEqual(customer_note, "customer") + self.assertIsNone(internal_note) + + def test_billable_row_missing_invoice_reference(self): + log = _dict(billable="100%", project="Project A", key=None, note="plain note") + self.assertFalse(billable_row_missing_invoice_reference(log, None)) + self.assertTrue(billable_row_missing_invoice_reference(log, "jira.example.com")) + + log.note = "+invoice note" + self.assertFalse(billable_row_missing_invoice_reference(log, "jira.example.com")) + + log.note = None + log.key = "KEY-1" + self.assertFalse(billable_row_missing_invoice_reference(log, "jira.example.com")) + + def test_get_description_without_jira_site(self): + self.assertEqual(get_description(None, "KEY-1", None), "KEY-1") + self.assertEqual(get_description(None, "KEY-1", "extra"), "KEY-1:\n\nextra") + self.assertEqual(get_description(None, None, "note"), "note") + self.assertEqual(get_description(None, None, None), "-") + def test_paid_break_totals(self): working_time = self.get_working_time( [ diff --git a/working_time/working_time/doctype/working_time/working_time.py b/working_time/working_time/doctype/working_time/working_time.py index f243eb6..bedf0d8 100644 --- a/working_time/working_time/doctype/working_time/working_time.py +++ b/working_time/working_time/doctype/working_time/working_time.py @@ -51,16 +51,16 @@ def before_validate(self): self.billable_pct = round(self.billable_time / self.working_time * 100, 0) def validate(self): + billable_projects = { + log.project for log in self.time_logs if log.billable != "0%" and log.project + } + jira_sites = get_jira_sites_for_projects(billable_projects) + for log in self.time_logs: if log.duration and log.duration < 0: frappe.throw(_("Please fix negative duration in row {0}").format(log.idx)) - if ( - log.billable != "0%" - and log.project - and not log.key - and (not log.note or not log.note.strip().startswith("+")) - ): + if billable_row_missing_invoice_reference(log, jira_sites.get(log.project)): frappe.throw( _("Please add an issue key or invoice note to the billable row {0}").format(log.idx) ) @@ -382,6 +382,34 @@ def get_billable_duration(log): return log.duration * float(log.billable.rstrip("% ")) / 100 +def get_jira_sites_for_projects(projects: set[str]) -> dict[str, str | None]: + if not projects: + return {} + + return { + row.name: row.jira_site + for row in frappe.get_all( + "Project", + filters={"name": ("in", list(projects))}, + fields=["name", "jira_site"], + ) + } + + +def billable_row_missing_invoice_reference(log, jira_site: str | None) -> bool: + if log.billable == "0%" or not log.project or log.key: + return False + + note = log.note.strip() if log.note else "" + if not note: + return True + + if jira_site: + return not note.startswith("+") + + return False + + def parse_note(note: str | None) -> tuple[str | None, str | None]: """Parse a note into customer note and internal note.""" customer_note = None From 6d835a0e1ffaedaabde3a766232670025c8b18d8 Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Tue, 23 Jun 2026 19:49:10 +0000 Subject: [PATCH 2/6] feat: added testcase for no key --- .../working_time/doctype/working_time/test_working_time.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/working_time/working_time/doctype/working_time/test_working_time.py b/working_time/working_time/doctype/working_time/test_working_time.py index d9a052e..8e52399 100644 --- a/working_time/working_time/doctype/working_time/test_working_time.py +++ b/working_time/working_time/doctype/working_time/test_working_time.py @@ -126,6 +126,9 @@ def test_billable_row_missing_invoice_reference(self): log.key = "KEY-1" self.assertFalse(billable_row_missing_invoice_reference(log, "jira.example.com")) + log.key = None + self.assertTrue(billable_row_missing_invoice_reference(log, None)) + def test_get_description_without_jira_site(self): self.assertEqual(get_description(None, "KEY-1", None), "KEY-1") self.assertEqual(get_description(None, "KEY-1", "extra"), "KEY-1:\n\nextra") From 505f376ff1e058c815ef374ead42d9469c8ca7d9 Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Tue, 23 Jun 2026 19:52:05 +0000 Subject: [PATCH 3/6] feat: add documentation --- README.md | 13 +++++++++++++ .../doctype/working_time/working_time.py | 4 +--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index ce830e1..1aa49e7 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,19 @@ In a **Working Time Log**, mark _Break_ (`is_break`) for any physical break. Reg German users may refer to [this article](https://www.kanzlei-chevalier.de/blog/dienstreise-als-arbeitszeit) for more information. +## Billable Time Logs + +On submit, every billable **Working Time Log** row (a row with a _Project_ and _Billable_ percentage other than 0%) must include an invoice reference: + +- a Jira issue _Key_, or +- a non-empty _Note_ + +A billable row with neither a _Key_ nor a _Note_ is rejected, including on projects that are not linked to a **Jira Site**. + +For projects linked to a **Jira Site**, a _Note_ alone is only accepted when it starts with `+`. That prefix marks the text as a customer-facing invoice note. Internal-only notes without the `+` prefix are not sufficient for billable rows on Jira projects. + +For projects without a **Jira Site**, any non-empty _Note_ is enough when no _Key_ is set. Notes starting with `+` are treated as customer notes on invoices; other notes are internal. + ## Further Reading Want to add pretty time logs to your invoice? Check out our [print formats](https://github.com/alyf-de/erpnext_druckformate). diff --git a/working_time/working_time/doctype/working_time/working_time.py b/working_time/working_time/doctype/working_time/working_time.py index bedf0d8..5d5f46a 100644 --- a/working_time/working_time/doctype/working_time/working_time.py +++ b/working_time/working_time/doctype/working_time/working_time.py @@ -51,9 +51,7 @@ def before_validate(self): self.billable_pct = round(self.billable_time / self.working_time * 100, 0) def validate(self): - billable_projects = { - log.project for log in self.time_logs if log.billable != "0%" and log.project - } + billable_projects = {log.project for log in self.time_logs if log.billable != "0%" and log.project} jira_sites = get_jira_sites_for_projects(billable_projects) for log in self.time_logs: From 6fc037bcad3ee47ed235a9db7f2c3a3db1c189d5 Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Fri, 26 Jun 2026 11:38:19 +0000 Subject: [PATCH 4/6] feat: updated rules --- README.md | 72 +++++++- .../doctype/working_time/test_working_time.py | 173 +++++++++++++++++- .../doctype/working_time/working_time.py | 73 ++++---- 3 files changed, 271 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 1aa49e7..e1b0e3e 100644 --- a/README.md +++ b/README.md @@ -61,18 +61,76 @@ In a **Working Time Log**, mark _Break_ (`is_break`) for any physical break. Reg German users may refer to [this article](https://www.kanzlei-chevalier.de/blog/dienstreise-als-arbeitszeit) for more information. -## Billable Time Logs +## Notes on Time Logs -On submit, every billable **Working Time Log** row (a row with a _Project_ and _Billable_ percentage other than 0%) must include an invoice reference: +Each **Working Time Log** _Note_ is either **internal** or **external**: -- a Jira issue _Key_, or -- a non-empty _Note_ +| Type | Format | Used for | +|------|--------|----------| +| **Internal** | plain text, at least 3 characters | internal documentation only | +| **External** | starts with `+`, at least 3 characters after `+` | customer-facing invoice text | +| **Too short** | fewer than 3 characters (with or without `+`) | rejected | + +On save, two independent checks may apply to a row. + +### Short rules + +**No note required when:** + +- the row is a break +- the row has no duration +- a _Task_ is set +- a Jira _Key_ is set + +**Internal note required when:** + +- the row is productive (not a break, has duration) +- no _Task_ and no Jira _Key_ +- this applies at any _Billable_ percentage, including `0%` + +**External note required when:** -A billable row with neither a _Key_ nor a _Note_ is rejected, including on projects that are not linked to a **Jira Site**. +- the row has a _Project_ and _Billable_ is greater than `0%` +- no _Task_ and no Jira _Key_ -For projects linked to a **Jira Site**, a _Note_ alone is only accepted when it starts with `+`. That prefix marks the text as a customer-facing invoice note. Internal-only notes without the `+` prefix are not sufficient for billable rows on Jira projects. +An internal note satisfies the general note rule, but **not** the billable rule. For billable rows without _Task_ or _Key_, only an external note (`+...`) works. + +### Decision flow + +```mermaid +flowchart TD + A[Working Time Log row] --> B{Break or no duration?} + B -->|yes| OK[No note required] + B -->|no| C{Task or Jira Key set?} + C -->|yes| OK + C -->|no| D{Billable > 0% and Project set?} + D -->|yes| E{External note +... with 3+ chars?} + E -->|yes| OK + E -->|no| FAIL1[Reject: add Task, Jira Key, or external note] + D -->|no| F{Note with 3+ chars?} + F -->|yes| OK + F -->|no| FAIL2[Reject: add internal or external note] +``` + +### Examples + +| Scenario | Result | +|----------|--------| +| `0%`, no _Task_/_Key_, note = `Fixed bug` | OK - internal note is enough | +| `100%`, no _Task_/_Key_, note = `Fixed bug` | Rejected - internal note does not count for billing | +| `100%`, no _Task_/_Key_, note = `+Fixed bug` | OK - external note satisfies billable rule | +| `100%`, _Task_ set, no note | OK - _Task_ satisfies billable rule | +| any productive row, note = `ab` | Rejected - minimum length is 3 characters | + +## Billable Time Logs + +Billable rows (_Project_ set and _Billable_ greater than `0%`) must include at least one billing reference: + +- a _Task_, or +- a Jira issue _Key_, or +- an external _Note_ (starting with `+`) -For projects without a **Jira Site**, any non-empty _Note_ is enough when no _Key_ is set. Notes starting with `+` are treated as customer notes on invoices; other notes are internal. +Rows with only an internal note are rejected on billable rows. Billable time is rounded to 5-minute increments when **Timesheets** are created on submit. ## Further Reading diff --git a/working_time/working_time/doctype/working_time/test_working_time.py b/working_time/working_time/doctype/working_time/test_working_time.py index 8e52399..63befeb 100644 --- a/working_time/working_time/doctype/working_time/test_working_time.py +++ b/working_time/working_time/doctype/working_time/test_working_time.py @@ -10,7 +10,9 @@ from working_time.working_time.doctype.working_time.working_time import ( aggregate_time_logs, billable_row_missing_invoice_reference, + get_note_content, parse_note, + row_requires_note, ) @@ -114,20 +116,47 @@ def test_parse_note(self): self.assertEqual(customer_note, "customer") self.assertIsNone(internal_note) + def test_get_note_content(self): + self.assertEqual(get_note_content("+ invoice note"), "invoice note") + self.assertEqual(get_note_content("internal note"), "internal note") + self.assertEqual(get_note_content("+ab"), "ab") + self.assertEqual(get_note_content(""), "") + def test_billable_row_missing_invoice_reference(self): - log = _dict(billable="100%", project="Project A", key=None, note="plain note") - self.assertFalse(billable_row_missing_invoice_reference(log, None)) - self.assertTrue(billable_row_missing_invoice_reference(log, "jira.example.com")) + log = _dict( + billable="100%", + project="Project A", + task=None, + key=None, + note=None, + is_break=0, + ) + self.assertTrue(billable_row_missing_invoice_reference(log)) + + log.note = "plain note" + self.assertTrue(billable_row_missing_invoice_reference(log)) log.note = "+invoice note" - self.assertFalse(billable_row_missing_invoice_reference(log, "jira.example.com")) + self.assertFalse(billable_row_missing_invoice_reference(log)) + + log.note = "+ab" + self.assertTrue(billable_row_missing_invoice_reference(log)) log.note = None log.key = "KEY-1" - self.assertFalse(billable_row_missing_invoice_reference(log, "jira.example.com")) + self.assertFalse(billable_row_missing_invoice_reference(log)) log.key = None - self.assertTrue(billable_row_missing_invoice_reference(log, None)) + log.task = "TASK-1" + self.assertFalse(billable_row_missing_invoice_reference(log)) + + log.task = None + log.billable = "0%" + self.assertFalse(billable_row_missing_invoice_reference(log)) + + log.billable = "50%" + log.is_break = 1 + self.assertFalse(billable_row_missing_invoice_reference(log)) def test_get_description_without_jira_site(self): self.assertEqual(get_description(None, "KEY-1", None), "KEY-1") @@ -238,3 +267,135 @@ def test_mandatory_breaks_policy_uses_productive_time(self): missing_break.before_validate() self.assertRaises(frappe.ValidationError, missing_break.validate_mandatory_breaks, policy) + + def test_note_minimum_length(self): + working_time = self.get_working_time( + [ + { + "from_time": "09:00:00", + "to_time": "10:00:00", + "is_break": 0, + }, + ] + ) + working_time.before_validate() + self.assertRaises(frappe.ValidationError, working_time.validate) + + working_time.time_logs[0].note = "+" + self.assertRaises(frappe.ValidationError, working_time.validate) + + working_time.time_logs[0].note = "ab" + self.assertRaises(frappe.ValidationError, working_time.validate) + + working_time.time_logs[0].note = "+xy" + self.assertRaises(frappe.ValidationError, working_time.validate) + + working_time.time_logs[0].note = "abc" + working_time.validate() + + working_time.time_logs[0].note = "+abc" + working_time.validate() + + def test_note_not_required_for_breaks(self): + working_time = self.get_working_time( + [ + { + "from_time": "12:00:00", + "to_time": "12:30:00", + "is_break": 1, + }, + ] + ) + working_time.before_validate() + working_time.validate() + + def test_note_optional_with_task(self): + working_time = self.get_working_time( + [ + { + "from_time": "09:00:00", + "to_time": "10:00:00", + "is_break": 0, + "project": "Project A", + "task": "TASK-1", + "billable": "0%", + }, + ] + ) + working_time.before_validate() + working_time.validate() + + def test_note_optional_with_jira_key(self): + working_time = self.get_working_time( + [ + { + "from_time": "09:00:00", + "to_time": "10:00:00", + "is_break": 0, + "project": "Project A", + "key": "KEY-1", + "billable": "0%", + }, + ] + ) + working_time.before_validate() + working_time.validate() + + def test_billable_row_optional_with_task(self): + working_time = self.get_working_time( + [ + { + "from_time": "09:00:00", + "to_time": "10:00:00", + "is_break": 0, + "project": "Project A", + "task": "TASK-1", + "billable": "100%", + }, + ] + ) + working_time.before_validate() + working_time.validate() + + def test_billable_row_requires_task_jira_key_or_external_note(self): + working_time = self.get_working_time( + [ + { + "from_time": "09:00:00", + "to_time": "10:00:00", + "is_break": 0, + "project": "Project A", + "billable": "100%", + }, + ] + ) + working_time.before_validate() + self.assertRaises(frappe.ValidationError, working_time.validate) + + working_time.time_logs[0].note = "internal note" + self.assertRaises(frappe.ValidationError, working_time.validate) + + working_time.time_logs[0].note = "+invoice note" + working_time.validate() + + def test_note_required_without_task_or_jira_key(self): + working_time = self.get_working_time( + [ + { + "from_time": "09:00:00", + "to_time": "10:00:00", + "is_break": 0, + "project": "Project A", + "billable": "0%", + }, + ] + ) + working_time.before_validate() + self.assertRaises(frappe.ValidationError, working_time.validate) + + def test_row_requires_note(self): + self.assertFalse(row_requires_note(_dict(is_break=0, duration=3600, task="TASK-1", key="KEY-1"))) + self.assertFalse(row_requires_note(_dict(is_break=0, duration=3600, task="TASK-1", key=None))) + self.assertFalse(row_requires_note(_dict(is_break=0, duration=3600, task=None, key="KEY-1"))) + self.assertTrue(row_requires_note(_dict(is_break=0, duration=3600, task=None, key=None))) + self.assertFalse(row_requires_note(_dict(is_break=1, duration=3600, task=None, key=None))) diff --git a/working_time/working_time/doctype/working_time/working_time.py b/working_time/working_time/doctype/working_time/working_time.py index 5d5f46a..1bc74fc 100644 --- a/working_time/working_time/doctype/working_time/working_time.py +++ b/working_time/working_time/doctype/working_time/working_time.py @@ -19,6 +19,7 @@ FIVE_MINUTES = 5 * 60 ONE_HOUR = 60 * 60 WHOLE_DAY_HOURS = 8 +MIN_NOTE_LENGTH = 3 class WorkingTime(Document): @@ -51,16 +52,18 @@ def before_validate(self): self.billable_pct = round(self.billable_time / self.working_time * 100, 0) def validate(self): - billable_projects = {log.project for log in self.time_logs if log.billable != "0%" and log.project} - jira_sites = get_jira_sites_for_projects(billable_projects) - for log in self.time_logs: if log.duration and log.duration < 0: frappe.throw(_("Please fix negative duration in row {0}").format(log.idx)) - if billable_row_missing_invoice_reference(log, jira_sites.get(log.project)): + if billable_row_missing_invoice_reference(log): + frappe.throw( + _("Please add a task, Jira key, or external note to billable row {0}").format(log.idx) + ) + + if row_requires_note(log) and not note_has_minimum_content(log.note): frappe.throw( - _("Please add an issue key or invoice note to the billable row {0}").format(log.idx) + _("Note in row {0} must contain at least {1} characters").format(log.idx, MIN_NOTE_LENGTH) ) self.validate_working_time_policy() @@ -380,46 +383,48 @@ def get_billable_duration(log): return log.duration * float(log.billable.rstrip("% ")) / 100 -def get_jira_sites_for_projects(projects: set[str]) -> dict[str, str | None]: - if not projects: - return {} +def has_external_note(note: str | None) -> bool: + stripped_note = note.strip() if note else "" + return stripped_note.startswith("+") and note_has_minimum_content(note) - return { - row.name: row.jira_site - for row in frappe.get_all( - "Project", - filters={"name": ("in", list(projects))}, - fields=["name", "jira_site"], - ) - } - -def billable_row_missing_invoice_reference(log, jira_site: str | None) -> bool: - if log.billable == "0%" or not log.project or log.key: +def billable_row_missing_invoice_reference(log) -> bool: + if log.billable == "0%" or not log.project or log.is_break: return False - note = log.note.strip() if log.note else "" - if not note: - return True + return not log.task and not log.key and not has_external_note(log.note) + - if jira_site: - return not note.startswith("+") +def get_note_content(note: str | None) -> str: + stripped_note = note.strip() if note else "" + if not stripped_note: + return "" + if stripped_note.startswith("+"): + return stripped_note[1:].strip() + return stripped_note - return False + +def note_has_minimum_content(note: str | None) -> bool: + return len(get_note_content(note)) >= MIN_NOTE_LENGTH + + +def row_requires_note(log) -> bool: + if log.is_break or not log.duration: + return False + + return not log.task and not log.key def parse_note(note: str | None) -> tuple[str | None, str | None]: """Parse a note into customer note and internal note.""" - customer_note = None - internal_note = None - stripped_note = note.strip() if note else None - if stripped_note: - if stripped_note.startswith("+"): - customer_note = stripped_note[1:].strip() - else: - internal_note = stripped_note + content = get_note_content(note) + if not content: + return None, None + + if note and note.strip().startswith("+"): + return content, None - return customer_note, internal_note + return None, content def calculate_hours(log) -> tuple[float, float]: From 4456c983df1c85e6a6f7ce457cc61d912a9d6dcb Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Fri, 26 Jun 2026 12:21:58 +0000 Subject: [PATCH 5/6] fix: jira issue url set none if no site exists --- working_time/jira_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/working_time/jira_utils.py b/working_time/jira_utils.py index 37d89f6..1ebb7b4 100644 --- a/working_time/jira_utils.py +++ b/working_time/jira_utils.py @@ -2,7 +2,7 @@ def get_jira_issue_url(jira_site, key): - return f"https://{jira_site}/browse/{key}" if key else None + return f"https://{jira_site}/browse/{key}" if key and jira_site else None def get_description(jira_site, key, note): From d2997d17455461c55c1814dbf373afc6989221a2 Mon Sep 17 00:00:00 2001 From: 0xD0M1M0 <76812428+0xD0M1M0@users.noreply.github.com> Date: Mon, 29 Jun 2026 08:28:54 +0000 Subject: [PATCH 6/6] fix: remove internal note requirement --- README.md | 27 ++++----------- .../doctype/working_time/test_working_time.py | 33 ++----------------- .../doctype/working_time/working_time.py | 12 ------- 3 files changed, 10 insertions(+), 62 deletions(-) diff --git a/README.md b/README.md index e1b0e3e..fbd2a91 100644 --- a/README.md +++ b/README.md @@ -67,60 +67,47 @@ Each **Working Time Log** _Note_ is either **internal** or **external**: | Type | Format | Used for | |------|--------|----------| -| **Internal** | plain text, at least 3 characters | internal documentation only | +| **Internal** | plain text | internal documentation only | | **External** | starts with `+`, at least 3 characters after `+` | customer-facing invoice text | -| **Too short** | fewer than 3 characters (with or without `+`) | rejected | -On save, two independent checks may apply to a row. +On save, billable rows without a _Task_ or Jira _Key_ must include an external note with at least 3 characters after `+`. ### Short rules -**No note required when:** +**No note required when any of the following applies:** - the row is a break -- the row has no duration - a _Task_ is set - a Jira _Key_ is set -**Internal note required when:** - -- the row is productive (not a break, has duration) -- no _Task_ and no Jira _Key_ -- this applies at any _Billable_ percentage, including `0%` - **External note required when:** - the row has a _Project_ and _Billable_ is greater than `0%` - no _Task_ and no Jira _Key_ -An internal note satisfies the general note rule, but **not** the billable rule. For billable rows without _Task_ or _Key_, only an external note (`+...`) works. - ### Decision flow ```mermaid flowchart TD - A[Working Time Log row] --> B{Break or no duration?} + A[Working Time Log row] --> B{Break?} B -->|yes| OK[No note required] B -->|no| C{Task or Jira Key set?} C -->|yes| OK C -->|no| D{Billable > 0% and Project set?} + D -->|no| OK D -->|yes| E{External note +... with 3+ chars?} E -->|yes| OK - E -->|no| FAIL1[Reject: add Task, Jira Key, or external note] - D -->|no| F{Note with 3+ chars?} - F -->|yes| OK - F -->|no| FAIL2[Reject: add internal or external note] + E -->|no| FAIL[Reject: add Task, Jira Key, or external note] ``` ### Examples | Scenario | Result | |----------|--------| -| `0%`, no _Task_/_Key_, note = `Fixed bug` | OK - internal note is enough | | `100%`, no _Task_/_Key_, note = `Fixed bug` | Rejected - internal note does not count for billing | | `100%`, no _Task_/_Key_, note = `+Fixed bug` | OK - external note satisfies billable rule | +| `100%`, no _Task_/_Key_, note = `+ab` | Rejected - external note needs 3+ characters after `+` | | `100%`, _Task_ set, no note | OK - _Task_ satisfies billable rule | -| any productive row, note = `ab` | Rejected - minimum length is 3 characters | ## Billable Time Logs diff --git a/working_time/working_time/doctype/working_time/test_working_time.py b/working_time/working_time/doctype/working_time/test_working_time.py index 63befeb..c451d25 100644 --- a/working_time/working_time/doctype/working_time/test_working_time.py +++ b/working_time/working_time/doctype/working_time/test_working_time.py @@ -12,7 +12,6 @@ billable_row_missing_invoice_reference, get_note_content, parse_note, - row_requires_note, ) @@ -268,13 +267,15 @@ def test_mandatory_breaks_policy_uses_productive_time(self): self.assertRaises(frappe.ValidationError, missing_break.validate_mandatory_breaks, policy) - def test_note_minimum_length(self): + def test_external_note_minimum_length_on_billable_row(self): working_time = self.get_working_time( [ { "from_time": "09:00:00", "to_time": "10:00:00", "is_break": 0, + "project": "Project A", + "billable": "100%", }, ] ) @@ -284,15 +285,9 @@ def test_note_minimum_length(self): working_time.time_logs[0].note = "+" self.assertRaises(frappe.ValidationError, working_time.validate) - working_time.time_logs[0].note = "ab" - self.assertRaises(frappe.ValidationError, working_time.validate) - working_time.time_logs[0].note = "+xy" self.assertRaises(frappe.ValidationError, working_time.validate) - working_time.time_logs[0].note = "abc" - working_time.validate() - working_time.time_logs[0].note = "+abc" working_time.validate() @@ -377,25 +372,3 @@ def test_billable_row_requires_task_jira_key_or_external_note(self): working_time.time_logs[0].note = "+invoice note" working_time.validate() - - def test_note_required_without_task_or_jira_key(self): - working_time = self.get_working_time( - [ - { - "from_time": "09:00:00", - "to_time": "10:00:00", - "is_break": 0, - "project": "Project A", - "billable": "0%", - }, - ] - ) - working_time.before_validate() - self.assertRaises(frappe.ValidationError, working_time.validate) - - def test_row_requires_note(self): - self.assertFalse(row_requires_note(_dict(is_break=0, duration=3600, task="TASK-1", key="KEY-1"))) - self.assertFalse(row_requires_note(_dict(is_break=0, duration=3600, task="TASK-1", key=None))) - self.assertFalse(row_requires_note(_dict(is_break=0, duration=3600, task=None, key="KEY-1"))) - self.assertTrue(row_requires_note(_dict(is_break=0, duration=3600, task=None, key=None))) - self.assertFalse(row_requires_note(_dict(is_break=1, duration=3600, task=None, key=None))) diff --git a/working_time/working_time/doctype/working_time/working_time.py b/working_time/working_time/doctype/working_time/working_time.py index 1bc74fc..418e2e5 100644 --- a/working_time/working_time/doctype/working_time/working_time.py +++ b/working_time/working_time/doctype/working_time/working_time.py @@ -61,11 +61,6 @@ def validate(self): _("Please add a task, Jira key, or external note to billable row {0}").format(log.idx) ) - if row_requires_note(log) and not note_has_minimum_content(log.note): - frappe.throw( - _("Note in row {0} must contain at least {1} characters").format(log.idx, MIN_NOTE_LENGTH) - ) - self.validate_working_time_policy() def validate_working_time_policy(self): @@ -408,13 +403,6 @@ def note_has_minimum_content(note: str | None) -> bool: return len(get_note_content(note)) >= MIN_NOTE_LENGTH -def row_requires_note(log) -> bool: - if log.is_break or not log.duration: - return False - - return not log.task and not log.key - - def parse_note(note: str | None) -> tuple[str | None, str | None]: """Parse a note into customer note and internal note.""" content = get_note_content(note)