refactor: code quality improvements across the backend
- Remove insecure SSL verification bypass in attendance.py - Add gunicorn as production WSGI server (Dockerfile + entrypoint) - Fix silent data loss in reconciliation (log + surface unmatched members) - Add required column validation in payment sheet parsing - Add input validation on /qr route (account format, amount bounds, SPD injection) - Centralize configuration into scripts/config.py - Extract credentials path to env-configurable constant - Hide unmatched transactions from reconcile-juniors page - Fix test mocks to bypass cache layer (all 8 tests now pass reliably) - Add pytest + pytest-cov dev dependencies - Fix typo "Inffering" in infer_payments.py - Update CLAUDE.md to reflect current project state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,38 +1,44 @@
|
||||
import unittest
|
||||
from unittest.mock import patch, MagicMock
|
||||
from unittest.mock import patch
|
||||
from app import app
|
||||
|
||||
|
||||
def _bypass_cache(cache_key, sheet_id, fetch_func, *args, serialize=None, deserialize=None, **kwargs):
|
||||
"""Test helper: call fetch_func directly, bypassing the cache layer."""
|
||||
return fetch_func(*args, **kwargs)
|
||||
|
||||
|
||||
class TestWebApp(unittest.TestCase):
|
||||
def setUp(self):
|
||||
# Configure app for testing
|
||||
app.config['TESTING'] = True
|
||||
self.client = app.test_client()
|
||||
|
||||
@patch('app.get_members_with_fees')
|
||||
def test_index_page(self, mock_get_members):
|
||||
def test_index_page(self):
|
||||
"""Test that / returns the refresh meta tag"""
|
||||
response = self.client.get('/')
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertIn(b'url=/fees', response.data)
|
||||
|
||||
@patch('app.get_cached_data', side_effect=_bypass_cache)
|
||||
@patch('app.get_members_with_fees')
|
||||
def test_fees_route(self, mock_get_members):
|
||||
@patch('app.fetch_exceptions', return_value={})
|
||||
def test_fees_route(self, mock_exceptions, mock_get_members, mock_cache):
|
||||
"""Test that /fees returns 200 and renders the dashboard"""
|
||||
# Mock attendance data
|
||||
mock_get_members.return_value = (
|
||||
[('Test Member', 'A', {'2026-01': (750, 4)})],
|
||||
['2026-01']
|
||||
)
|
||||
|
||||
|
||||
response = self.client.get('/fees')
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertIn(b'FUJ Fees Dashboard', response.data)
|
||||
self.assertIn(b'Test Member', response.data)
|
||||
|
||||
@patch('app.get_cached_data', side_effect=_bypass_cache)
|
||||
@patch('app.get_junior_members_with_fees')
|
||||
def test_fees_juniors_route(self, mock_get_junior_members):
|
||||
@patch('app.fetch_exceptions', return_value={})
|
||||
def test_fees_juniors_route(self, mock_exceptions, mock_get_junior_members, mock_cache):
|
||||
"""Test that /fees-juniors returns 200 and renders the junior dashboard"""
|
||||
# Mock attendance data: one with string symbol '?', one with integer
|
||||
mock_get_junior_members.return_value = (
|
||||
[
|
||||
('Test Junior 1', 'J', {'2026-01': ('?', 1, 0, 1)}),
|
||||
@@ -40,7 +46,7 @@ class TestWebApp(unittest.TestCase):
|
||||
],
|
||||
['2026-01']
|
||||
)
|
||||
|
||||
|
||||
response = self.client.get('/fees-juniors')
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertIn(b'FUJ Junior Fees Dashboard', response.data)
|
||||
@@ -48,16 +54,16 @@ class TestWebApp(unittest.TestCase):
|
||||
self.assertIn(b'? / 1 (J)', response.data)
|
||||
self.assertIn(b'500 CZK / 4 (1A+3J)', response.data)
|
||||
|
||||
@patch('app.get_cached_data', side_effect=_bypass_cache)
|
||||
@patch('app.fetch_sheet_data')
|
||||
@patch('app.fetch_exceptions', return_value={})
|
||||
@patch('app.get_members_with_fees')
|
||||
def test_reconcile_route(self, mock_get_members, mock_fetch_sheet):
|
||||
def test_reconcile_route(self, mock_get_members, mock_exceptions, mock_fetch_sheet, mock_cache):
|
||||
"""Test that /reconcile returns 200 and shows matches"""
|
||||
# Mock attendance data
|
||||
mock_get_members.return_value = (
|
||||
[('Test Member', 'A', {'2026-01': (750, 4)})],
|
||||
['2026-01']
|
||||
)
|
||||
# Mock sheet data - include all keys required by reconcile
|
||||
mock_fetch_sheet.return_value = [{
|
||||
'date': '2026-01-01',
|
||||
'amount': 750,
|
||||
@@ -67,17 +73,17 @@ class TestWebApp(unittest.TestCase):
|
||||
'sender': 'External Bank User',
|
||||
'inferred_amount': 750
|
||||
}]
|
||||
|
||||
|
||||
response = self.client.get('/reconcile')
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertIn(b'Payment Reconciliation', response.data)
|
||||
self.assertIn(b'Test Member', response.data)
|
||||
self.assertIn(b'OK', response.data)
|
||||
|
||||
@patch('app.get_cached_data', side_effect=_bypass_cache)
|
||||
@patch('app.fetch_sheet_data')
|
||||
def test_payments_route(self, mock_fetch_sheet):
|
||||
def test_payments_route(self, mock_fetch_sheet, mock_cache):
|
||||
"""Test that /payments returns 200 and groups transactions"""
|
||||
# Mock sheet data
|
||||
mock_fetch_sheet.return_value = [{
|
||||
'date': '2026-01-01',
|
||||
'amount': 750,
|
||||
@@ -92,10 +98,11 @@ class TestWebApp(unittest.TestCase):
|
||||
self.assertIn(b'Test Member', response.data)
|
||||
self.assertIn(b'Direct Member Payment', response.data)
|
||||
|
||||
@patch('app.get_cached_data', side_effect=_bypass_cache)
|
||||
@patch('app.fetch_sheet_data')
|
||||
@patch('app.fetch_exceptions')
|
||||
@patch('app.get_junior_members_with_fees')
|
||||
def test_reconcile_juniors_route(self, mock_get_junior, mock_exceptions, mock_transactions):
|
||||
def test_reconcile_juniors_route(self, mock_get_junior, mock_exceptions, mock_transactions, mock_cache):
|
||||
"""Test that /reconcile-juniors correctly computes balances for juniors."""
|
||||
mock_get_junior.return_value = (
|
||||
[
|
||||
@@ -114,7 +121,7 @@ class TestWebApp(unittest.TestCase):
|
||||
'sender': 'Parent',
|
||||
'inferred_amount': 500
|
||||
}]
|
||||
|
||||
|
||||
response = self.client.get('/reconcile-juniors')
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertIn(b'Junior Payment Reconciliation', response.data)
|
||||
|
||||
Reference in New Issue
Block a user