feature/pricelist add server pagination for balance #17

Merged
ferfissimo merged 3 commits from feature/pricelist into develop 2021-11-25 11:22:51 +00:00
5 changed files with 140 additions and 30 deletions
Showing only changes of commit 2634181d5e - Show all commits

View File

@ -118,8 +118,12 @@ def modify_user(user, password, new_password=None):
messageController.send_message(messageController.Message(user, text, subject))
def get_users():
return User.query.all()
def get_users(userids=None):

Siehe unten: https://flaschengeist.dev/Flaschengeist/flaschengeist/pulls/17/files#issuecomment-164

Bez. die funktion im controller ist schon ok, aber damit wird alles immer sortiert, nicht so sicher ob das sortieren hier immer sinnvoll ist.
Also a] ob es sinnvoll ist das immer zu tun und b] ob die sortierung hardcoded Sinn ergibt (z.b. meist sieht man ja die User mit dem display_name und da kann das z.b. manchmal zu komischen ergebnisse führen)

Siehe unten: https://flaschengeist.dev/Flaschengeist/flaschengeist/pulls/17/files#issuecomment-164 Bez. die funktion im controller ist schon ok, aber damit wird alles immer sortiert, nicht so sicher ob das sortieren hier immer sinnvoll ist. Also a] ob es sinnvoll ist das immer zu tun und b] ob die sortierung hardcoded Sinn ergibt (z.b. meist sieht man ja die User mit dem display_name und da kann das z.b. manchmal zu komischen ergebnisse führen)

Ich dachte, ich arbeite schon mal vor, falls wir hier auch server side pagination brauchen. demzufolge, können wir das hier auch wieder raus nehmen.

Ich dachte, ich arbeite schon mal vor, falls wir hier auch server side pagination brauchen. demzufolge, können wir das hier auch wieder raus nehmen.
query = User.query
if userids:
query.filter(User.userid in userids)
query = query.order_by(User.lastname.asc(), User.firstname.asc())
return query.all()
def get_user_by_role(role: Role):
@ -175,8 +179,8 @@ def register(data):
allowed_keys = User().serialize().keys()
values = {key: value for key, value in data.items() if key in allowed_keys}
roles = values.pop("roles", [])
if "birthday" in values:
values["birthday"] = from_iso_format(values["birthday"]).date()
if "birthday" in data:
values["birthday"] = from_iso_format(data["birthday"]).date()
user = User(**values)
set_roles(user, roles)
@ -195,6 +199,8 @@ def register(data):
)
messageController.send_message(messageController.Message(user, text, subject))
find_user(user.userid)

Sieht für mich ziemlich unnötig aus, gibt es einen Grund das hier aufzurufen?

Sieht für mich ziemlich unnötig aus, gibt es einen Grund das hier aufzurufen?

Damit werden gleich Userattributes geupdatet. Ohne diese Funktion, gibt es keine "DN" und es erschien mir einfacher, das einfach damit aufzurufen. (Benutze ich auch in run_flaschengeist ldap_sync)

Damit werden gleich _Userattributes geupdatet_. Ohne diese Funktion, gibt es keine "DN" und es erschien mir einfacher, das einfach damit aufzurufen. (Benutze ich auch in run_flaschengeist ldap_sync)

Allgemein, sollte das eigentlich in einem anderen commit rein.

Allgemein, sollte das eigentlich in einem anderen commit rein.
return user

View File

@ -104,6 +104,11 @@ class User(db.Model, ModelSerializeMixin):
def has_permission(self, permission):
return permission in self.get_permissions()
def __repr__(self):

Sieht nach debugging aus? Ist das für logs gedacht? Dann vielleicht lieber __str__ als __repr__ (repr sollte eindeutig sein, das string ist aber nicht per-se eindeutig).

Sieht nach debugging aus? Ist das für logs gedacht? Dann vielleicht lieber `__str__` als `__repr__` (repr sollte eindeutig sein, das string ist aber nicht per-se eindeutig).

Ja dies war für debugging. Evtl. könnten wir sowas ja auch Einführen dass wir __str__ und __repr__ für models einführen. (Liest sich schöner im Log)

Ja dies war für debugging. Evtl. könnten wir sowas ja auch Einführen dass wir `__str__` und `__repr__` für models einführen. (Liest sich schöner im Log)
return (
f"User({self.userid}, {self.firstname}, {self.lastname}, {self.mail}, {self.display_name}, {self.birthday})"
)
class _UserAttribute(db.Model, ModelSerializeMixin):
__tablename__ = "user_attribute"

View File

@ -3,12 +3,13 @@
# English: Debit -> from account
# Credit -> to account
from sqlalchemy import func
from sqlalchemy import func, case, and_
from sqlalchemy.ext.hybrid import hybrid_property
from datetime import datetime
from werkzeug.exceptions import BadRequest, NotFound, Conflict
from flaschengeist.database import db
from flaschengeist.models.user import User
from flaschengeist.models.user import User, _UserAttribute
from .models import Transaction
from . import permissions, BalancePlugin
@ -38,27 +39,114 @@ def get_balance(user, start: datetime = None, end: datetime = None):
return credit, debit, credit - debit
def get_balances(start: datetime = None, end: datetime = None):
debit = db.session.query(Transaction.sender_id, func.sum(Transaction.amount)).filter(Transaction.sender_ != None)
credit = db.session.query(Transaction.receiver_id, func.sum(Transaction.amount)).filter(
Transaction.receiver_ != None
)
if start:
debit = debit.filter(start <= Transaction.time)
credit = credit.filter(start <= Transaction.time)
if end:
debit = debit.filter(Transaction.time <= end)
credit = credit.filter(Transaction.time <= end)
def get_balances(start: datetime = None, end: datetime = None, limit=None, offset=None, descending=None, sortBy=None):

Der Ansatz ist cool! NICE 👍

Der Ansatz ist cool! NICE 👍

Noch irgendwas hier zu machen?

Noch irgendwas hier zu machen?
class _User(User):
_debit = db.relationship(Transaction, back_populates="sender_", foreign_keys=[Transaction._sender_id])
_credit = db.relationship(Transaction, back_populates="receiver_", foreign_keys=[Transaction._receiver_id])
debit = debit.group_by(Transaction._sender_id).all()
credit = credit.group_by(Transaction._receiver_id).all()
@hybrid_property
def debit(self):
return sum([cred.amount for cred in self._debit])
@debit.expression
def debit(cls):
a = (
db.select(func.sum(Transaction.amount))
.where(cls.id_ == Transaction._sender_id, Transaction.amount)
.scalar_subquery()
)
return case([(a, a)], else_=0)
@hybrid_property
def credit(self):
return sum([cred.amount for cred in self._credit])
@credit.expression
def credit(cls):
b = (
db.select(func.sum(Transaction.amount))
.where(cls.id_ == Transaction._receiver_id, Transaction.amount)
.scalar_subquery()
)
return case([(b, b)], else_=0)
@hybrid_property
def limit(self):
return self.get_attribute("balance_limit", None)
@limit.expression
def limit(cls):
return (
db.select(_UserAttribute.value)
.where(and_(cls.id_ == _UserAttribute.user, _UserAttribute.name == "balance_limit"))
.scalar_subquery()
)
def get_debit(self, start: datetime = None, end: datetime = None):
if start and end:
return sum([deb.amount for deb in self._debit if start <= deb.time and deb.time <= end])
if start:
return sum([deb.amount for deb in self._dedit if start <= deb.time])
if end:
return sum([deb.amount for deb in self._dedit if deb.time <= end])
return self.debit
def get_credit(self, start: datetime = None, end: datetime = None):
if start and end:
return sum([cred.amount for cred in self._credit if start <= cred.time and cred.time <= end])
if start:
return sum([cred.amount for cred in self._credit if start <= cred.time])
if end:
return sum([cred.amount for cred in self._credit if cred.time <= end])
return self.credit
query = _User.query
if start:
q1 = query.join(_User._credit).filter(start <= Transaction.time)
q2 = query.join(_User._debit).filter(start <= Transaction.time)
query = q1.union(q2)
if end:
q1 = query.join(_User._credit).filter(Transaction.time <= end)
q2 = query.join(_User._debit).filter(Transaction.time <= end)
query = q1.union(q2)
if sortBy == "balance":
if descending:
query = query.order_by((_User.credit - _User.debit).desc(), _User.lastname.asc(), _User.firstname.asc())
else:
query = query.order_by((_User.credit - _User.debit).asc(), _User.lastname.asc(), _User.firstname.asc())
elif sortBy == "limit":
if descending:
query = query.order_by(_User.limit.desc(), User.lastname.asc(), User.firstname.asc())
else:
query = query.order_by(_User.limit.asc(), User.lastname.asc(), User.firstname.asc())
elif sortBy == "firstname":
if descending:
query = query.order_by(User.firstname.desc(), User.lastname.desc())
else:
query = query.order_by(User.firstname.asc(), User.lastname.asc())
elif sortBy == "lastname":
if descending:
query = query.order_by(User.lastname.desc(), User.firstname.desc())
else:
query = query.order_by(User.lastname.asc(), User.firstname.asc())
count = None
if limit:
count = query.count()
query = query.limit(limit)
if offset:
query = query.offset(offset)
users = query
all = {}
for uid, cred in credit:
all[uid] = [cred, 0]
for uid, deb in debit:
all.setdefault(uid, [0, 0])
all[uid][1] = deb
return all
for user in users:
all[user.userid] = [user.get_credit(start, end), 0]
all[user.userid][1] = user.get_debit(start, end)
return all, count
def send(sender: User, receiver, amount: float, author: User):

View File

@ -110,8 +110,10 @@ def limits(current_session: Session):
Returns:
JSON encoded array of userid with limit or HTTP-error
"""
users = userController.get_users()
userids = None
if "userids" in request.args:
Siehe unten: https://flaschengeist.dev/Flaschengeist/flaschengeist/pulls/17/files#issuecomment-164
[x for x in request.args.get("userids").split(",") if x]
users = userController.get_users(userids=userids)
if request.method == "GET":
return jsonify([{"userid": user.userid, "limit": user.get_attribute("balance_limit")} for user in users])
@ -311,5 +313,11 @@ def get_balances(current_session: Session):
Returns:
JSON Array containing credit, debit and userid for each user or HTTP error
"""
balances = balance_controller.get_balances()
return jsonify([{"userid": u, "credit": v[0], "debit": v[1]} for u, v in balances.items()])
limit = request.args.get("limit", type=int)
offset = request.args.get("offset", type=int)
descending = request.args.get("descending", False, type=bool)
sortBy = request.args.get("sortBy", type=str)
balances, count = balance_controller.get_balances(limit=limit, offset=offset, descending=descending, sortBy=sortBy)
return jsonify(
{"balances": [{"userid": u, "credit": v[0], "debit": v[1]} for u, v in balances.items()], "count": count}
)

View File

@ -69,7 +69,10 @@ def list_users(current_session):
JSON encoded array of `flaschengeist.models.user.User` or HTTP error
"""
logger.debug("Retrieve list of all users")
users = userController.get_users()
userids = None

Wird die Änderung eigentlich irgendwo verwendet?
Die widerspricht dem bisherigen REST Ansatz, daher entweder einzelner Datensatz (/user/xy) oder einem Block (/users bez. /users?limit...&offset...).

Ich kann mir denken wofür die gedacht ist, aber ich bin mir nicht so sicher ob das sinnvoll ist.

Wird die Änderung eigentlich irgendwo verwendet? Die widerspricht dem bisherigen REST Ansatz, daher entweder einzelner Datensatz (`/user/xy`) oder einem Block (`/users` bez. `/users?limit...&offset...`). Ich kann mir denken wofür die gedacht ist, aber ich bin mir nicht so sicher ob das sinnvoll ist.

Wird im Plugin verwendet. Damit braucht man nicht mehr alle Users laden (was im übrigen irgendwann ja sehr viele sein werden) sondern nur ein Bruchteil direkt über die userids.

Wird im Plugin verwendet. Damit braucht man nicht mehr alle Users laden (was im übrigen irgendwann ja sehr viele sein werden) sondern nur ein Bruchteil direkt über die userids.

guckst du hier

guckst du [hier](https://flaschengeist.dev/Flaschengeist/flaschengeist-frontend/src/commit/ade6d06eb6df430ed842a10d2bc867c8371c866d/api/src/stores/user.ts#L40)
if "userids" in request.args:
userids = [x for x in request.args.get("userids").split(",") if x]
users = userController.get_users(userids=userids)
return jsonify(users)