Sep 16 - Optimize code, part 2
This commit is contained in:
@@ -210,6 +210,18 @@ is the reference for who may use what; routes enforce the same rules:
|
||||
- New routes in a gated blueprint are covered automatically; new routes elsewhere need an
|
||||
explicit decorator.
|
||||
|
||||
**Project Manager data scoping (Set 27)** — role gates decide which *pages* a PM may open;
|
||||
`load_project_manager_scope()` (`utils/helpers.py`) decides which *rows* they see. It returns
|
||||
`(is_project_manager, allowed_project_ids, allowed_location_names)` and **fails closed** (a PM with
|
||||
no assignments, or a permission lookup error, sees nothing). Applied in:
|
||||
- `routes/attendance.py`: the report, live updates, `attendance_locations_api`,
|
||||
`time_attendance_locations_api`, `search_employees_api`, `get_project_locations_api`
|
||||
- `routes/dashboard.py`: dashboard QR list + project list, `project_qr_codes` (403-style redirect
|
||||
for someone else's project), `dashboard_stats_api`, `dashboard_realtime_api`
|
||||
- A PM scoped only by **locations** is resolved to the projects behind those locations when
|
||||
employee names are searched, so their filter still works without exposing other projects.
|
||||
- **Any new endpoint returning attendance, employee, QR or project rows must call it.**
|
||||
|
||||
---
|
||||
|
||||
## 5. Application Factory & Initialization Order
|
||||
@@ -756,8 +768,19 @@ within the interval. This is pre-existing behaviour for suffixed IDs.
|
||||
### Special Handling
|
||||
- `Recorded Address`: read via openpyxl directly (not pandas) to preserve HYPERLINK formulas
|
||||
- Duplicate detection: hash of `employee_id + date + time + action_description`
|
||||
- `_clean_employee_id()` normalises IDs to the stored form (Set 26): `1234.0` → `1234`,
|
||||
`1759.PW` / `1759 - PW` / `PW.1759` → `1759PW`. Zero padding is **preserved** (`01234`) and a
|
||||
real fraction (`1234.5`) is **never truncated** — both would change the duplicate hash of rows
|
||||
already imported, or silently move hours to another employee
|
||||
- Import tracked by `import_batch_id` (UUID)
|
||||
- `self.db` not `db` — in `TimeAttendanceImportService`, always access SQLAlchemy via `self.db`
|
||||
- **Dates go through `_parse_date_field()` (Set 27) — never `pd.to_datetime()` directly.** It handles
|
||||
real date cells, Excel serial numbers (a General-formatted column: `45123` → 2023-07-16, which
|
||||
`pd.to_datetime()` read as 1970-01-01), and text dates **month-first** (US time clocks), falling
|
||||
back to day-first only when the first number cannot be a month (`13/04/2026`). Unparseable dates
|
||||
fail the row instead of importing a wrong one
|
||||
- **A failed import removes its own rows** (`_rollback_partial_batch()`): the loop commits every 50
|
||||
records, so a failure part-way used to leave a partial batch with nothing marking it incomplete
|
||||
- Validation `except` blocks must not silently swallow exceptions (fail-open prevention)
|
||||
|
||||
---
|
||||
@@ -777,8 +800,15 @@ within the interval. This is pre-existing behaviour for suffixed IDs.
|
||||
- **SP** = Special Project, **PW** = Periodic Work, **PT** = Project Team (Part-Time),
|
||||
**C** = Covering (added Sept 2026)
|
||||
- Parsed from `employee_id` via `parse_employee_id_for_work_type()`
|
||||
- Suffix and prefix forms, with or without a separator: `1234SP`, `1234 SP`, `SP1234`,
|
||||
`1234.PW`. Two-letter codes are matched **before** the single-letter `C`
|
||||
- Suffix and prefix forms, separated by **any run of non-alphanumeric characters, or nothing**:
|
||||
`1234SP`, `1234 SP`, `1234.PW`, `1234-PT`, `1234 . C`, `SP1234`, `SP 1234`, `PW.1234`.
|
||||
Two-letter codes are matched **before** the single-letter `C`
|
||||
- The separator class is `[^0-9A-Z]*` in all three parsers — `parse_employee_id_for_work_type()`,
|
||||
`build_employee_id_regex()` (SQL filters) and `parseEmployeeIdWorkType()` (report JS). Keep them
|
||||
in sync: until Sept 16 2026 the Python one accepted only a space, so `1759.PW` was counted as a
|
||||
separate Regular employee in the exports while the report showed it as PW (Set 26)
|
||||
- **Not** work types: `1234.5` (no code) and `1234SPX` (code runs into a word) — both stay regular
|
||||
with the ID unchanged
|
||||
- Like SP/PW/PT, `C` hours are excluded from the 40-hour overtime rule
|
||||
- **Codes are declared in four places — keep them in sync:**
|
||||
| File | Symbol |
|
||||
@@ -791,6 +821,26 @@ within the interval. This is pre-existing behaviour for suffixed IDs.
|
||||
aggregation dicts are keyed `regular/SP/PW/PT`, so adding `C` to its parser alone
|
||||
would raise `KeyError`
|
||||
|
||||
### Overtime — 40-Hour Rule (Sept 16, 2026, confirmed by the user)
|
||||
|
||||
**SP / PW / PT / C hours are paid but never build toward overtime.** Both exports compute the
|
||||
weekly Regular and OT columns from regular hours only:
|
||||
|
||||
```python
|
||||
week_regular = min(weekly_regular_hours, 40.0) # NOT weekly_total_hours
|
||||
week_overtime = max(0, weekly_regular_hours - 40.0)
|
||||
```
|
||||
|
||||
- `weekly_regular_hours` accumulates `_qtr()` of each day's regular-only pair hours;
|
||||
`_pair_is_regular(in, out)` decides, preferring the **OUT** record's work type (a Regular IN
|
||||
paired with an SP OUT counts as SP, mirroring `effective_work_type`).
|
||||
- **Column H (Weekly Total) still shows every hour worked**, so H ≠ Regular + OT whenever the
|
||||
employee has special hours — the SP/PW/PT/C and Regular rows below break that down.
|
||||
- Example: 36 h Regular + 8 h SP → H 44, Regular 36, **OT 0** (before Sept 16: OT 4).
|
||||
- **Export by Building applies the same rule per BUILDING**, so an employee with 30 h at two
|
||||
buildings shows 0 OT in each block while the main export shows 20 h. A note row under the date
|
||||
range says so. Treat that sheet as review-only; pay from the main export (§20 Set 25).
|
||||
|
||||
### Overnight Shift Handling
|
||||
**Rule 1 — Sort key:** early-morning OUTs (`hour <= 3`) use `_overnight_aware_sort_key()` which adds 86400 seconds — pushes them past midnight so they sort after same-day evening INs.
|
||||
|
||||
@@ -1053,6 +1103,12 @@ new_name = qr_code.name # always use existing name, never request.form['name']
|
||||
### Timestamp Convention
|
||||
Use `datetime.now()` (local time) throughout — **not** `datetime.utcnow()`.
|
||||
|
||||
Check-in dates/times are stored local, so anything compared against them must be local too.
|
||||
Fixed in Set 27: `routes/attendance_edit.py` (record timestamps + audit note), `routes/dashboard.py`
|
||||
("today" / "last 30 days" stats, which rolled over mid-evening) and `app.py` (`CURRENT_YEAR`).
|
||||
Still UTC on purpose or harmlessly: `users.last_login_date`, security-middleware event stamps,
|
||||
`qr_codes.coordinates_updated_date`, import batch `import_date`.
|
||||
|
||||
### Context Safety
|
||||
- Use `has_request_context()` (not `if not request:`) to check Flask request context
|
||||
- Capture `current_app._get_current_object()` in route body, not inside lazy generators
|
||||
@@ -1333,6 +1389,34 @@ it (the workers share no pub/sub).
|
||||
| `extensions.py` | `logger_handler` is now a proxy (see §14). `utils/helpers.py` imports it at module import time, which happens BEFORE `init_logger()`, so it was permanently `None`: the Set 23 role check logged a warning when denying access and raised `AttributeError: 'NoneType' object has no attribute 'logger'` → 500 for a project manager opening `/time-attendance` or editing a QR code. The same latent bug affected `generate_qr_code()` logging in `utils/helpers.py` and every call in `utils/template_helpers.py` / `utils/geocoding.py` |
|
||||
| — | The step-1 tests had stubbed a working logger into `extensions`, which hid it. They now import the real module and run the role checks BEFORE `init_logger()`, exactly like a gunicorn worker |
|
||||
|
||||
### Set 25 — Overtime Counted SP/PW/PT/C Hours (Sept 16, 2026)
|
||||
| File | Fix |
|
||||
|---|---|
|
||||
| `routes/time_attendance_export.py` | Both exports added EVERY paired hour to the weekly total that overtime is calculated from, so 36 h Regular + 8 h SP produced 4 h OT. Added `_pair_is_regular()` and a `weekly_regular_hours` counter; the weekly-boundary and final weekly rows now take Regular/OT from it (§13). Column H is unchanged (all hours worked) |
|
||||
| `routes/time_attendance_export.py` | Export by Building: a note row under the date range states that its Weekly Total / Regular / OT are **per building** and exclude SP/PW/PT/C — the user was unsure whether that sheet is used for pay, so it is marked rather than changed. Shifts every Sheet0 row down by one (the Filtered Report is built from row bookkeeping, so it follows) |
|
||||
| — | Verified by running both real export functions over the same fixture week, before (HEAD) vs after: 36 h Reg + 8 h SP → OT 4 → 0; 40 h Reg + 4 h C → 4 → 0; Regular-IN/SP-OUT cross-type pair → 4 → 0; 44 h plain Regular unchanged at 4; two-building employee unchanged (20 h in the main export, 0 per building block); punch rows identical; SP/C summary rows unchanged |
|
||||
| — | **Payroll impact:** weeks where an employee had both special-type hours and 40+ total hours will now show less overtime than the same export produced before |
|
||||
|
||||
### Set 26 — Work-Type IDs With a Separator (`1759.PW`) (Sept 16, 2026)
|
||||
| File | Fix |
|
||||
|---|---|
|
||||
| `working_hours_calculator.py` | `parse_employee_id_for_work_type()` accepted only a space (`\s*`), so `1759.PW` — a spelling that really exists in imported data — parsed as base `1759.PW` / regular. The exports then showed a separate REGULAR employee "1759.PW" while the report and SQL filters (which use `[^0-9A-Z]*`) treated it as PW hours for 1759. Separator class aligned with the other two parsers (§13) |
|
||||
| `time_attendance_import_service.py` | `_clean_employee_id()` now stores work-type IDs canonically (`1759.PW` → `1759PW`), keeps zero padding, and no longer truncates `1234.5` to `1234` (silent data loss) |
|
||||
| — | Verified: 17 ID spellings through the parser (before vs after), agreement with the report's JS parser, 11 cleaner cases (incl. the two old bugs), and an end-to-end export where a 7 h `1759.PW` shift now appears as PW hours for employee 1759 (39 h worked / 32 regular / 0 OT) instead of its own block. Overtime (24) and step-1 (71) suites still pass |
|
||||
| — | **No data migration:** existing rows keep their stored spelling and are read correctly now. Exports for past weeks will move those hours from "Employee 1759.PW" Regular into 1759's PW row — and out of the overtime base (Set 25) |
|
||||
|
||||
### Set 27 — Import Dates, PM Data Scoping, Manual Entry (Sept 16, 2026)
|
||||
| File | Fix |
|
||||
|---|---|
|
||||
| `time_attendance_import_service.py` | `_parse_date_field()` replaces `pd.to_datetime()` at all four call sites: Excel serial numbers imported as **1970-01-01**, and `03/04/2026` was read month-first with no rule written down (§12) |
|
||||
| `time_attendance_import_service.py` | `_rollback_partial_batch()` — a mid-import failure left a partial batch (commits every 50 rows); the batch is now deleted and reported, so an import is all-or-nothing |
|
||||
| `utils/helpers.py` | `load_project_manager_scope()` — one fail-closed source for a PM's projects/locations (§4) |
|
||||
| `routes/attendance.py` | Scope applied to `attendance_locations_api`, `time_attendance_locations_api`, `search_employees_api` (location-only PMs resolved to their projects), `get_project_locations_api` |
|
||||
| `routes/dashboard.py` | Scope applied to the dashboard QR + project lists, `project_qr_codes` (redirect for another project), `dashboard_stats_api`, `dashboard_realtime_api`. "Today" now uses local time |
|
||||
| `routes/attendance_edit.py` | `_normalize_manual_employee_id()` — manual add did `int(employee_id)`, so `1234SP` could not be entered at all, and edit accepted any text (pseudo-employees in the exports). Both now accept 1–4 digits with an optional work type and store it canonically; timestamps local |
|
||||
| `app.py` | `CURRENT_YEAR` from local time |
|
||||
| — | Verified offline (42 checks): 16 date cases + 6 rejections, partial-batch rollback against a fake session, PM scope loading, scoped vs unscoped SQL for both locations APIs (values bound as parameters), empty result for an unassigned PM, 9 manual-ID cases, and no `utcnow()` left in the two route files. Suites still green: 71 (step 1), 24 (overtime), 38 (work-type IDs). Not run against MySQL or a browser |
|
||||
|
||||
---
|
||||
|
||||
## 21. Infrastructure & Deployment
|
||||
|
||||
Reference in New Issue
Block a user