Skip to content

Commit

Permalink
Fix GH actions
Browse files Browse the repository at this point in the history
  • Loading branch information
AlvaroLQueiroz committed Apr 7, 2024
1 parent 30cf195 commit 6b1ede6
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 49 deletions.
38 changes: 24 additions & 14 deletions .github/workflows/test.yml → .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,29 @@ on:
jobs:
code-check:
name: Code checking
runs-on: macos-latest
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: "3.11"
- name: Install poetry
run: pipx install poetry
- name: Set python version for poetry
run: poetry env use python3.11
- name: Install dependencies
run: poetry install
- name: Run Isort
run: poetry run -v -- isort -c notifications/ sample_website/
- name: Run Black
run: poetry run -v -- black --check notifications/ sample_website/
- name: Run Pylint
run: poetry run -v -- pylint --rcfile=pyproject.toml notifications/ sample_website/
run: poetry run -v -- pylint --rcfile=pyproject.toml --recursive=y -v notifications/ sample_website/
- name: Run Bandit
run: poetry run -v -- bandit -c pyproject.toml -r notifications/ sample_website/

- name: Run Mypy
run: poetry run -v -- mypy

test:
name: Testing
Expand All @@ -37,24 +44,26 @@ jobs:
python-version: ["3.8", "3.9", "3.10", "3.11"]

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
- name: Install poetry
run: pipx install poetry
- name: Set python version for poetry
run: poetry env use python${{ matrix.python-version }}
- name: Install dependencies
run: poetry install
- name: Run tests
run: poetry run pytest
run: poetry run -- pytest
env:
COVERAGE_FILE: ".coverage.${{ matrix.python_version }}"
COVERAGE_FILE: ".coverage.${{ matrix.python-version }}"
- name: Store coverage file
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: coverage
path: .coverage.${{ matrix.python_version }}
name: coverage-${{ matrix.python-version }}
path: .coverage.${{ matrix.python-version }}


coverage:
Expand All @@ -65,12 +74,13 @@ jobs:
pull-requests: write
contents: write
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4

- uses: actions/download-artifact@v3
- uses: actions/download-artifact@v4
id: download
with:
name: 'coverage'
pattern: coverage-*
merge-multiple: true

- name: Coverage comment
id: coverage_comment
Expand All @@ -80,7 +90,7 @@ jobs:
MERGE_COVERAGE_FILES: true

- name: Store Pull Request comment to be posted
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
if: steps.coverage_comment.outputs.COMMENT_FILE_WRITTEN == 'true'
with:
name: python-coverage-comment-action
Expand Down
32 changes: 32 additions & 0 deletions .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Post coverage comment

on:
workflow_run:
workflows: ["CI"]
types:
- completed

jobs:
test:
name: Coverage
runs-on: ubuntu-latest
if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success'
permissions:
# Gives the action the necessary permissions for publishing new
# comments in pull requests.
pull-requests: write
# Gives the action the necessary permissions for editing existing
# comments (to avoid publishing multiple comments in the same PR)
contents: write
# Gives the action the necessary permissions for looking up the
# workflow that launched this workflow, and download the related
# artifact that contains the comment to be published
actions: read
steps:
# DO NOT run actions/checkout here, for security reasons
# For details, refer to https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
- name: Post comment
uses: py-cov-action/python-coverage-comment-action@v3
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_PR_RUN_ID: ${{ github.event.workflow_run.id }}
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ repos:
hooks:
- id: pylint
name: pylint
entry: poetry run pylint
entry: poetry run -- pylint
language: system
args: ["--rcfile=pyproject.toml"]

Expand Down
2 changes: 1 addition & 1 deletion dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ RUN pyenv local system 3.8 3.9 3.10
RUN apt-get --purge autoremove -y gnupg; \
rm -rf /var/cache/apt/lists;

ENTRYPOINT poetry install && poetry run pre-commit install && /bin/bash
ENTRYPOINT poetry install && poetry run -- pre-commit install && /bin/bash
4 changes: 2 additions & 2 deletions notifications/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ def get_queryset(self, request: HttpRequest):
return qs.prefetch_related("actor")

@admin.action(description=gettext_lazy("Mark selected notifications as unread"))
def mark_unread(self, request: HttpRequest, queryset: NotificationQuerySet): # pylint: disable=unused-argument
def mark_unread(self, request: HttpRequest, queryset: NotificationQuerySet):
queryset.update(unread=True)

@admin.action(description=gettext_lazy("Mark selected notifications as read"))
def mark_read(self, request: HttpRequest, queryset: NotificationQuerySet): # pylint: disable=unused-argument
def mark_read(self, request: HttpRequest, queryset: NotificationQuerySet):
queryset.update(unread=False)
3 changes: 2 additions & 1 deletion notifications/models/base.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# -*- coding: utf-8 -*-
import datetime
from typing import Union

