From 221bbb226e2848d0316b77ac51d1ed89cb202be5 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Tue, 11 Aug 2026 16:10:20 -0400 Subject: [PATCH] employees: an endpoint that says WHY a name did not resolve The shopfloor board lost every photo and started showing lowercase SSOs where names belong, and nothing in the system could say why. Both resolvers returned None on any problem and three separate bare excepts threw the cause away, so an unreachable HR host, a rotated credential, a renamed column and a genuinely unknown SSO all produced identical output and no log line. That is not a bug in the resolution so much as a hole where the diagnosis should be. GET /api/employees/resolve/ returns the same answer the board gets, plus which source produced it (directory, hrdirectory, useaccount), which mode the directory is in, and the exception text when a source failed. It is the difference between "the board is broken" and "the HR host refused the connection". The two bare excepts in the shared resolvers now log rather than pass. The external-directory branch is the one that fails on a live site; it was the one saying nothing. No behaviour change to the board itself - it still falls back exactly as before. What changed is that the fallback is now visible. --- plugins/employees/api/routes.py | 88 +++++++++++++++++++- tests/test_plugins/test_employees_resolve.py | 68 +++++++++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 tests/test_plugins/test_employees_resolve.py diff --git a/plugins/employees/api/routes.py b/plugins/employees/api/routes.py index 2cbccf3..72162e1 100644 --- a/plugins/employees/api/routes.py +++ b/plugins/employees/api/routes.py @@ -147,7 +147,13 @@ def resolve_employee_display_name(sso): if name: return name except Exception: - pass + # Logged, not swallowed. This is the branch that fails on a live + # site - an unreachable HR host, a rotated credential, a renamed + # column - and a bare pass made all of those look identical to + # "no such employee": a lowercase SSO where a name should be, and + # no photo, with nothing written down anywhere. + current_app.logger.exception( + 'Employee directory lookup failed for SSO %s', sso) # 2. Fallback: a shopdb login account for this SSO (e.g. a user from another # location not carried in the directory). Their name lives on the User @@ -160,7 +166,8 @@ def resolve_employee_display_name(sso): if name: return name except Exception: - pass + current_app.logger.exception( + 'User-account name fallback failed for SSO %s', sso) return None @@ -189,6 +196,83 @@ def _with_photo_url(employee): return employee +@employees_bp.route('/resolve/', methods=['GET']) +@jwt_required() +def resolve_employee(sso): + """Resolve one SSO to a name and photo, and say HOW. + + Built because the failure it diagnoses is invisible. The shopfloor board + resolves names and photos through two functions that returned None on any + problem, so an unreachable HR host, a rotated credential and a genuinely + unknown SSO all produced the same thing: a lowercase SSO where a name + should be, and no photo. Nothing distinguished them and nothing was logged. + + This returns the same answer the board gets, plus which source produced it, + which mode the directory is in, and the error when a source failed. It is + the difference between "the board is broken" and "the HR host refused the + connection at 09:12". + """ + result = { + 'sso': sso, + 'mode': 'selfhosted' if _selfhosted() else 'external', + 'name': None, + 'photourl': None, + 'source': None, + 'error': None, + } + + if sso is None or not str(sso).isdigit(): + result['error'] = 'SSO must be digits' + return success_response(result) + + if _selfhosted(): + employee = db.session.get(DirectoryEmployee, int(sso)) + if employee: + name = '{} {}'.format(employee.firstname or '', + employee.lastname or '').strip() + if name: + result['name'] = name + result['source'] = 'directory' + else: + try: + conn = employee_connection() + with conn.cursor() as cur: + cur.execute( + 'SELECT First_Name, Last_Name FROM employees WHERE SSO = %s', + (int(sso),)) + row = cur.fetchone() + conn.close() + if row: + name = '{} {}'.format((row.get('First_Name') or '').strip(), + (row.get('Last_Name') or '').strip()).strip() + if name: + result['name'] = name + result['source'] = 'hrdirectory' + else: + result['error'] = 'No row in the HR directory for this SSO' + except Exception as exception: + # Reported, not hidden. This string is the whole point of the + # endpoint. + result['error'] = '{}: {}'.format(type(exception).__name__, exception) + current_app.logger.exception( + 'Employee directory lookup failed for SSO %s', sso) + + if not result['name']: + from shopdb.api import User + user = User.query.filter_by(username=str(sso)).first() + if user: + name = '{} {}'.format((user.firstname or '').strip(), + (user.lastname or '').strip()).strip() + if name: + result['name'] = name + result['source'] = 'useraccount' + + result['photourl'] = resolve_employee_photo_url(sso) + if result['name'] is None and result['error'] is None: + result['error'] = 'No name found in the directory or in a user account' + return success_response(result) + + @employees_bp.route('/search', methods=['GET']) def search_employees(): """ diff --git a/tests/test_plugins/test_employees_resolve.py b/tests/test_plugins/test_employees_resolve.py new file mode 100644 index 0000000..8f074a9 --- /dev/null +++ b/tests/test_plugins/test_employees_resolve.py @@ -0,0 +1,68 @@ +"""Resolving an SSO says HOW it resolved, or why it did not. + +Built after the shopfloor board lost every photo and showed lowercase SSOs +instead of names, and nothing in the system could say why. Both resolvers +returned None on any problem, so an unreachable HR host, a rotated credential +and a genuinely unknown SSO were indistinguishable - and none of them were +logged. This endpoint exists to tell those apart. +""" + +URL = '/api/employees/resolve/' + + +def _selfhosted(db): + from shopdb.core.models import Setting + row = Setting.query.filter_by(key='employee_directory_mode').first() + if row: + row.value = 'selfhosted' + else: + db.session.add(Setting(key='employee_directory_mode', value='selfhosted')) + db.session.commit() + + +def test_a_known_employee_resolves_and_names_its_source(client, db, auth_headers): + from plugins.employees.models import DirectoryEmployee + _selfhosted(db) + db.session.add(DirectoryEmployee(sso=502123, firstname='Ada', + lastname='Lovelace')) + db.session.commit() + + data = client.get(URL + '502123', headers=auth_headers).get_json()['data'] + assert data['name'] == 'Ada Lovelace' + assert data['source'] == 'directory' + assert data['mode'] == 'selfhosted' + assert data['error'] is None + + +def test_an_unknown_sso_says_so_rather_than_returning_nothing(client, db, + auth_headers): + """The board shows a lowercase SSO for this case AND for a broken + directory. The whole point is that they now read differently.""" + _selfhosted(db) + data = client.get(URL + '999999', headers=auth_headers).get_json()['data'] + assert data['name'] is None + assert data['error'] + + +def test_a_user_account_answers_when_the_directory_does_not(client, db, + auth_headers): + """Someone from another site has no directory row but does have a login. + That fallback exists; without this endpoint nothing showed it had fired.""" + from shopdb.core.models import User + _selfhosted(db) + db.session.add(User(username='701234', firstname='Grace', lastname='Hopper', + email='g@example.com', passwordhash='x')) + db.session.commit() + + data = client.get(URL + '701234', headers=auth_headers).get_json()['data'] + assert data['name'] == 'Grace Hopper' + assert data['source'] == 'useraccount' + + +def test_a_non_numeric_sso_is_rejected_clearly(client, db, auth_headers): + data = client.get(URL + 'abc', headers=auth_headers).get_json()['data'] + assert data['error'] == 'SSO must be digits' + + +def test_the_endpoint_needs_a_login(client, db): + assert client.get(URL + '502123').status_code == 401