Skip to content

Commit

Permalink
Merge pull request #214 from Teknologforeningen/feature/bill-sync
Browse files Browse the repository at this point in the history
Enhance syncing of BILL accounts
  • Loading branch information
filiptypjeu authored Oct 29, 2024
2 parents 526d2c3 + 79373c5 commit b2de58b
Show file tree
Hide file tree
Showing 12 changed files with 191 additions and 187 deletions.
158 changes: 77 additions & 81 deletions teknologr/api/bill.py
Original file line number Diff line number Diff line change
@@ -1,92 +1,88 @@
import requests
import re
import json
import html
from getenv import env

# All BILL accounts are connected to a specific LDAP username.
# Let's use this fact to connect Members to BILL accounts,
# instead of storing the account number separately.

class BILLException(Exception):
pass


class BILLAccountManager:
ERROR_ACCOUNT_DOES_NOT_EXIST = "BILL account does not exist"

def __init__(self):
self.api_url = env("BILL_API_URL")
self.user = env("BILL_API_USER")
self.password = env("BILL_API_PW")

def admin_url(self, bill_code):
if not self.api_url:
return ''
return f'{"/".join(self.api_url.split("/")[:-2])}/admin/userdata?id={bill_code}'

def __request(self, path):
try:
r = requests.post(self.api_url + path, auth=(self.user, self.password))
except:
raise BILLException("Could not connect to BILL server")

if r.status_code != 200:
raise BILLException(f"BILL returned status code {r.status_code}")

# Not a number, return as text
try:
number = int(r.text)
except ValueError:
return r.text

# A negative number means an error code
if number == -3:
raise BILLException(BILLAccountManager.ERROR_ACCOUNT_DOES_NOT_EXIST)
if number < 0:
raise BILLException(f"BILL returned error code: {number}")

return number

def create_bill_account(self, username):
if not re.search(r'^[A-Za-z0-9]+$', username):
raise BILLException("Can not create a BILL account using an LDAP username containing anything other than letters and numbers")

result = self.__request(f"add?type=user&id={username}")
if type(result) == int:
return result
ERROR_NO_BILL = BILLException("BILL is not set up")
ERROR_ACCOUNT_DOES_NOT_EXIST = BILLException("BILL account does not exist")

def admin_url(bill_id):
api_url = env("BILL_API_URL")
if not api_url:
raise ERROR_NO_BILL
return f'{"/".join(api_url.split("/")[:-2])}/admin/userdata?id={bill_id}'

def __request(path):
api_url = env("BILL_API_URL")
user = env("BILL_API_USER")
password = env("BILL_API_PW")
if not all([api_url, user, password]):
raise ERROR_NO_BILL

try:
r = requests.post(api_url + path, auth=(user, password))
except:
raise BILLException("Could not connect to BILL server")

if r.status_code != 200:
raise BILLException(f"BILL returned status code: {r.status_code}")

# The response can be anything, and it's up to the caller to parse it correctly.
# If not a number, return as text.
try:
number = int(r.text)
except ValueError:
# BILL uses HTML ecoding to represent non-ASCII characters
return html.unescape(r.text)

# A negative number means an error code
if number == -3:
raise ERROR_ACCOUNT_DOES_NOT_EXIST
if number < 0:
raise BILLException(f"BILL returned error code: {number}")

# A non-negative number is a valid response
return number

def create_account(username):
if not re.search(r'^[A-Za-z0-9]+$', username):
raise BILLException("Can not create a BILL account using an LDAP username containing anything other than letters and numbers")

result = __request(f"add?type=user&id={username}")
if type(result) == int:
return result
raise BILLException(f"BILL returned error: {result}")

def delete_account(username):
# If the BILL account does not exist all is ok
if not get_account(username):
return

result = __request(f"del?type=user&id={username}")
if result != 0:
raise BILLException(f"BILL returned error: {result}")

def delete_bill_account(self, bill_code):
info = self.get_account_by_code(bill_code)

# If the BILL account does not exist all is ok
if not info:
return

result = self.__request(f"del?type=user&acc={bill_code}")

if result != 0:
raise BILLException(f"BILL returned error: {result}")

def get_account_by_username(self, username):
'''
Get the info for a certain BILL account. Returns None if the account does not exist.
'''
try:
result = self.__request(f"get?type=user&id={username}")
return json.loads(result)
except BILLException as e:
s = str(e)
if s == BILLAccountManager.ERROR_ACCOUNT_DOES_NOT_EXIST:
return None
raise e

def get_account_by_code(self, bill_code):
'''
Get the info for a certain BILL account. Returns None if the account does not exist.
'''
try:
result = self.__request(f"get?type=user&acc={bill_code}")
return json.loads(result)
except BILLException as e:
s = str(e)
if s == BILLAccountManager.ERROR_ACCOUNT_DOES_NOT_EXIST:
return None
raise e
def get_account(username):
'''
Get the info for the BILL account connected to a certain LDAP username.
Returns None if the account does not exist.
'''
try:
result = __request(f"get?type=user&id={username}")
info = json.loads(result)
info['acc'] = int(info['acc'])
# Balance is a float, but leave it as a string to avoid having to format it later
# info['balance'] = float(info['balance'])
return info
except BILLException as e:
if e == ERROR_ACCOUNT_DOES_NOT_EXIST:
return None
raise e
5 changes: 1 addition & 4 deletions teknologr/api/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class MemberFilter(BaseFilter):

# Staff only filters
STAFF_ONLY = Member.STAFF_ONLY_FIELDS
STAFF_ONLY.remove('bill_code') # No longer cached, so can not filter on it
birth_date = django_filters.DateFromToRangeFilter(
label='Född mellan',
)
Expand All @@ -156,9 +157,6 @@ class MemberFilter(BaseFilter):
username = CharFilterWithKeywords(
label='Användarnamn',
)
bill_code = CharFilterWithKeywords(
label='BILL-konto',
)

def includes_hidable_field(self):
''' Check if the current query includes filtering on any hidable Member fields '''
Expand Down Expand Up @@ -332,7 +330,6 @@ class ApplicantFilter(MemberFilter):
graduated_year = None
comment = None
dead = None
bill_code = None
created = None
modified = None
n_functionaries = None
Expand Down
10 changes: 10 additions & 0 deletions teknologr/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ class Meta:
def to_representation(self, instance):
data = super().to_representation(instance)

# Hide bill_code by default, because it is no longer cached in Member
if 'bill_code' in data:
data.pop('bill_code')

hide = not self.is_staff and not instance.show_contact_info()
if hide:
for field in Member.HIDABLE_FIELDS:
Expand All @@ -61,6 +65,12 @@ def to_representation(self, instance):
# Add the actual related objects if detail view
# XXX: Do we need to prefetch all related objects here? It's now done earlier, by the caller...
if self.detail:
if self.is_staff:
# Fetch and add BILL id on detail view only
# XXX: 'bill_code' is the wrong term
bill_info = instance.get_bill_info() or {}
data['bill_code'] = bill_info.get('acc')

data['decorations'] = [{
'decoration': {'id': do.decoration.id, 'name': do.decoration.name},
'acquired': do.acquired,
Expand Down
4 changes: 3 additions & 1 deletion teknologr/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ def test_post_for_superuser(self):
'allow_studentbladet': bool,
'comment': str,
'username': (str, None),
'bill_code': (str, None),
# No longer cached in Member, only fetched for detail view
# 'bill_code': (str, None),
}
MEMBER_DETAIL = {
'functionaries': [{
Expand Down Expand Up @@ -326,6 +327,7 @@ def test_post_for_superuser(self):
'begin_date': str,
'end_date': (str, None),
}],
'bill_code': (int, None),
}

class MembersAPITest(BaseAPITest, GetAllMethodTests, PostMethodTests):
Expand Down
4 changes: 3 additions & 1 deletion teknologr/api/tests_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def setUp(self):
allow_publish_info=True,
comment='Dummy',
username='dummyd1',
bill_code=42,
# bill_code=42,
)

def login_user(self):
Expand Down Expand Up @@ -847,6 +847,7 @@ def setUp(self):
)


'''
class MemberFilterBillCodeTest(BaseAPITest, TestCases):
def setUp(self):
super().setUp()
Expand All @@ -872,3 +873,4 @@ def setUp(self):
allow_publish_info=True,
dead=False,
)
'''
63 changes: 25 additions & 38 deletions teknologr/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from api.serializers import *
from api.filters import *
from api.ldap import LDAPAccountManager, LDAPError_to_string
from api.bill import BILLAccountManager, BILLException
import api.bill as bill
from api.utils import assert_public_member_fields
from api.mailutils import mailNewPassword, mailNewAccount
from members.models import GroupMembership, Member, Group
Expand Down Expand Up @@ -272,7 +272,7 @@ def post(self, request, member_id):
return HttpResponse('Username or password field missing', status=400)

if member.username:
return HttpResponse('Member already has an LDAP account', status=400)
return HttpResponse('Member already has a username', status=400)

if Member.objects.filter(username=username).exists():
return HttpResponse(f'Username "{username}" is already taken', status=400)
Expand All @@ -299,8 +299,9 @@ def delete(self, request, member_id):
member = get_object_or_404(Member, id=member_id)

if not member.username:
return HttpResponse('Member has no LDAP account', status=400)
if member.bill_code:
return HttpResponse('Member has no username', status=400)

if member.get_bill_info():
return HttpResponse('BILL account must be deleted first', status=400)

try:
Expand Down Expand Up @@ -380,58 +381,44 @@ def change_ldap_password(request, member_id):
class BILLAccountView(APIView):
def get(self, request, member_id):
member = get_object_or_404(Member, id=member_id)

if not member.username:
return Response({'detail': 'username missing'}, status=404)

try:
account = BILLAccountManager().get_account_by_code(member.bill_code)
except Exception as e:
return Response({'detail': str(e)}, status=500)
if not account:
return Response({'detail': 'Could not find BILL account.'}, status=404)
return Response(account)
info = bill.get_account(member.username)
if info:
return Response(info)
except bill.BILLException as e:
return Response({'detail': str(e)}, status=404)
return Response({'detail': 'No BILL account found'}, status=404)

def post(self, request, member_id):
member = get_object_or_404(Member, id=member_id)

if member.bill_code:
return HttpResponse('Member already has a BILL account', status=400)
if not member.username:
return HttpResponse('LDAP account missing', status=400)
return HttpResponse('username missing', status=400)

bm = BILLAccountManager()
bill_code = None

# Check if there already is a BILL account with this LDAP name
# Check if the Member already has a BILL account before creating a new one
try:
bill_code = bm.get_account_by_username(member.username).get('acc')
except:
pass

# If not, create a new BILL account
if not bill_code:
try:
bill_code = bm.create_bill_account(member.username)
except BILLException as e:
return HttpResponse(str(e), status=400)

member.bill_code = bill_code
member.save()
if not bill.get_account(member.username):
bill.create_account(member.username)
except bill.BILLException as e:
return HttpResponse(str(e), status=400)

return HttpResponse(status=200)

def delete(self, request, member_id):
member = get_object_or_404(Member, id=member_id)

if not member.bill_code:
return HttpResponse('Member has no BILL account', status=400)
if not member.username:
return HttpResponse('username missing', status=400)

bm = BILLAccountManager()
try:
bm.delete_bill_account(member.bill_code)
except BILLException as e:
bill.delete_account(member.username)
except bill.BILLException as e:
return HttpResponse(str(e), status=400)

member.bill_code = None
member.save()

return HttpResponse(status=200)


Expand Down
11 changes: 8 additions & 3 deletions teknologr/katalogen/templates/profile.html
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,19 @@ <h5>Övrig privat information</h5>
<tr>
<th>BILL-konto</th>
<td class="text-right">
<span class="monospace">{{ member.bill_code|default:'' }}</span>
<span class="monospace">{{ bill.acc }}</span>
</td>
</tr>
<tr>
<th>BILL-smeknamn</th>
<td class="text-right">{{ bill.nick }}
</td>
</tr>
<tr>
<th>BILL-saldo</th>
<td class="text-right">
{% if bill_balance %}
{{ bill_balance }} €
{% if bill.balance %}
{{ bill.balance }} €
{% endif %}
</td>
</tr>
Expand Down
Loading

0 comments on commit b2de58b

Please sign in to comment.