from django.conf import settings
from django.contrib.contenttypes.fields import GenericForeignKey
Expand Down Expand Up @@ -130,7 +131,7 @@ def __str__(self):
return _("%(actor)s %(verb)s %(action_object)s %(timesince)s ago") % ctx
return _("%(actor)s %(verb)s %(timesince)s ago") % ctx

def timesince(self, now: None | datetime.datetime = None) -> str:
def timesince(self, now: Union[None, datetime.datetime] = None) -> str:
"""
Shortcut for the ``django.utils.timesince.timesince`` function of the
current timestamp.
Expand Down
14 changes: 7 additions & 7 deletions notifications/querysets.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Type
from typing import Type, Union

from django.core.exceptions import ImproperlyConfigured
from django.db import models
Expand Down Expand Up @@ -46,7 +46,7 @@ def read(self, include_deleted: bool = False) -> "NotificationQuerySet":
# in this case, to improve query performance, don't filter by 'deleted' field
return self.filter(unread=False)

def mark_all_as_read(self, recipient: None | Type[AbstractUser] = None) -> int:
def mark_all_as_read(self, recipient: Union[None, Type[AbstractUser]] = None) -> int:
"""Mark as read any unread messages in the current queryset.
Optionally, filter these by recipient first.
Expand All @@ -59,7 +59,7 @@ def mark_all_as_read(self, recipient: None | Type[AbstractUser] = None) -> int:

return qset.update(unread=False)

def mark_all_as_unread(self, recipient: None | Type[AbstractUser] = None) -> int:
def mark_all_as_unread(self, recipient: Union[None, Type[AbstractUser]] = None) -> int:
"""Mark as unread any read messages in the current queryset.
Optionally, filter these by recipient first.
Expand All @@ -81,7 +81,7 @@ def active(self) -> "NotificationQuerySet":
assert_soft_delete()
return self.filter(deleted=False)

def mark_all_as_deleted(self, recipient: None | Type[AbstractUser] = None) -> int:
def mark_all_as_deleted(self, recipient: Union[None, Type[AbstractUser]] = None) -> int:
"""Mark current queryset as deleted.
Optionally, filter by recipient first.
"""
Expand All @@ -92,7 +92,7 @@ def mark_all_as_deleted(self, recipient: None | Type[AbstractUser] = None) -> in

return qset.update(deleted=True)

def mark_all_as_active(self, recipient: None | Type[AbstractUser] = None) -> int:
def mark_all_as_active(self, recipient: Union[None, Type[AbstractUser]] = None) -> int:
"""Mark current queryset as active(un-deleted).
Optionally, filter by recipient first.
"""
Expand All @@ -103,13 +103,13 @@ def mark_all_as_active(self, recipient: None | Type[AbstractUser] = None) -> int

return qset.update(deleted=False)

def mark_as_unsent(self, recipient: None | Type[AbstractUser] = None) -> int:
def mark_as_unsent(self, recipient: Union[None, Type[AbstractUser]] = None) -> int:
qset = self.sent()
if recipient:
qset = qset.filter(recipient=recipient)
return qset.update(emailed=False)

def mark_as_sent(self, recipient: None | Type[AbstractUser] = None) -> int:
def mark_as_sent(self, recipient: Union[None, Type[AbstractUser]] = None) -> int:
qset = self.unsent()
if recipient:
qset = qset.filter(recipient=recipient)
Expand Down
20 changes: 12 additions & 8 deletions notifications/settings.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
""" Django notifications settings file """
# -*- coding: utf-8 -*-
from typing import Any, TypedDict
from typing import Any, TypedDict, Union

from django.conf import settings

Expand Down Expand Up @@ -39,15 +39,17 @@ class NotificationSettings:
apps, and test helpers like `override_settings` may not work as expected.
"""

def __init__(self, defaults: None | NotificationDefaultsType = None):
self._user_settings = None
self.defaults = defaults or NotificationDefaultsType(**NOTIFICATION_DEFAULTS)
def __init__(self, defaults: Union[None, NotificationDefaultsType] = None):
self._user_settings: Union[NotificationDefaultsType, None] = None
self.defaults = defaults or NotificationDefaultsType(**NOTIFICATION_DEFAULTS) # type: ignore[typeddict-item]
self._cached_attrs: set[str] = set()

