Feature/calendar privacy #247
Conversation
| @@ -42,6 +42,7 @@ class User(Base): | |||
| avatar = Column(String, default="profile.png") | |||
| telegram_id = Column(String, unique=True) | |||
| is_active = Column(Boolean, default=False) | |||
| privacy = Column(String, default="Private", nullable=False) | |||
There was a problem hiding this comment.
If the only values you expect are "private" and "public", consider using boolean type - True ( for private) and Falsr (for public).
There was a problem hiding this comment.
Originally the feature was planned to contain "friends" as a level of privacy.
This feature has yet to be approved or implemented but I think a room for this option and other levels of privacy can be allowed
There was a problem hiding this comment.
As this is not a free text field but rather something from a set of closed options in a dropdown I don't think there is any reason to fear an incorrect value here
Also I am not sure whether in the setting here the solution would be Enum rather than another connecting table like they did in language. In any case I dont think that it is relevant in the situation here right now
There was a problem hiding this comment.
I think that it's a good practice to add a validation also in backend and in db.
Also, I think that you can get better performance with this option (but I guess we are not worried about that..).
I agree that right now it's too much to add another table.
Please make sure that it's invalid to add a bad value, even if someone tries :)
There was a problem hiding this comment.
where would you suggest doing this? I didn't see that anyone did something similar I think... It can't be in the models file obviously.. and as I mentioned no one can enter values I did not specify in the drropdown field, it's not a free text where anyone can enter what they want, just a set of options. Does that not qualify as verifying that only possible values can be entered?
There was a problem hiding this comment.
so anything? or can I resolve this? :)
…i/calendar into feature/calendar_privacy
Codecov Report
@@ Coverage Diff @@
## develop #247 +/- ##
===========================================
- Coverage 99.06% 99.05% -0.02%
===========================================
Files 48 53 +5
Lines 2251 2431 +180
===========================================
+ Hits 2230 2408 +178
- Misses 21 23 +2
Continue to review full report at Codecov.
|
app/templates/profile.html
Outdated
| @@ -54,7 +60,7 @@ | |||
| <div class="modal-dialog profile-modal-dialog profile-modal-fadeIn"> | |||
| <div class="modal-content"> | |||
| <div class="modal-header profile-modal-header"> | |||
| <h5 class="modal-title">{{ gettext("Update name") }}</h5> | |||
| <h1 class="modal-title">{{ gettext("Update name") }}</h1> | |||
There was a problem hiding this comment.
There should be only single h1 in each page. You can lots of h2s under a single h1
There was a problem hiding this comment.
well there is no h1 in the entire profile page but I changed them all to h2 if that's alright. They are all in the same level
| data = await request.form() | ||
| new_privacy = data['privacy'] | ||
|
|
||
| # Update database |
There was a problem hiding this comment.
you can delete the comment
There was a problem hiding this comment.
This comment appears in all the other forms so I just kept the same structure :) Unless a decision is made to remove the comments from all the file I think this should stay
| db=session, current_user=current_user | ||
| ) | ||
| assert result_a is False | ||
| assert result_b is True |
There was a problem hiding this comment.
you can write it down as
assert result_b
There was a problem hiding this comment.
I got a previous comment saying to do the opposite haha ... let's just stay with this:)
Add privacy setting to the user's calendar.
https://forums.pythonic.guru/t/topic/8757