Flaschengeist/flaschengeist
Flaschengeist
/
flaschengeist
7
0
Fork 0
Code Issues 20 Pull Requests Packages Projects Releases 2 Wiki Activity

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
Conversation 1 Commits 3 Files Changed 3 +8 -20
4 changed files with 8 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit c3468eea03 - Show all commits

8
flaschengeist/controller/userController.py
View File

@ -118,12 +118,8 @@ def modify_user(user, password, new_password=None):
messageController.send_message(messageController.Message(user, text, subject)) messageController.send_message(messageController.Message(user, text, subject))
def get_users(userids=None): def get_users():
ferfissimo commented 2021-11-22 15:06:25 +00:00
Outdated
Review
Copy Link

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)
crimsen commented 2021-11-22 22:23:31 +00:00
Outdated
Review
Copy Link

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 return User.query.all()
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): def get_user_by_role(role: Role):

5
flaschengeist/models/user.py
View File

@ -104,11 +104,6 @@ class User(db.Model, ModelSerializeMixin):
def has_permission(self, permission): def has_permission(self, permission):
return permission in self.get_permissions() return permission in self.get_permissions()
def __repr__(self):
return (
f"User({self.userid}, {self.firstname}, {self.lastname}, {self.mail}, {self.display_name}, {self.birthday})"
)
ferfissimo commented 2021-11-22 14:49:38 +00:00
Outdated
Review
Copy Link

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).
crimsen commented 2021-11-22 22:30:11 +00:00
Outdated
Review
Copy Link

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)
class _UserAttribute(db.Model, ModelSerializeMixin): class _UserAttribute(db.Model, ModelSerializeMixin):
__tablename__ = "user_attribute" __tablename__ = "user_attribute"

10
flaschengeist/plugins/balance/routes.py
View File

@ -110,10 +110,7 @@ def limits(current_session: Session):
Returns: Returns:
JSON encoded array of userid with limit or HTTP-error JSON encoded array of userid with limit or HTTP-error
""" """
userids = None users = userController.get_users()
if "userids" in request.args:
[x for x in request.args.get("userids").split(",") if x]
users = userController.get_users(userids=userids)
if request.method == "GET": if request.method == "GET":
ferfissimo commented 2021-11-22 15:02:30 +00:00
Outdated
Review
Copy Link

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

Siehe unten: https://flaschengeist.dev/Flaschengeist/flaschengeist/pulls/17/files#issuecomment-164
return jsonify([{"userid": user.userid, "limit": user.get_attribute("balance_limit")} for user in users]) return jsonify([{"userid": user.userid, "limit": user.get_attribute("balance_limit")} for user in users])
@ -319,5 +316,8 @@ def get_balances(current_session: Session):
sortBy = request.args.get("sortBy", type=str) sortBy = request.args.get("sortBy", type=str)
balances, count = balance_controller.get_balances(limit=limit, offset=offset, descending=descending, sortBy=sortBy) balances, count = balance_controller.get_balances(limit=limit, offset=offset, descending=descending, sortBy=sortBy)
return jsonify( return jsonify(
{"balances": [{"userid": u, "credit": v[0], "debit": v[1]} for u, v in balances.items()], "count": count} {
"balances": [{"userid": u, "credit": v[0], "debit": v[1]} for u, v in balances.items()],
"count": count,
}
) )

5
flaschengeist/plugins/users/__init__.py
View File

@ -69,10 +69,7 @@ def list_users(current_session):
JSON encoded array of `flaschengeist.models.user.User` or HTTP error JSON encoded array of `flaschengeist.models.user.User` or HTTP error
""" """
logger.debug("Retrieve list of all users") logger.debug("Retrieve list of all users")
userids = None users = userController.get_users()
ferfissimo commented 2021-11-22 15:01:48 +00:00
Outdated
Review
Copy Link

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.
crimsen commented 2021-11-22 22:27:26 +00:00
Outdated
Review
Copy Link

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.
crimsen commented 2021-11-22 22:39:15 +00:00
Outdated
Review
Copy Link

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) return jsonify(users)