Skip to content

Profile update menu #71

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

Closed
wants to merge 15 commits into from

Conversation

leddcode
Copy link
Contributor

Now a user able to:

  • Change full name
  • Change email
  • Change short user description ("about me")
  • Update profile picture

@@ -11,6 +13,10 @@ class User(Base):
username = Column(String, unique=True)
email = Column(String, unique=True)
password = Column(String)
full_name = Column(String)
description = Column(String, default="Happy new user!")
Copy link
Contributor

@ivarshav ivarshav Jan 17, 2021

Choose a reason for hiding this comment

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

Maybe set the default in a const ?
Also in other files in the PR

async def update_user_email(
request: Request, session=Depends(get_db)):

user = session.query(User).filter_by(id=1).first()
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should add a TODO to replace the id=1 part

async def update_user_fullname(
request: Request, session=Depends(get_db)):

user = session.query(User).filter_by(id=1).first()
Copy link
Contributor

Choose a reason for hiding this comment

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

Filter always id=1 not by user id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's still no user verification

schema.md Outdated
@@ -26,4 +28,6 @@
├── schema.md
└── tests
├── __init__.py
└── config.py
Copy link
Contributor

Choose a reason for hiding this comment

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

conftest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for paying attention

# Post new data
profile = profile_test_client.post(
'/profile/update_user_fullname', data=new_name_data)
assert profile.status_code == 302
Copy link
Contributor

Choose a reason for hiding this comment

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

Can use from starlette import status

)


def get_new_user():
Copy link
Member

Choose a reason for hiding this comment

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

Please change to "get_placeholder_user"

@@ -48,5 +50,16 @@
<script src="https://cdnjs.cloudflare.com/ajax/libs/Chart.js/2.9.4/Chart.min.js"
integrity="sha512-d9xgZrVZpmmQlfonhQUvTR7lMPtO7NkZMkA0ABN3PHCbKA5nqylQ/yWlFAyY6hYgdF1Qh6nYiuADWwKB4C2WSw=="
crossorigin="anonymous"></script>
<script>
Copy link
Member

Choose a reason for hiding this comment

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

Can you please move it to another JS file?



async def process_image(image, user):
img = Image.open(io.BytesIO(image))
Copy link
Member

Choose a reason for hiding this comment

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

Can we somehow close this handler?

session.close()

url = router.url_path_for("profile")
response = RedirectResponse(url=url, status_code=HTTP_302_FOUND)
Copy link
Member

Choose a reason for hiding this comment

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

You can directly return it


# Save to database
user.avatar = f"{user.username}{PICTURE_EXTENSION}"
print(user.avatar)
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 print

@leddcode leddcode changed the base branch from main to develop January 18, 2021 16:51
@leddcode leddcode requested a review from yammesicka January 18, 2021 17:29
@codecov-io
Copy link

Codecov Report

Merging #71 (3b4107c) into develop (185a8ab) will increase coverage by 63.14%.
The diff coverage is 98.19%.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop      #71       +/-   ##
============================================
+ Coverage    35.41%   98.56%   +63.14%     
============================================
  Files            3        4        +1     
  Lines           48      139       +91     
============================================
+ Hits            17      137      +120     
+ Misses          31        2       -29     
Impacted Files Coverage Δ
app/database/database.py 90.90% <75.00%> (+90.90%) ⬆️
app/main.py 94.73% <93.75%> (+13.78%) ⬆️
app/database/models.py 100.00% <100.00%> (+100.00%) ⬆️
app/routers/profile.py 100.00% <100.00%> (ø)

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 c370572...3da5ca1. Read the comment docs.

@leddcode leddcode closed this Jan 18, 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.

5 participants