refactor: overhaul erpnext_sync and add test suite#1
Open
MhmdSinanKT wants to merge 1 commit into
Open
Conversation
- extract _auth_headers(), _resolve_punch_direction(), _find_resume_index(), _get_dump_path(), _build_allowlisted_errors() as standalone functions - replace string concatenation with f-strings throughout - fix double-encoded JSON body in send_shift_sync_to_erpnext (data= → json=) - fix _safe_parse_dt to tolerate timestamps without microseconds - add get_shift_type_device_mapping() to fetch shift-device config from ERPNext at startup instead of requiring manual config - use os.path.join and os.makedirs(exist_ok=True) over manual path building - replace bare except with specific exception types - add type hints on all public functions test: add unit and live integration test suite - 47 unit tests covering all pure functions and mocked API calls - 9 live integration tests behind --live flag hitting a real ERPNext server, with mock attendance data generation and automatic checkin cleanup - conftest.py registers the --live pytest option
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors the core sync script for maintainability and correctness, adds
get_shift_type_device_mapping()to fetch shift-device configurationdirectly from ERPNext at startup, and introduces a full test suite with
both unit and live integration tests.
Changes
erpnext_sync.py— RefactorNew functionality
get_shift_type_device_mapping(): fetches Shift Type → device mappingsfrom ERPNext at startup instead of requiring the user to manually maintain
shift_type_device_mappinginlocal_config.py. Config is stillsupported for backward compatibility.
Bug fixes
send_shift_sync_to_erpnextwas double-encoding the request body(
data=json.dumps(data)→json=payload), causing silent failures onthe ERPNext side.
_safe_parse_dtfailed to parse timestamps stored viastr(datetime),which produces no microseconds. This caused
update_shift_last_sync_timestampto silently skip syncing for any device whose pull timestamp landed on a
whole second.
_find_resume_indexhad an off-by-one risk in the originalindex_of_last + 1pattern; replaced with a cleaner direct index return.Refactors
_auth_headers()— single place to manage API credentials format_resolve_punch_direction()— punch AUTO logic out of the loop_find_resume_index()— resume logic now independently testable_build_allowlisted_errors()— replaces module-level procedural blockos.path.joinreplaces manual'/'join([...])string buildingos.makedirs(exist_ok=True)replaces theif not existsguard"token " + x + ":" + yconcatenationrequests.post/put/getreplacesrequests.request("POST", ...)callsexcept:replaced withexcept Exception:throughouttest_erpnext_sync.py+conftest.py— New test suiteUnit tests (47, always run)
TestSafeParseDtTestSafeGetErrorStrexcextraction, plain JSON, fallbackTestResolvePunchDirectionTestApplyFnToKeyTestGetDumpPathTestGetLastLineTestFindResumeIndexTestBuildAllowlistedErrorsTestSendToErpnextTestSendShiftSyncToErpnextTestGetShiftTypeDeviceMappingTestPullProcessAndPushDataTestUpdateShiftLastSyncTimestampLive integration tests (9, opt-in via
--live)Hit a real ERPNext server using credentials from
local_config.py.All checkins created during the run are automatically deleted on teardown.
TestLiveErpNextConnectionTestLiveSendToErpnextTestLiveBatchUploadSet
LIVE_TEST_EMPLOYEE_IDat the top of the test file to pin a specificemployee, or leave as
Noneto auto-detect the first active employee.How to test