Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,64 @@ 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.

## Notes on Time Logs

Each **Working Time Log** _Note_ is either **internal** or **external**:

| Type | Format | Used for |
|------|--------|----------|
| **Internal** | plain text | internal documentation only |
| **External** | starts with `+`, at least 3 characters after `+` | customer-facing invoice text |

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 any of the following applies:**

- the row is a break
- a _Task_ is set
- a Jira _Key_ is set

**External note required when:**

- the row has a _Project_ and _Billable_ is greater than `0%`
- no _Task_ and no Jira _Key_

### Decision flow

```mermaid
flowchart TD
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| FAIL[Reject: add Task, Jira Key, or external note]
```

### Examples

| Scenario | Result |
|----------|--------|
| `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 |

## 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 `+`)

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

Want to add pretty time logs to your invoice? Check out our [print formats](https://github.com/alyf-de/erpnext_druckformate).
Expand Down
18 changes: 10 additions & 8 deletions working_time/jira_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@


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):
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()
187 changes: 186 additions & 1 deletion working_time/working_time/doctype/working_time/test_working_time.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@
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,
get_note_content,
parse_note,
)


class TestWorkingTime(unittest.TestCase):
Expand Down Expand Up @@ -84,6 +90,79 @@ 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_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",
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))

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))

log.key = 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")
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(
[
Expand Down Expand Up @@ -187,3 +266,109 @@ 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_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%",
},
]
)
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 = "+xy"
self.assertRaises(frappe.ValidationError, 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()
51 changes: 35 additions & 16 deletions working_time/working_time/doctype/working_time/working_time.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
FIVE_MINUTES = 5 * 60
ONE_HOUR = 60 * 60
WHOLE_DAY_HOURS = 8
MIN_NOTE_LENGTH = 3


class WorkingTime(Document):
Expand Down Expand Up @@ -55,14 +56,9 @@ def validate(self):
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):
frappe.throw(
_("Please add an issue key or invoice note to the billable row {0}").format(log.idx)
_("Please add a task, Jira key, or external note to billable row {0}").format(log.idx)
)

self.validate_working_time_policy()
Expand Down Expand Up @@ -382,18 +378,41 @@ def get_billable_duration(log):
return log.duration * float(log.billable.rstrip("% ")) / 100


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)


def billable_row_missing_invoice_reference(log) -> bool:
if log.billable == "0%" or not log.project or log.is_break:
return False

return not log.task and not log.key and not has_external_note(log.note)


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


def note_has_minimum_content(note: str | None) -> bool:
return len(get_note_content(note)) >= MIN_NOTE_LENGTH


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]:
Expand Down
Loading