Employee resolve diagnostic
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