Add utility functions for enum value handling and platform normalization#847
Add utility functions for enum value handling and platform normalization#847tenkus47 wants to merge 1 commit into
Conversation
- Introduced `_enum_value` to convert enum and string representations to their corresponding string values. - Added `_normalize_platform` to standardize platform values to lowercase. - Updated `get_users_with_matching_timeblocks` to utilize these new functions for improved data consistency. - Created unit tests for both utility functions to ensure correct behavior across various input types.
Confidence Score: 4/5Safe to merge for the current enum values in use; the helper functions behave correctly for The refactoring works correctly for all known inputs. The dot-splitting fallback is the sharpest edge — any future string platform value that contains a dot would be silently truncated — but this is speculative given the current enum definitions. The The Reviews (1): Last reviewed commit: "Add utility functions for enum value han..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if hasattr(value, "value"): | ||
| return str(value.value) |
There was a problem hiding this comment.
hasattr check may match non-enum objects
hasattr(value, "value") will return True for any object that has a value attribute — not just enum.Enum instances. If a SQLAlchemy row proxy, a Pydantic model, or a named tuple with a value field is accidentally passed in, its .value will be silently consumed without a type error. Using isinstance(value, enum.Enum) makes the intent explicit and avoids false positives.
| raw = str(value) | ||
| if "." in raw: | ||
| return raw.rsplit(".", 1)[-1] |
There was a problem hiding this comment.
The "." in raw branch is designed to handle SQLAlchemy returning enum members as their repr-like string (e.g. "SessionType.PLAN"), but it applies to any string that happens to contain a dot. A future platform value like "fcm.android" or a URL-like string would be silently truncated to just the last segment. Consider narrowing the check (e.g. only strip when the prefix matches a known class name) or document the assumed input range clearly.
| from pecha_api.push_devices.push_device_enums import PushPlatform | ||
| from pecha_api.routines.routine_notifications.routine_notification_repository import ( | ||
| _enum_value, | ||
| _normalize_platform, | ||
| ) | ||
| from pecha_api.routines.routines_enums import SessionType | ||
|
|
||
|
|
||
| def test_enum_value_from_session_type_enum(): | ||
| assert _enum_value(SessionType.SERIES) == "SERIES" | ||
| assert _enum_value(SessionType.PLAN) == "PLAN" | ||
|
|
||
|
|
||
| def test_enum_value_from_stringified_enum_name(): | ||
| assert _enum_value("SessionType.SERIES") == "SERIES" | ||
| assert _enum_value("SessionType.PLAN") == "PLAN" | ||
|
|
||
|
|
||
| def test_enum_value_from_plain_string(): | ||
| assert _enum_value("SERIES") == "SERIES" | ||
|
|
||
|
|
||
| def test_normalize_platform_from_push_platform_enum(): | ||
| assert _normalize_platform(PushPlatform.ANDROID) == "android" | ||
| assert _normalize_platform(PushPlatform.IOS) == "ios" | ||
|
|
||
|
|
||
| def test_normalize_platform_from_stringified_enum_name(): | ||
| assert _normalize_platform("PushPlatform.ANDROID") == "android" | ||
| assert _normalize_platform("pushplatform.android") == "android" | ||
|
|
||
|
|
||
| def test_normalize_platform_from_plain_string(): | ||
| assert _normalize_platform("ANDROID") == "android" |
There was a problem hiding this comment.
None input — an explicitly handled case — has no test coverage
_enum_value has an explicit early-return for None (returning ""), and _normalize_platform(None) therefore also returns "". This is a deliberate behavior change from the previous str(None).lower() → "none" path, but no test asserts on it. Without a test, the None branch could be silently altered in a future refactor without any signal.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
_enum_valueto convert enum and string representations to their corresponding string values._normalize_platformto standardize platform values to lowercase.get_users_with_matching_timeblocksto utilize these new functions for improved data consistency.