feature/event privacy + fix eventview error #271
Conversation
app/routers/event.py
Outdated
| event = create_event(db=session, | ||
| title=title, | ||
| start=start, | ||
| end=end, | ||
| owner_id=owner_id, | ||
| privacy=privacy, | ||
| content=content, | ||
| location=location, | ||
| invitees=invited_emails, | ||
| category_id=category_id, | ||
| availability=availability) |
There was a problem hiding this comment.
create_events(
session, title, start, end, owner_id, privacy, content, location,
invited_emails, category_id, availability,
)
(less space, without repeating things, easy enough to see git diif on next addition)
There was a problem hiding this comment.
I don't know why this happened...
Almost for all of your comments are stuff I didn't actually change . It's from the prehooks that automatically formatted everything 😭
There was a problem hiding this comment.
Changed it back to the way it was.
There was a problem hiding this comment.
So, I know you said that it's preferable to not repeat things but as the function itself has some deafult values that sometimes are necessary or not I changed the formatting but did leave the explicit attributing of each variable to the parameters as I saw that otherwise this causes problems and misunderstanding in tests and so on.
It's less pretty yes, but leaves less room for mistakes and doesn't require to change the create_event function parameters order (especially parameters that are not related to my feature which could confuse or mess up other things)
app/internal/privacy.py
Outdated
| class PrivateEvent: | ||
| """Represents a private event to show a non-owner of private event""" | ||
| def __init__(self, start, end, owner_id) -> None: | ||
| self.title = PrivacyKinds.Private.name | ||
| self.start = start | ||
| self.end = end | ||
| self.privacy = PrivacyKinds.Private.name | ||
| self.content = PrivacyKinds.Private.name | ||
| self.owner_id = owner_id | ||
| self.location = PrivacyKinds.Private.name | ||
| self.color = Null | ||
| self.invitees = PrivacyKinds.Private.name | ||
| self.category_id = Null | ||
| self.emotion = Null |
There was a problem hiding this comment.
Why aren't we using the regular Event object from the ORM?
There was a problem hiding this comment.
A few reasons - as to not modify the original event to hide all the details, to not create an extra redundant instance in the database and also when using the Event instance there is a problem with an sa_instance attribute that created a lot of problems when I tried to create a copy of the original event.
In short I think that using this neutral object will prevent messing up and cluttering the database and provide an event element with only the relevant non private details.
There was a problem hiding this comment.
There's a major problem with such an object, 'cause we will need to keep it updated for any changes of the original event object.
Don't you create it from existing event anyway?
There was a problem hiding this comment.
Do you have a better suggestion ?
Originally I tried to create a copy of the original event but this resulted in a lot of errors as sqlalchemy adds an "sa_instance" attribute to the object, and when I tried to remove it only for the event copy that also cause a lot of errors. I researched this a lot and found that this is the way others solved these issues- by creating another object to represent only the values we need without a connection to the database.
Is there some way that I could make sure this object will accept any future attributes that will be added to the Event table? Like maybe make it inherit from it ?
There was a problem hiding this comment.
Originally I tried to create a copy of the original event but this resulted in a lot of errors as sqlalchemy adds an "sa_instance" attribute to the object
Why is that a problem?
I think the best solution is to use the original Event instance.
If it is not possible, maybe we should inherit from it.
There was a problem hiding this comment.
ok I found out (lots of trial and error and additional research) how to overcome sqlalchemy weird additional attributes and create a copy of the event as an Event object, without saving it :)
will be in the commit I will soon upload
Codecov Report
@@ Coverage Diff @@
## develop #271 +/- ##
===========================================
+ Coverage 98.23% 98.25% +0.01%
===========================================
Files 70 72 +2
Lines 3010 3155 +145
===========================================
+ Hits 2957 3100 +143
- Misses 53 55 +2
Continue to review full report at Codecov.
|
…alendar into feature/event_privacy
| @@ -1,86 +0,0 @@ | |||
| <div class="form_row"> | |||
There was a problem hiding this comment.
Did you removed this on purpose?
There was a problem hiding this comment.
Well yes.
I noticed the file was moved to templates/partials/calendar/event so I thought this was left over accidentally as it wasn't being loaded from anywhere.
nonexisting_event -> raise_for_non_existing_event
3 levels of event privacy-
'Public' - can be shown by anyone
'Private' - for other users - The event start date end date and owner is shown all other details are 'private' or not shown
'Hidden' - can only be seen by owner, the event will not be shown to anyone other than the owner
small fix in eventview function - a '?' was missing between 'event_id' and 'messages=' in the routing link.