@property
def user_settings(self) -> NotificationDefaultsType:
if not self._user_settings:
self._user_settings = NotificationDefaultsType(**getattr(settings, "DJANGO_NOTIFICATIONS_CONFIG") or {})
self._user_settings = NotificationDefaultsType(
**getattr(settings, "DJANGO_NOTIFICATIONS_CONFIG") or {}
) # type: ignore[typeddict-item]
return self._user_settings

def __getattr__(self, attr: str) -> NotificationDefaultsType:
Expand All @@ -56,10 +58,10 @@ def __getattr__(self, attr: str) -> NotificationDefaultsType:

try:
# Check if present in user settings
val = self.user_settings[attr]
val = self.user_settings[attr] # type: ignore[literal-required]
except KeyError:
# Fall back to defaults
val = self.defaults[attr]
val = self.defaults[attr] # type: ignore[literal-required]

# Cache the result
self._cached_attrs.add(attr)
Expand All @@ -73,7 +75,9 @@ def reload(self):
self._user_settings = None


notification_settings = NotificationSettings(NOTIFICATION_DEFAULTS)
notification_settings = NotificationSettings(
NotificationDefaultsType(**NOTIFICATION_DEFAULTS) # type: ignore[typeddict-item]
)


def reload_notification_settings(*args: Any, **kwargs: Any): # pylint: disable=unused-argument
Expand Down
11 changes: 5 additions & 6 deletions notifications/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
Replace this with more appropriate tests for your application.
"""
# -*- coding: utf-8 -*-
# pylint: disable=too-many-lines,missing-docstring
import json
from datetime import datetime, timezone

Expand Down Expand Up @@ -107,7 +106,7 @@ def test_notify_send_return_val(self):
# only check types for now
self.assertEqual(type(result[1][0]), Notification)

def test_notify_send_return_val_group(self): # pylint: disable=invalid-name
def test_notify_send_return_val_group(self):
results = notify.send(self.from_user, recipient=self.to_group, verb="commented", action_object=self.from_user)
for result in results:
if result[0] is notify_handler:
Expand Down Expand Up @@ -137,7 +136,7 @@ def test_mark_all_as_read_manager(self):
Notification.objects.filter(recipient=self.to_user).mark_all_as_read()
self.assertEqual(self.to_user.notifications.unread().count(), 0)

@override_settings(DJANGO_NOTIFICATIONS_CONFIG={"SOFT_DELETE": True}) # pylint: disable=invalid-name
@override_settings(DJANGO_NOTIFICATIONS_CONFIG={"SOFT_DELETE": True})
def test_mark_all_as_read_manager_with_soft_delete(self):
# even soft-deleted notifications should be marked as read
# refer: https://github.com/django-notifications/django-notifications/issues/126
Expand All @@ -155,7 +154,7 @@ def test_mark_all_as_unread_manager(self):
Notification.objects.filter(recipient=self.to_user).mark_all_as_unread()
self.assertEqual(Notification.objects.unread().count(), self.message_count)

def test_mark_all_deleted_manager_without_soft_delete(self): # pylint: disable=invalid-name
def test_mark_all_deleted_manager_without_soft_delete(self):
self.assertRaises(ImproperlyConfigured, Notification.objects.active)
self.assertRaises(ImproperlyConfigured, Notification.objects.active)
self.assertRaises(ImproperlyConfigured, Notification.objects.mark_all_as_deleted)
Expand Down Expand Up @@ -295,7 +294,7 @@ def test_delete_messages_pages(self):
self.assertEqual(len(response.context["notifications"]), len(self.to_user.notifications.unread()))
self.assertEqual(len(response.context["notifications"]), self.message_count - 1)

@override_settings(DJANGO_NOTIFICATIONS_CONFIG={"SOFT_DELETE": True}) # pylint: disable=invalid-name
@override_settings(DJANGO_NOTIFICATIONS_CONFIG={"SOFT_DELETE": True})
def test_soft_delete_messages_manager(self):
self.login("to", "pwd")

Expand Down Expand Up @@ -439,7 +438,7 @@ def test_all_list_api(self):
self.assertEqual(data["all_list"][0]["verb"], "commented")
self.assertEqual(data["all_list"][0]["slug"], data["all_list"][0]["id"])

def test_unread_list_api_mark_as_read(self): # pylint: disable=invalid-name
def test_unread_list_api_mark_as_read(self):
self.login("to", "pwd")
num_requested = 3
response = self.client.get(
Expand Down
2 changes: 1 addition & 1 deletion notifications/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

from django.contrib.auth.base_user import AbstractBaseUser

AbstractUser = NewType("AbstractUser", AbstractBaseUser)
AbstractUser = NewType("AbstractUser", AbstractBaseUser) # type: ignore[valid-newtype]

0 comments on commit 6b1ede6

Please sign in to comment.