Skip to content
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

Edit achievements #437

Merged
merged 29 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4c58a85
edit_achievements
AleksandrKosmylev Jun 15, 2024
1dd4735
edit_achievements_addition
AleksandrKosmylev Jun 15, 2024
be4c7d1
Added common statistics in footer
AleksandrKosmylev Jun 18, 2024
df93d1a
Merge branch 'Hexlet:main' into edit_achievements
AleksandrKosmylev Jun 18, 2024
a68db2e
add test for contributors:contributor_achievements and minor linter fix
AleksandrKosmylev Jun 19, 2024
3b4a8fa
Merge remote-tracking branch 'origin/edit_achievements' into edit_ach…
AleksandrKosmylev Jun 20, 2024
883c439
Add helper function to calc achivements percentage
AleksandrKosmylev Jun 30, 2024
c43a264
fix inline styles
AleksandrKosmylev Jun 30, 2024
8ddf4bb
update common and personal stat
AleksandrKosmylev Aug 18, 2024
23abc8c
resolve conflict
AleksandrKosmylev Oct 6, 2024
eb11834
resolve wps
AleksandrKosmylev Oct 6, 2024
3849707
resolve wps editon 1
AleksandrKosmylev Oct 6, 2024
f9c26ed
resolve wps editon 2
AleksandrKosmylev Oct 6, 2024
fa49f24
resolve wps editon 3
AleksandrKosmylev Oct 6, 2024
a068fe8
resolve wps editon 4
AleksandrKosmylev Oct 6, 2024
439944b
resolve wps editon 6
AleksandrKosmylev Oct 6, 2024
be1689c
resolve wps editon 7
AleksandrKosmylev Oct 6, 2024
023b76d
Merge branch 'Hexlet:main' into edit_achievements
AleksandrKosmylev Oct 6, 2024
eeaa715
Merge pull request #467 from Hexlet/fix-views
sgmdlt Oct 16, 2024
0b2574e
Update CI.yml
sgmdlt Oct 16, 2024
9568aeb
Merge branch 'Hexlet:main' into main
AleksandrKosmylev Oct 23, 2024
91289fb
fix linter notifications
AleksandrKosmylev Oct 27, 2024
130b868
fix linter notifications
AleksandrKosmylev Oct 27, 2024
4b3067c
refactor contributor_achievements.py
AleksandrKosmylev Nov 17, 2024
8c312f8
delete sum brackets
AleksandrKosmylev Nov 17, 2024
aa74637
Merge branch 'Hexlet:main' into main
AleksandrKosmylev Dec 11, 2024
1973456
Merge branch 'Hexlet:main' into main
AleksandrKosmylev Feb 6, 2025
d1aa574
CHECK differences
AleksandrKosmylev Feb 6, 2025
5b97a44
Resolve conflicts
AleksandrKosmylev Feb 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions contributors/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@
views.achievements.AchievementListView.as_view(),
name='achievements',
),
path(
'contributor_achievements/<slug:slug>',
views.contributor_achievements.ContributorAchievementListView.as_view(), # noqa: E501
name='contributor_achievements',
),
path(
'landing/',
views.landing.LandingView.as_view(),
Expand Down
1 change: 1 addition & 0 deletions contributors/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
about,
achievements,
config,
contributor_achievements,
contributor_compare,
filters,
home,
Expand Down
109 changes: 109 additions & 0 deletions contributors/views/contributor_achievements.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
from django.db.models import Count, Q, Sum # noqa: WPS235, WPS347
Copy link
Collaborator

Choose a reason for hiding this comment

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

Здесь снова используется игнорирование правил. Вообще от WSP в Хекслете мы отказались ,поэтому и в проекте его можно убрать.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

пока убирал свои noqa увидел,что во многих местах есть строки закомментированные с A,WPS.
Не убирал в тех местах, которые не касаются моего изменения

Copy link
Collaborator

Choose a reason for hiding this comment

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

ага, тогда можете создать ищщьюс с задачкой почистить от игноров подобные штуки. Затыкать линтер плохая идея, а если он массово затыкается, то может быть это правило не очень нужно (или следует другой линтер использовать)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

добавлю новый issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Добавил

from django.db.models.functions import Coalesce
from django.views import generic

from contributors.models import Contributor, Repository

ID = 'id'


class ContributorAchievementListView(generic.ListView):
"""Achievement list."""

template_name = 'contributor/contributor_achievements_list.html'
model = Contributor
contributors = Contributor.objects.with_contributions()

pull_request_ranges_for_achievements = [100, 50, 25, 10, 1]
commit_ranges_for_achievements = [200, 100, 50, 25, 1]
issue_ranges_for_achievements = [50, 25, 10, 5, 1]
comment_ranges_for_achievements = [200, 100, 50, 25, 1]
edition_ranges_for_achievements = [1000, 500, 250, 100, 1]

def get_context_data(self, **kwargs):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Я бы разделил метод на несколько. Сейчас это огромная стена кода. Выделите подметоды, сделайте их приватными (_method_name), разделите логику.

"""Add context data for achievement list."""
self.contributors_amount = Contributor.objects.count()
context = super().get_context_data(**kwargs)
contributors = Contributor.objects.with_contributions()
current_contributor = (
Contributor.objects.get(login=self.kwargs['slug'])
)

repositories = Repository.objects.select_related(
'organization',
).filter(
is_visible=True,
contribution__contributor=current_contributor,
).annotate(
commits=Count('id', filter=Q(contribution__type='cit')),
additions=Coalesce(Sum('contribution__stats__additions'), 0),
deletions=Coalesce(Sum('contribution__stats__deletions'), 0),
pull_requests=Count(
'contribution', filter=Q(contribution__type='pr'),
),
issues=Count('contribution', filter=Q(contribution__type='iss')),
comments=Count('contribution', filter=Q(contribution__type='cnt')),
).order_by('organization', 'name')

contributions = repositories.values().aggregate(
contributor_deletions=Sum('deletions'),
contributor_additions=Sum('additions'),
contributor_commits=Sum('commits'),
contributor_pull_requests=Sum('pull_requests'),
contributor_issues=Sum('issues'),
contributor_comments=Sum('comments'),
)

context['commits'] = contributions['contributor_commits']
context['pull_requests'] = contributions['contributor_pull_requests']
context['issues'] = contributions['contributor_issues']
context['comments'] = contributions['contributor_commits']
context['total_editions'] = (
contributions['contributor_additions'] + contributions['contributor_deletions'] # noqa: E501
)
context['total_actions'] = sum(contributions.values())
context['pull_request_ranges_for_achievements'] = (
self.pull_request_ranges_for_achievements
)
context['current_contributor'] = current_contributor
context['contributors_amount'] = self.contributors_amount
context['contributors_with_any_contribution'] = (
contributors.filter(contribution_amount__gte=1).count()
)

# Pull request achievements:
for pr_num in self.pull_request_ranges_for_achievements:
context[f'contributor_pull_requests_gte_{pr_num}'] = pr_num
context[f'contributors_pull_requests_gte_{pr_num}'] = (
contributors.filter(pull_requests__gte=pr_num).count()
)

# Commit achievements:
for commit_num in self.commit_ranges_for_achievements:
context[f'contributor_commits_gte_{commit_num}'] = commit_num
context[f'contributors_commits_gte_{commit_num}'] = (
contributors.filter(commits__gte=commit_num).count()
)

# Issue achievements:
for issue_num in self.issue_ranges_for_achievements:
context[f'contributor_issues_gte_{issue_num}'] = issue_num
context[f'contributors_issues_gte_{issue_num}'] = (
contributors.filter(issues__gte=issue_num).count()
)

# Comment achievements:
for comment_num in self.comment_ranges_for_achievements:
context[f'contributor_comments_gte_{comment_num}'] = comment_num
context[f'contributors_comments_gte_{comment_num}'] = (
contributors.filter(comments__gte=comment_num).count()
)

# Edition achievements:
for ed_num in self.edition_ranges_for_achievements:
context[f'contributor_editions_gte_{ed_num}'] = ed_num
context[f'contributors_editions_gte_{ed_num}'] = (
contributors.filter(editions__gte=ed_num).count()
)

return context
37 changes: 26 additions & 11 deletions templates/components/footer.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,22 @@
</ul>
</div>
<div class="col">
<p class="h6">{% trans "Other projects" %}</p>
<ul class="list-unstyled my-0">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Вообще тут получилось очень страшно, на будущее его можно переделать.
Списки с дивами не используются. Если у вас список, то между ul > li нет блока. Можете посмотреть, как выглядит футер в код бейзиксе или в самом Хекслете.
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

можете завести здесь ищщус на привести футер в порядок

Copy link
Contributor Author

Choose a reason for hiding this comment

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

добавлю новый issue

<div class="row">
<div class="col">
<li><p class="h6">{% trans "Other projects" %}</p></li>
</div>
<div class="col">
<li> </li>
Copy link
Collaborator

Choose a reason for hiding this comment

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

а зачем здесь пустой элемент?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Вот это не поправлено.

Copy link
Contributor Author

@AleksandrKosmylev AleksandrKosmylev Jun 30, 2024

Choose a reason for hiding this comment

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

элемент списка специально выполнен пустым для того чтобы сохранить оформление

</div>
<div class="col">
<li><p class="h6">{% trans "Useful resources" %}</p></li>
</div>
<div class="col">
<li><a target="_blank" href="{% url 'contributors:achievements' %}">{% trans "Common statistics" %}</a></li>
</div>
</div>
</ul>
<hr>
<ul class="list-unstyled my-0">
<div class="row">
Expand All @@ -22,18 +37,18 @@
<li><a target="_blank" href="http://codebattle.hexlet.io">Codebattle</a></li>
<li><a target="_blank" href="http://code-basics.com">Code Basics</a></li>
</div>
<div class="col">
<li><a target="_blank" href="http://guides.hexlet.io">Hexlet Guides</a></li>
<li><a target="_blank" href="http://ru.hexlet.io/knowledge">{% trans "Knowledge base" %}</a></li>
<li><a target="_blank" href="http://ru.hexlet.io/pages/recommended-books">{% trans "Recommended books" %}</a></li>
<li><a target="_blank" href="http://ru.hexlet.io/blog">{% trans "Blog" %}</a></li>
</div>
<div class="col">
<li> </li>
</div>
</div>
</ul>
</div>
<div class="col">
<p class="h6">{% trans "Useful resources" %}</p>
<hr>
<ul class="list-unstyled my-0">
<li><a target="_blank" href="http://guides.hexlet.io">Hexlet Guides</a></li>
<li><a target="_blank" href="http://ru.hexlet.io/knowledge">{% trans "Knowledge base" %}</a></li>
<li><a target="_blank" href="http://ru.hexlet.io/pages/recommended-books">{% trans "Recommended books" %}</a></li>
<li><a target="_blank" href="http://ru.hexlet.io/blog">{% trans "Blog" %}</a></li>
</ul>

</div>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{% load i18n static mathfilters %}

<div class="progress d-flex justify-content-between" style="height: max-content;">
Copy link
Collaborator

Choose a reason for hiding this comment

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

здесь тоже какой-то кастомный styel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

перенесено в base.css как кастомный стиль.

<div class="bg-info text-dark text-start aligns-items-center" role="progressbar" style="width: {{ contribution|div:achievement_made_count|mul:100|floatformat:0 }}%;" aria-valuenow="25" aria-valuemin="0" aria-valuemax="100">
Copy link
Collaborator

Choose a reason for hiding this comment

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

здес стиль используется чтобы задать прогерсс бар?
Можно ли расчет width засунуть в хелпер-функцию и здесь просто вызывать?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

здес стиль используется чтобы задать прогерсс бар? Можно ли расчет width засунуть в хелпер-функцию и здесь просто вызывать?

  • Да , style="width" отвечает за заполнение полосы прогресса
    https://getbootstrap.com/docs/5.0/components/progress/

  • процедура получения данных для таблицы и сама таблица c личными достижениями сделана по аналогии с achievements.py(было уже разработано раньше), где получали общую статистику.
    Непонятно про функцию хелпер: функция должна быть реализована во view и передана в шаблон, где будет получать аргументы для вычисления процента выполнения?

Copy link
Collaborator

Choose a reason for hiding this comment

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

вот насчет джанги не знаю, в других фреймворках (рельсе, ларавел) хелпер обьявляется где-то в модулях приложения и используется во вью.
Если у вас есть другой шаблон, где абсолютно та же логика, но немного другие данные, а вычисления те же - то скорее всего это одна и та же абстракция.
В теории тут можно по всем ачивкам пройтись и сформировать данные для отображения, тогда вы все готовите и передаете в шаблон, который будет слегка чище.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Единственный вариант реализации функции-хелпера, который придумал через @register.simple_tag :

@register.simple_tag
def calc_percent_achievement(numerator, denominator):
"""Get contributor statistics and required quantity."""
if numerator is not None:
if numerator / denominator > 1:
return 100.0
return numerator / denominator * 100
return 0

размеры кода остаются те же. объем кода при этом решении почти не сократился.

<dl class="p-2 fs-6 mb-0" style="width:50vw;">
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут какой-то скастомный стиль используется. Давайте от такого избавляться (он инлайн стилей). Посмотрите, есть ли в бутстрапе похожий класс, если нет, то можно добавить кастомный класс, который именуется через префикс x-*, возможно есть другие кастомные классы уже, можно на них посмотреть.

Copy link
Collaborator

Choose a reason for hiding this comment

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

А тут не поправили? Посмотрите, как в сикпе сделаны кастомные стили - https://github.com/Hexlet/hexlet-sicp/blob/f02ff453e6c8b045764f497fe536e8ecd24913b6/resources/sass/_custom.scss#L1-L23

Copy link
Contributor Author

Choose a reason for hiding this comment

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

style="height: max-content и style="width:50vw перенесены в base.css как кастомные стили.

style="width: {{ contribution|div:achievement_made_count|mul:100|floatformat:0 }} используемый прогрессбаром остается, так как увидел подобное решение в образце от bootstrap

<dt>{% trans achievement_name %}</dt>
<dd class="mb-0">{% trans achievement_description %}</dd>
</dl>
</div>

<div class="p-2 fs-6 d-flex justify-content-center align-items-center">
{% if contribution|div:achievement_made_count|mul:100 > 100 %}
100.0%
{% else %}
{{ contribution|div:achievement_made_count|mul:100|floatformat:1 }}%
{% endif %}
</div>
</div>
Loading
Loading