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/<sso> 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.
This commit is contained in:
@@ -147,7 +147,13 @@ def resolve_employee_display_name(sso):
|
|||||||
if name:
|
if name:
|
||||||
return name
|
return name
|
||||||
except Exception:
|
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
|
# 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
|
# location not carried in the directory). Their name lives on the User
|
||||||
@@ -160,7 +166,8 @@ def resolve_employee_display_name(sso):
|
|||||||
if name:
|
if name:
|
||||||
return name
|
return name
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
current_app.logger.exception(
|
||||||
|
'User-account name fallback failed for SSO %s', sso)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
@@ -189,6 +196,83 @@ def _with_photo_url(employee):
|
|||||||
return employee
|
return employee
|
||||||
|
|
||||||
|
|
||||||
|
@employees_bp.route('/resolve/<sso>', 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'])
|
@employees_bp.route('/search', methods=['GET'])
|
||||||
def search_employees():
|
def search_employees():
|
||||||
"""
|
"""
|
||||||
|
|||||||
68
tests/test_plugins/test_employees_resolve.py
Normal file
68
tests/test_plugins/test_employees_resolve.py
Normal file
@@ -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
|
||||||
Reference in New Issue
Block a user