Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Feature/public event #287

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

Open
noam-y wants to merge 41 commits into PythonFreeCourse:develop
base: develop
Choose a base branch
Loading
from noam-y:feature/public_event

Conversation

Copy link
Contributor

@noam-y noam-y commented Feb 14, 2021

a feature that supports describing an event as a public event. as a part of this definition, it will allow sending emails to all event participants and allow joining events on the event page.

noam-y added 13 commits February 11, 2021 14:48 
...public attr to event model, a function that adds a user to existing event and test for that function.
...ntent given as a var, and also a func that sends email to all participants in event.
...t creation. fixing tests. tests are not working properly yet!
@noam-y noam-y changed the base branch from main to develop February 14, 2021 22:26
@@ -61,8 +61,8 @@ class Event(Base):
color = Column(String, nullable=True)
availability = Column(Boolean, default=True, nullable=False)
invitees = Column(String)
is_public = Column(Boolean, default=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Event doesn't have "friends" option?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the meaning of friends is different- public event is meant to allow any user in the platform to join the event, just like the public events on facebook. so it means the owner's friends don't matter in this feature :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explain what is the meaning of public/private event? is it like busy/free?
Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a pubic event is event anyone can join. it means you don't need to be invited directly from the owner, you can join the event by yourself by clicking a button on event page



@pytest.fixture
def new_user(session):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you can use existing fixtures?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried but didn't really find a proper user fixture inside the file. ill try going over the code again

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are enough user fixtures, use one of the others.

Copy link

codecov-io commented Feb 19, 2021 •
edited
Loading

Codecov Report

Merging #287 (71ee772) into develop (1048956) will decrease coverage by 0.38%.
The diff coverage is 56.00%.

Impacted file tree graph 

@@ Coverage Diff @@
## develop #287 +/- ##
===========================================
- Coverage 98.01% 97.63% -0.39% 
===========================================
 Files 68 68 
 Lines 2979 3002 +23 
===========================================
+ Hits 2920 2931 +11 
- Misses 59 71 +12
Impacted Files Coverage Δ
app/routers/email.py 100.00% <ø> (ø)
app/internal/email.py 84.00% <26.66%> (-16.00%) ⬇️
app/database/models.py 96.73% <100.00%> (+0.03%) ⬆️
app/routers/event.py 96.94% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1048956...9cf15b1. Read the comment docs.

Copy link
Member

@yammesicka yammesicka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! Please see my insights, and use precommit hooks to format your code automatically :)

Copy link
Member

@yammesicka yammesicka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the problems in this version could be resolved easily using git precommit hooks.
Please run black or git precommit hooks on this code and I'll check it again :)

templates,
)
from app.database.models import Event, User, UserEvent
from app.internal.utils import get_current_user
Copy link
Member

@yammesicka yammesicka Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use Kobi's user system instead

Comment on lines 88 to 99
@pytest.fixture
def no_event_user(session):
"""a user made for testing who doesn't own any event."""
user = create_user(
session=session,
username='new_test_username',
password='new_test_password',
email='new2_test.email@gmail.com',
language_id='english'
)

return user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't create a new fixture, use one of the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have to use several different users since i'm testing the use of the mailing list.

Comment on lines 102 to 124
@pytest.fixture
def event_owning_user(session):
"""a user made for testing who already owns an event."""
user = create_user(
session=session,
username='new_test_username2',
password='new_test_password2',
email='new_test_love231.email@gmail.com',
language_id='english'
)

data = {
'title': 'event_owning_user event',
'start': datetime.strptime('2021年05月05日 14:59', '%Y-%m-%d %H:%M'),
'end': datetime.strptime('2021年05月05日 15:01', '%Y-%m-%d %H:%M'),
'location': 'https://us02web.zoom.us/j/875384596',
'content': 'content',
'owner_id': user.id,
}

create_event(session, **data)

return user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't create a new fixture, use one of the others. You can use a user fixture and an event fixture in your code to create this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have to use several different users since i'm testing the use of the mailing list.

__

Comment on lines 127 to 137
@pytest.fixture
def user1(session):
"""another user made for testing"""
user = create_user(
session=session,
username='user2user2',
password='verynicepass',
email='trulyyours1.email@gmail.com',
language_id='english'
)
return user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't create a new user fixture, use one of the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have to use several different users since i'm testing mailing list.

Comment on lines 140 to 152
@pytest.fixture
def event_example(session, event_owning_user):
data = {
'title': 'test event title',
'start': datetime.strptime('2021年05月05日 14:59', '%Y-%m-%d %H:%M'),
'end': datetime.strptime('2021年05月05日 15:01', '%Y-%m-%d %H:%M'),
'location': 'https://us02web.zoom.us/j/87538459r6',
'content': 'content',
'owner_id': event_owning_user.id,
}

event = create_event(session, **data)
return event
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't create a new event fixture, use one of the others.



@pytest.fixture
def new_user(session):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are enough user fixtures, use one of the others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yammesicka yammesicka yammesicka requested changes

+2 more reviewers

@ivarshav ivarshav ivarshav left review comments

@Gonzom Gonzom Gonzom left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /