Skip to content

Feature/notifications #331

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

Merged
merged 27 commits into from
Feb 24, 2021

Conversation

IdanPelled
Copy link
Contributor

Notification page:

The notification page binds all the messages and the invitations the user received.
to create a new message use from app.routers.notification import create_message.
* bonus: create_message can get an extra parameter (link), that turns the message into a link.

Possible useses:

  • Notify the user about a change in the meeting
  • System updates

image

@IdanPelled IdanPelled changed the base branch from main to develop February 23, 2021 10:10
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.

Awesome job! Very nice.
I've added few insights about this PR, please take a look.

username: str
email: str
full_name: str

# TODO: Add language_id field
Copy link
Member

Choose a reason for hiding this comment

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

Remove the TODO

message_id: int,
session: Session,
) -> Union[Message, None]:
"""Returns an invitation by an id.
Copy link
Member

Choose a reason for hiding this comment

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

Leave blank line after the summary line.
The closing """ should be in their own line.

See PEP 257 for further details.

MessageStatusEnum.UNREAD,
]

ARCHIVED = [
Copy link
Member

Choose a reason for hiding this comment

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

You check if things are in ARCHIVED/UNREAD_STATUS- prefer{to create aset`

func: Union[_is_unread, _is_archived],
) -> List[NOTIFICATION_TYPE]:
"""Filters notifications by "func"."""
return list(filter(func, get_all_notifications(session, user_id)))
Copy link
Member

Choose a reason for hiding this comment

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

yield from instead list to imitate the behavior of real filter

def filter_notifications(
session: Session,
user_id: int,
func: Union[_is_unread, _is_archived],
Copy link
Member

Choose a reason for hiding this comment

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

_is_unread and is_archived are not a type. Use Callable. See PEP 484 or mypy documentations for more details.
Prefer validating your code using mypy.

Comment on lines 7 to 13
get_invitation_by_id,
get_unread_notifications,
get_all_messages,
get_message_by_id,
get_archived_notifications,
raise_wrong_id_error,
is_owner,
Copy link
Member

Choose a reason for hiding this comment

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

abc

@@ -0,0 +1,188 @@
from fastapi import APIRouter, Depends, Request, Form
Copy link
Member

Choose a reason for hiding this comment

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

abc

from sqlalchemy.orm import Session

from app.database.models import MessageStatusEnum
from app.dependencies import templates, get_db
Copy link
Member

Choose a reason for hiding this comment

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

abc

@@ -1,30 +1,37 @@
from typing import Generator, Iterator
from typing import Generator, Iterator, Dict
Copy link
Member

Choose a reason for hiding this comment

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

ABC

Comment on lines 3 to 5
import pytest
from fastapi.testclient import TestClient
from sqlalchemy.orm import Session
Copy link
Member

Choose a reason for hiding this comment

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

ABC

@yammesicka yammesicka merged commit b4b0ede into PythonFreeCourse:develop Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants