Skip to content
Open
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
27 changes: 25 additions & 2 deletions src/woffu_client/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,34 @@ def main() -> None:
)

# ---- get_status ----
subparsers.add_parser(
st_parser = subparsers.add_parser(
"get-status",
help="Get current status and current day's \
total amount of worked hours",
)
st_period = st_parser.add_mutually_exclusive_group()
st_period.add_argument(
"--week",
dest="period",
action="store_const",
const="week",
help="Get current week's total hours",
)
st_period.add_argument(
"--month",
dest="period",
action="store_const",
const="month",
help="Get current month's total hours",
)
st_period.add_argument(
"--year",
dest="period",
action="store_const",
const="year",
help="Get current year's total hours",
)
st_parser.set_defaults(period="today")
Copy link
Copy Markdown
Owner

@ProtossGP32 ProtossGP32 Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to also define a subparser argument for --today even though it's the default value, so users using some kind of automated script to invoke woffu-cli can easily swap between any of the options and not worry about having to leave it empty. What do you think?


# ---- sign ----
sign_parser = subparsers.add_parser(
Expand Down Expand Up @@ -144,7 +167,7 @@ def main() -> None:
sys.exit(1)
case "get-status":
try:
client.get_status()
client.get_status(period=args.period)
except Exception as e:
print(f"❌ Error retrieving status: {e}", file=sys.stderr)
case "sign":
Expand Down
46 changes: 44 additions & 2 deletions src/woffu_client/woffu_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""
from __future__ import annotations

import calendar
Copy link
Copy Markdown
Owner

@ProtossGP32 ProtossGP32 Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calendar package is deprecated (https://pypi.org/project/Calendar/). Please use any alternative actively maintained to avoid technical debt in the future.

Also, when importing new modules, please add them to the corresponding pyproject.toml section.

import csv
import json
import logging
Expand All @@ -15,6 +16,7 @@
from getpass import getpass
from operator import itemgetter
from pathlib import Path
from typing import Optional

from tzlocal import get_localzone

Expand Down Expand Up @@ -417,9 +419,14 @@ def get_sign_requests(self, date: str) -> dict | list:
return {}

def get_status(
self, only_running_clock: bool = False,
) -> tuple[timedelta, bool]:
self, only_running_clock: bool = False, period: str = "today",
) -> tuple[timedelta, Optional[bool]]:
"""Return the total amount of worked hours and current sign status."""
# Retrieve info for the period flags, e.g., --week, --month, --year
if period != "today":
return self._get_status_period(period), None
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_status returns a tuple[timedelta, bool] but you're returning tuple[timedelta, None]. Pylance is complaining about this, so we should fix this. Maybe we should invoke a new method that only returns self._get_status_period(period) if you only want the total time... Or make the bool return value optional.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather use Optional[bool] to avoid dispersing subcommands and their flags. Ideally, we would execute one function from cli.py that represents a subcommand and pass any flags as arguments.

Another option is to return the sign-in status also for each variation of get-status, which would imply 2 REST API calls for period flags (e.g., --week, --month, --year).

What do you think?

Copy link
Copy Markdown
Owner

@ProtossGP32 ProtossGP32 Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with the Optional[bool] option for now. I'll let you know if Pylance complains about any other thing then (it hates types inconsistencies during unit tests).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, the get-status command in woffu-cli ends with error when defining different time windows due to the status not being available:

(.venv) mpalacin@bsc-84885021:~/GitHub/personal_projects/woffu-client$ woffu-cli get-status
[2025-10-17 10:04:53] INFO woffu_api_client: Hours worked today: 00:42:22
[2025-10-17 10:04:53] INFO woffu_api_client: You're currently signed in.
(.venv) mpalacin@bsc-84885021:~/GitHub/personal_projects/woffu-client$ woffu-cli get-status --week
[2025-10-17 10:05:02] INFO woffu_api_client: Hours worked this week: 14:25
❌ Error retrieving status: not enough values to unpack (expected 2, got 1)

Please try to secure this.


# Get current sign-in status and worked hours for today
signs_in_day = self.get(url=f"{self._woffu_api_url}/api/signs").json()

# Initialize a timer and the running clock boolean
Expand Down Expand Up @@ -487,6 +494,41 @@ def get_status(
)
return total_time, running_clock

def _get_status_period(self, period: str) -> timedelta:
today = datetime.now().date()
match period:
case "week":
from_date = today - timedelta(days=today.weekday())
to_date = from_date + timedelta(days=6)
case "month":
from_date = today.replace(day=1)
last_day = calendar.monthrange(today.year, today.month)[1]
to_date = today.replace(day=last_day)
case "year":
from_date = today.replace(month=1, day=1)
to_date = today.replace(month=12, day=31)

diary_json = self.get(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you're calling the same HTTP endpoint as in _get_presence() private method. Maybe we can create a base method for presence retrieving to avoid duplicated code? And then this new method can be used for both the current _get_presence() and the new _get_status_period().

url=f"https://{self._domain}/api/svc/core/diariesquery/users/\
{self._user_id}/diaries/summary/presence",
params={
"fromDate": from_date,
"toDate": to_date,
"showHidden": False,
},
).json()

h, m = map(int, diary_json["totalWorkedTimeFormatted"]["values"])
logger.info(
"Hours worked this {}: {:02d}:{:02d}".format(period, h, m),
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency's sake with the default results, we should use the same time format, including hours, minutes and seconds. This way we ease the job of any external parser that might want to extract this value:

"Hours worked today: {:02d}:{:02d}:{:02d}".format(
                int(hours), int(minutes), int(seconds),
            ),

)
total_time = timedelta(hours=h, minutes=m)

h, m = diary_json["totalWorkingTimeFormatted"]["values"]
logger.info(f"Hours scheduled for this {period}: {h}:{m}")

return total_time

def sign(self, type: str = "") -> HTTPResponse | None:
"""
Sign in/out on Woffu.
Expand Down
56 changes: 49 additions & 7 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,65 @@ def test_download_all_documents_failure(self, mock_client_cls):
self.assertIn("❌ Error downloading files: Boom!", error_output)

@patch("src.woffu_client.cli.WoffuAPIClient")
def test_get_status_success(self, mock_client_cls):
"""Test status retrieval success."""
def test_get_status_today(self, mock_client_cls):
"""Test get-status without a flag (today)."""
self._test_get_status(mock_client_cls, None)

@patch("src.woffu_client.cli.WoffuAPIClient")
def test_get_status_week(self, mock_client_cls):
"""Test get-status with --week flag."""
self._test_get_status(mock_client_cls, "--week")

@patch("src.woffu_client.cli.WoffuAPIClient")
def test_get_status_month(self, mock_client_cls):
"""Test get-status with --month flag."""
self._test_get_status(mock_client_cls, "--month")

@patch("src.woffu_client.cli.WoffuAPIClient")
def test_get_status_year(self, mock_client_cls):
"""Test get-status with --year flag."""
self._test_get_status(mock_client_cls, "--year")

def _test_get_status(self, mock_client_cls, flag):
mock_client = mock_client_cls.return_value
mock_client.get_status.return_value = None

with patch.object(sys, "argv", ["cli", "get-status"]):
argv = ["cli", "get-status"]
if flag:
argv.append(flag)
with patch.object(sys, "argv", argv):
cli.main()

mock_client.get_status.assert_called_once()

@patch("src.woffu_client.cli.WoffuAPIClient")
def test_get_status_failure(self, mock_client_cls):
"""Test status retrieval failure."""
def test_get_status_failure_today(self, mock_client_cls):
"""Test failure for get-status without a flag (today)."""
self._test_get_status_failure(mock_client_cls, None)

@patch("src.woffu_client.cli.WoffuAPIClient")
def test_get_status_failure_week(self, mock_client_cls):
"""Test failure for get-status with --week flag."""
self._test_get_status_failure(mock_client_cls, "--week")

@patch("src.woffu_client.cli.WoffuAPIClient")
def test_get_status_failure_month(self, mock_client_cls):
"""Test failure for get-status with --month flag."""
self._test_get_status_failure(mock_client_cls, "--month")

@patch("src.woffu_client.cli.WoffuAPIClient")
def test_get_status_failure_year(self, mock_client_cls):
"""Test failure for get-status with --year flag."""
self._test_get_status_failure(mock_client_cls, "--year")

def _test_get_status_failure(self, mock_client_cls, flag):
mock_client = mock_client_cls.return_value
mock_client.get_status.side_effect = Exception("status failed")

with patch.object(sys, "argv", ["cli", "get-status"]):
argv = ["cli", "get-status"]
if flag:
argv.append(flag)

with patch.object(sys, "argv", argv):
cli.main()

error_output = cast(StringIO, sys.stderr).getvalue()
Expand Down
33 changes: 33 additions & 0 deletions tests/test_woffu_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,39 @@ def test_get_diary_hour_types_missing_key(self, mock_get):
result = self.client._get_diary_hour_types("2025-09-12")
self.assertEqual(result, {})

@patch.object(WoffuAPIClient, "get")
def test_get_status_period_week(self, mock_get):
"""Test get-status --week."""
self._test_get_status_period(mock_get, "week")

@patch.object(WoffuAPIClient, "get")
def test_get_status_period_month(self, mock_get):
"""Test get-status --month."""
self._test_get_status_period(mock_get, "month")

@patch.object(WoffuAPIClient, "get")
def test_get_status_period_year(self, mock_get):
"""Test get-status --year."""
self._test_get_status_period(mock_get, "year")

def _test_get_status_period(self, mock_get, period):
mock_get.return_value.status = 200
mock_get.return_value.json.return_value = {
"totalWorkingTimeFormatted": {
"resource": "_HoursMinutesFormatted",
"values": ["225", "0"],
},
"totalWorkedTimeFormatted": {
"resource": "_HoursMinutesFormatted",
"values": ["42", "24"],
},
}

total, running = self.client.get_status(period=period)
self.assertIsInstance(total, timedelta)
self.assertIsNone(running)
mock_get.assert_called_once()

@patch.object(WoffuAPIClient, "get")
def test_download_document_file_exists(self, mock_get):
"""Test download_document skips download if file already exists."""
Expand Down
Loading