GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — feat: Add avatars by orronai · Pull Request #331 · PythonFreeCourse/lms

Conversation

@orronai

@orronai orronai commented Oct 10, 2021

Copy link
Copy Markdown
Collaborator
  • Added avatars to users tables
  • Added a template in order to change or delete avatar
  • Added the form and validators for the picture file
  • Added tests
  • Added translations
  • Added to the navbar near the username the avatar (if the user has one)
  • Added to gitignore the avatars

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@orronai
feat: Add avatars
c3e3d66 
- Added avatars to users tables
- Added a template in order to change or delete avatar
- Added the form and validators for the picture file
- Added tests
- Added translations
- Added to the navbar near the username the avatar (if the user has one)
- Added to gitignore the avatars
@orronai orronai linked an issue Oct 10, 2021 that may be closed by this pull request
Add avatars #330
Open
@sourcery-ai sourcery-ai Bot mentioned this pull request Oct 10, 2021
feat: Add avatars (Sourcery refactored) #332
Merged
sourcery-ai Bot and others added 2 commits October 10, 2021 18:07
@sourcery-ai
'Refactored by Sourcery' (#332)
d50bfda 
Co-authored-by: Sourcery AI <>
@orronai
Added the mkdir of the avatars path if it doesn't exist
657715e
@codecov

codecov Bot commented Oct 10, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.02564% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.34%. Comparing base (9499e89) to head (3052f0b).
⚠️ Report is 39 commits behind head on master.

Files with missing lines Patch % Lines
lms/lmsdb/bootstrap.py 20.00% 4 Missing ⚠️
lms/models/users.py 92.59% 2 Missing ⚠️
lms/lmsweb/views.py 96.96% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #331      +/-   ##
==========================================
+ Coverage   84.18%   84.34%   +0.15%     
==========================================
  Files          63       64       +1     
  Lines        2953     3027      +74     
==========================================
+ Hits         2486     2553      +67     
- Misses        467      474       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
yammesicka
yammesicka requested changes Oct 10, 2021
View reviewed changes
Comment thread .gitignore Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lms/lmsdb/models.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lms/lmsweb/forms/update_avatar.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lms/lmsweb/translations/he/LC_MESSAGES/messages.po Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lms/lmsweb/views.py
return render_template('update-avatar.html', form=form)


@webapp.route('/avatar/delete')

@yammesicka yammesicka Oct 10, 2021

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If you can make it work with the DELETE http verb it'll be awesome. If not, that's also good

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@orronai orronai Oct 11, 2021

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

for delete http verb we can't just use html - we need also JS functions so I think it'd be more complicated. In addition - because of the implementation of this delete function - we redirect to the users page which is a get method according to my opinion

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@yammesicka yammesicka Oct 15, 2021

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yeah that's PITA. Agree

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lms/models/users.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lms/templates/navbar.html Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread tests/test_users.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread tests/test_users.py

@staticmethod
def open_file(filename: str) -> BufferedReader:
return open(Path(conftest.SAMPLES_DIR) / filename, 'br')

@yammesicka yammesicka Oct 10, 2021

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

You can do it with fixture: use file_handler = open(...), then yield file_handler and finally file_handler.close it

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread tests/test_users.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
orronai and others added 11 commits October 11, 2021 15:41
@orronai
- Merged with master
215b341 
- Fixed translations
- Changed avatars directory
- Added constant of MB conversion
@orronai
Merge branch 'master' of https://github.com/PythonFreeCourse/lms into…
d2d51e7 
… edit-profile
@orronai
Image saving proccess is now async
7f44c03
@orronai
fixed translations
ddd195f
@orronai
fixed asyncio test
1fb34b9
@orronai
fixed asyncio test
2314d37
@orronai
fixed asyncio test
997899f
@orronai
fixed asyncio test
b1de37a
@orronai
fix merge conflicts
791fd06
@orronai
Merge branch 'master' into edit-profile
f647f9e
@orronai
removed unnecessary line
3052f0b
@sourcery-ai

sourcery-ai Bot commented Oct 19, 2021

Copy link
Copy Markdown

Sourcery Code Quality Report

✅  Merging this PR will increase code quality in the affected files by 0.20%.

Quality metrics Before After Change
Complexity 1.67 ⭐ 1.64 ⭐ -0.03 👍
Method Length 36.29 ⭐ 36.27 ⭐ -0.02 👍
Working memory 6.56 🙂 6.54 🙂 -0.02 👍
Quality 81.75% ⭐ 81.95% ⭐ 0.20% 👍
Other metrics Before After Change
Lines 2461 2593 132
Changed files Quality Before Quality After Quality Change
lms/lmsdb/bootstrap.py 81.17% ⭐ 81.43% ⭐ 0.26% 👍
lms/lmsdb/models.py 84.12% ⭐ 84.12% ⭐ 0.00%
lms/lmsweb/init.py 64.84% 🙂 59.70% 🙂 -5.14% 👎
lms/lmsweb/views.py 75.40% ⭐ 75.73% ⭐ 0.33% 👍
lms/models/users.py 89.57% ⭐ 90.99% ⭐ 1.42% 👍
lms/utils/consts.py 90.75% ⭐ 90.29% ⭐ -0.46% 👎
tests/conftest.py 90.98% ⭐ 91.05% ⭐ 0.07% 👍
tests/test_users.py 83.50% ⭐ 84.31% ⭐ 0.81% 👍

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
lms/lmsweb/views.py comment 13 🙂 202 😞 8 🙂 50.15% 🙂 Try splitting into smaller methods
lms/lmsdb/models.py Solution.status 1 ⭐ 118 🙂 17 ⛔ 55.65% 🙂 Extract out complex expressions
lms/lmsweb/views.py view 8 ⭐ 140 😞 10 😞 56.16% 🙂 Try splitting into smaller methods. Extract out complex expressions
lms/lmsweb/views.py login 5 ⭐ 129 😞 12 😞 56.69% 🙂 Try splitting into smaller methods. Extract out complex expressions
lms/lmsdb/models.py Solution._base_next_unchecked 0 ⭐ 95 🙂 20 ⛔ 57.42% 🙂 Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.

Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@orronai
orronai requested a review from yammesicka October 19, 2021 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yammesicka yammesicka Awaiting requested review from yammesicka

Requested changes must be addressed to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add avatars

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@orronai @yammesicka
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.