Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/user notes #120

Open
wants to merge 2 commits into
base: master
from
Open

Feature/user notes #120

wants to merge 2 commits into from

Conversation

@IdoElk
Copy link
Member

IdoElk commented Jun 15, 2020

Add user notes, backend only!

  • Needs another frontend PR
@IdoElk IdoElk added the Backend label Jun 15, 2020
@IdoElk IdoElk requested a review from yammesicka Jun 15, 2020
@IdoElk IdoElk linked an issue that may be closed by this pull request Jun 15, 2020
note = request.json.get('note')
user = User.get_or_none(request.json.get('user_id', 0))

if not act or not user or note is None:

This comment has been minimized.

@yammesicka

yammesicka Jun 17, 2020

Member

Remove "not act" to get the right error message

return fail(400, 'Invalid user_id or note')

if act == 'fetch':
return jsonify({user.note})

This comment has been minimized.

@yammesicka

yammesicka Jun 17, 2020

Member

Will it blend?
(Srsly though, you can just put user.note in there? Have you checked the API using Postman [or any similar tool]? What about the key?)

@@ -93,6 +93,7 @@ class User(UserMixin, BaseModel):
mail_address = CharField(unique=True)
password = CharField()
role = ForeignKeyField(Role, backref='users')
note = CharField(max_length=4096, default='')

This comment has been minimized.

@yammesicka

yammesicka Jun 17, 2020

Member

It should be many to one - each user should many notes (Notes -> User)

This comment has been minimized.

@IdoElk

IdoElk Jun 17, 2020

Author Member

This was my initial thinking but then I was thinking about our actual usage.
Wouldn't we want to have a general note for a specific user? or am I missing the usecase?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.