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

Repo consistency #639

Open
wants to merge 43 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
9126a9f
Need to merge develop back in
fritzbrand Oct 17, 2024
d47888e
Merge branch 'develop' into repo-consistency
fritzbrand Oct 17, 2024
6f9842a
Added direct git install for iso639
fritzbrand Oct 17, 2024
9996a04
Perparing to remove pip
fritzbrand Oct 17, 2024
a45b777
Unpinned versions
fritzbrand Oct 17, 2024
904614d
Pinned specific 'rapidpro-python' version
fritzbrand Oct 17, 2024
5a88fc4
Added blanket ignores until ruff throws no errors
fritzbrand Oct 17, 2024
a9f57f1
Redefinition of unused and ruff format
fritzbrand Oct 21, 2024
346e40f
Use 'enumerate()' for index variable 'total' in 'for' loop
fritzbrand Oct 21, 2024
a7c53c2
Use ternary operator instead of 'if'-'else'-block
fritzbrand Oct 21, 2024
3d470a6
Return the condition 'label in labels' directly
fritzbrand Oct 21, 2024
95a413f
'typing.Text' is deprecated, use 'str'
fritzbrand Oct 21, 2024
e4fd7bd
Unnecessary 'map' usage (rewrite using a generator expression)
fritzbrand Oct 21, 2024
a4e5d64
Do not use mutable data structures for argument defaults
fritzbrand Oct 21, 2024
570704f
Found useless expression. Either assign it to a variable or remove it.
fritzbrand Oct 21, 2024
d982515
Possible hardcoded password assigned
fritzbrand Oct 21, 2024
721c825
Unnecessary imports for target Python version
fritzbrand Oct 21, 2024
462ce1b
Within an 'except' clause, raise exceptions with 'raise ... from err'…
fritzbrand Oct 21, 2024
3fb00c5
Standard pseudo-random generators are not suitable for cryptographic …
fritzbrand Oct 21, 2024
fed49a6
Use 'super()' instead of 'super(__class__, self)'
fritzbrand Oct 21, 2024
ac1974f
Use format specifiers instead of percent format
fritzbrand Oct 21, 2024
61b890a
'try'-'except'-'continue' detected, consider logging the exception
fritzbrand Oct 21, 2024
13ec3dd
Use 'contextlib.suppress(Exception)' instead of 'try'-'except'-'pass'
fritzbrand Oct 21, 2024
340904e
Use a single 'if' statement instead of nested 'if' statements
fritzbrand Oct 21, 2024
3072035
Use 'is' and 'is not' for type comparisons, or 'isinstance()' for isi…
fritzbrand Oct 21, 2024
7bd8a0f
Use 'return any(post in group[name].lower() for group in contact[grou…
fritzbrand Oct 21, 2024
1f1eb00
'try'-'except'-'pass' detected, consider logging the exception
fritzbrand Oct 21, 2024
143e247
Possible hardcoded password assigned to argument
fritzbrand Oct 21, 2024
e4e40d3
Loop control variable 'status_code' not used within loop body
fritzbrand Oct 21, 2024
4f8c9a9
Class 'Template' inherits from 'object'
fritzbrand Oct 21, 2024
2febd39
Unnecessary 'list' comprehension (rewrite using 'list()')
fritzbrand Oct 21, 2024
94575cd
Use implicit references for positional format fields
fritzbrand Oct 21, 2024
0c5fa39
'open()' should be replaced by 'Path.open()' maybe use pathlib
fritzbrand Oct 21, 2024
9a93005
Use f-string instead of 'format' call
fritzbrand Oct 21, 2024
fd2c60f
GitHub Actions update
fritzbrand Oct 22, 2024
3b2c0bc
Update Dockerfile
fritzbrand Oct 22, 2024
9b2034b
Update merge conflicts
fritzbrand Oct 23, 2024
958975e
Removed setup.py and updated test.yaml
fritzbrand Oct 23, 2024
5b2d52d
Removed global S106 ignore, and added inline ignores
fritzbrand Oct 23, 2024
7d8bc10
RUF - enabled ruff-specific rules
fritzbrand Oct 23, 2024
e54ac24
Formatting
fritzbrand Oct 23, 2024
165fac8
Updated wording for S106 inlines
fritzbrand Oct 23, 2024
f54b07e
Updates after feedback
fritzbrand Oct 24, 2024
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
1 change: 1 addition & 0 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ on:
push:
branches:
- develop
- repo-consistency
tags:
- "*"

Expand Down
43 changes: 22 additions & 21 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,33 +24,34 @@ jobs:
env:
HUB_DATABASE: postgres://postgres:postgres@localhost:5432/ndoh_hub
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- name: Install gettext
run: sudo apt-get install gettext
- uses: actions/cache@v2
with:
path: ~/.cache/pip
key: ${{ hashFiles('requirements.txt', 'requirements-dev.txt') }}-pip
- uses: actions/setup-python@v2
- uses: actions/setup-python@v5
with:
python-version: 3.9
- name: Install dependancies
- uses: abatilo/actions-poetry@v3
- name: Install dependencies
id: install-deps
run: |
poetry install
- name: Check formatting
# Lints/tests should always run, even if other lints/tests have failed.
if: success() || failure() && steps.install-deps.outcome == 'success'
run: |
python -m pip install --upgrade pip
pip install -r requirements.txt -r requirements-dev.txt
django-admin compilemessages
poetry run ruff format --check
- name: Lint
if: success() || failure() && steps.install-deps.outcome == 'success'
run: |
flake8
python manage.py makemigrations registrations changes eventstore --dry-run | grep 'No changes detected' || (echo 'There are changes which require migrations.' && exit 1)
black --check .
isort -c -rc .
- name: Test
poetry run ruff check
- name: Run migrations
if: success() || failure() && steps.install-deps.outcome == 'success'
run: |
py.test
- name: Coverage comment
id: coverage_comment
uses: py-cov-action/python-coverage-comment-action@v3
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
poetry run ./manage.py makemigrations registrations changes\
eventstore --dry-run | grep 'No changes detected' || (echo 'There\
are changes which require migrations.' && exit 1)
- name: Run tests
if: success() || failure() && steps.install-deps.outcome == 'success'
run: |
poetry run pytest -vv

6 changes: 4 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
FROM ghcr.io/praekeltfoundation/docker-django-bootstrap-nw:py3.9-bullseye

COPY setup.py /app
RUN pip install --no-cache-dir -e .
RUN pip install poetry==1.8.3
fritzbrand marked this conversation as resolved.
Show resolved Hide resolved
ENV DJANGO_SETTINGS_MODULE "ndoh_hub.settings"

COPY . /app
RUN poetry config virtualenvs.create false \
&& poetry install --no-dev --no-interaction --no-ansi --no-cache

RUN apt-get-install.sh gettext; \
django-admin compilemessages; \
apt-get-purge.sh gettext
Expand Down
2 changes: 1 addition & 1 deletion aaq/tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def post_inbound_check(self, request):
[
"26",
"Fainting in pregnancy",
"*Fainting could mean anemia visit the clinic to find out",
"*Fainting could mean anemia - visit the clinic to find out",
],
[
"114",
Expand Down
1 change: 0 additions & 1 deletion aaq/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@


class SearchFunctionTest(APITestCase):

@responses.activate
def test_search_function(self):
user = get_user_model().objects.create_user("test")
Expand Down
2 changes: 1 addition & 1 deletion aaq/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def test_get_first_page_view(self):
"body": {
"1": {"text": "*Yes, pregnancy can affect your breathing", "id": "21"},
"2": {
"text": "*Fainting could mean anemia visit the clinic to "
"text": "*Fainting could mean anemia - visit the clinic to "
"find out",
"id": "26",
},
Expand Down
1 change: 0 additions & 1 deletion aaq/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ def response_feedback(request, *args, **kwargs):
@api_view(("POST",))
@renderer_classes((JSONRenderer,))
def aaq_search(request):

serializer = SearchSerializer(data=request.data)
serializer.is_valid(raise_exception=True)
query_text = serializer.validated_data["query_text"]
Expand Down
1 change: 0 additions & 1 deletion eventstore/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class ApproximatePaginator(Paginator):

@cached_property
def count(self):
cursor = connection.cursor()
with transaction.atomic(), connection.cursor() as cursor:
cursor.execute("SET LOCAL statement_timeout TO 50")
try:
Expand Down
33 changes: 7 additions & 26 deletions eventstore/management/commands/migrate_to_eventstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,31 +36,24 @@ def add_arguments(self, parser):
def execute_with_progress(self, func, iterator):
total = 0
start, d_print = time.time(), time.time()
for arg in iterator:
for total, arg in enumerate(iterator):
func(arg)
if time.time() - d_print > 1:
self.stdout.write(
f"\rProcessed {total} at {total/(time.time() - start):.0f}/s",
ending="",
)
d_print = time.time()
total += 1
self.stdout.write("")

def handle_public_registration(self, reg):
data = reg.data or {}
if "whatsapp" in reg.reg_type:
channel = "WhatsApp"
else:
channel = "SMS"
channel = "WhatsApp" if "whatsapp" in reg.reg_type else "SMS"
uuid_registrant = data.pop("uuid_registrant", None)
operator_id = data.pop("operator_id", None)
uuid_device = data.pop("uuid_device", None)
language = data.pop("language", "")
if language in LANGUAGES:
language = language.rstrip("_ZA")
else:
language = ""
language = language.rstrip("_ZA") if language in LANGUAGES else ""

PublicRegistration.objects.update_or_create(
id=reg.id,
Expand All @@ -78,10 +71,7 @@ def handle_public_registration(self, reg):

def handle_CHW_registration(self, reg):
data = reg.data or {}
if "whatsapp" in reg.reg_type:
channel = "WhatsApp"
else:
channel = "SMS"
channel = "WhatsApp" if "whatsapp" in reg.reg_type else "SMS"
uuid_registrant = data.pop("uuid_registrant", None)
operator_id = data.pop("operator_id", None)
uuid_device = data.pop("uuid_device", None)
Expand All @@ -91,10 +81,7 @@ def handle_CHW_registration(self, reg):
passport_number = data.pop("passport_no", "")
date_of_birth = data.pop("mom_dob", None)
language = data.pop("language", "")
if language in LANGUAGES:
language = language.rstrip("_ZA")
else:
language = ""
language = language.rstrip("_ZA") if language in LANGUAGES else ""

CHWRegistration.objects.update_or_create(
id=reg.id,
Expand All @@ -117,10 +104,7 @@ def handle_CHW_registration(self, reg):

def handle_prebirth_registration(self, reg):
data = reg.data or {}
if "whatsapp" in reg.reg_type:
channel = "WhatsApp"
else:
channel = "SMS"
channel = "WhatsApp" if "whatsapp" in reg.reg_type else "SMS"
uuid_registrant = data.pop("uuid_registrant", None)
contact_id = reg.registrant_id or uuid_registrant
uuid_device = data.pop("uuid_device", None)
Expand All @@ -133,10 +117,7 @@ def handle_prebirth_registration(self, reg):
facility_code = data.pop("faccode", "")
edd = data.pop("edd", None)
language = data.pop("language", "")
if language in LANGUAGES:
language = language.rstrip("_ZA")
else:
language = ""
language = language.rstrip("_ZA") if language in LANGUAGES else ""

PrebirthRegistration.objects.update_or_create(
id=reg.id,
Expand Down
47 changes: 23 additions & 24 deletions eventstore/models.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import random
import uuid
from datetime import date
from typing import Text

import pycountry
from django.conf import settings
Expand Down Expand Up @@ -85,9 +84,7 @@ class OptOut(models.Model):
data = models.JSONField(default=dict, blank=True, null=True)

def __str__(self):
return "{} opt out: {} <{}>".format(
self.get_optout_type_display(), self.get_reason_display(), self.contact_id
)
return f"{self.get_optout_type_display()} opt out: {self.get_reason_display()} <{self.contact_id}>"


class BabySwitch(models.Model):
Expand Down Expand Up @@ -348,10 +345,7 @@ def has_label(self, label):
.get("labels", [])
]

if label in labels:
return True

return False
return label in labels


class Event(models.Model):
Expand Down Expand Up @@ -592,9 +586,11 @@ class HCSStudyBRandomization(models.Model):
)

def process_study_b(self):
if self.msisdn not in settings.HCS_STUDY_B_WHITELIST:
if not settings.HCS_STUDY_B_ACTIVE:
return
if (
self.msisdn not in settings.HCS_STUDY_B_WHITELIST
and not settings.HCS_STUDY_B_ACTIVE
):
return
if self.created_by in settings.HCS_STUDY_B_CREATED_BY and not self.study_b_arm:
self.study_b_arm = self.get_random_study_b_arm()

Expand All @@ -612,7 +608,7 @@ def get_random_study_b_arm(self):
actual_total, actual_percentage = self.get_study_totals_per_province()

if actual_total < target_total or actual_percentage < target_percentage:
return random.choice(self.STUDY_ARM_B_CHOICES)[0]
return random.choice(self.STUDY_ARM_B_CHOICES)[0] # noqa: S311 - Not being used for crypto purposes

def get_study_totals_per_province(self):
all_provinces = HCSStudyBRandomization.objects.filter(
Expand All @@ -636,7 +632,7 @@ def save(self, *args, **kwargs):


class HealthCheckUserProfileManager(models.Manager):
def get_or_prefill(self, msisdn: Text) -> "HealthCheckUserProfile":
def get_or_prefill(self, msisdn: str) -> "HealthCheckUserProfile":
"""
Either gets the existing user profile, or creates one using data in the
historical healthchecks
Expand Down Expand Up @@ -758,9 +754,11 @@ def update_post_screening_study_arms(self, risk, created_by):
self.process_study_c(risk, created_by)

def process_study_a(self, created_by):
if self.msisdn not in settings.HCS_STUDY_A_WHITELIST:
if not settings.HCS_STUDY_A_ACTIVE:
return
if (
self.msisdn not in settings.HCS_STUDY_A_WHITELIST
and not settings.HCS_STUDY_A_ACTIVE
):
return

if created_by == settings.HCS_STUDY_A_CREATED_BY and not self.hcs_study_a_arm:
self.hcs_study_a_arm = self.get_random_study_arm()
Expand Down Expand Up @@ -819,10 +817,10 @@ def get_random_study_arm(self):
actual_total, actual_percentage = self.get_study_totals_per_province()

if actual_total < target_total or actual_percentage < target_percentage:
return random.choice(self.STUDY_ARM_CHOICES)[0]
return random.choice(self.STUDY_ARM_CHOICES)[0] # noqa: S311 - Not being used for crypto purposes

def get_random_study_quarantine_arm(self):
return random.choice(self.STUDY_ARM_QUARANTINE_CHOICES)[0]
return random.choice(self.STUDY_ARM_QUARANTINE_CHOICES)[0] # noqa: S311 - Not being used for crypto purposes


class CDUAddressUpdate(models.Model):
Expand Down Expand Up @@ -1052,15 +1050,15 @@ def clean(self):
if not is_valid_edd_date(edd):
raise ValidationError("EDD must be between now and 9 months")
except ValueError as e:
raise ValidationError(f"Invalid EDD date, {str(e)}")
raise ValidationError(f"Invalid EDD date, {e!s}") from e
except TypeError:
# Should be handled by the individual field validator
pass

try:
date(self.baby_dob_year, self.baby_dob_month, self.baby_dob_day)
except ValueError as e:
raise ValidationError(f"Invalid Baby DOB date, {str(e)}")
raise ValidationError(f"Invalid Baby DOB date, {e!s}") from e
except TypeError:
# Should be handled by the individual field validator
pass
Expand All @@ -1081,9 +1079,10 @@ def clean(self):
raise ValidationError("Passport country required for passport ID type")
if self.id_type == self.IDType.PASSPORT and not self.passport_number:
raise ValidationError("Passport number required for passport ID type")
if self.id_type == self.IDType.NONE:
if self.dob_year is None or self.dob_month is None or self.dob_year is None:
raise ValidationError("Date of birth required for none ID type")
if self.id_type == self.IDType.NONE and (
self.dob_year is None or self.dob_month is None or self.dob_year is None
):
raise ValidationError("Date of birth required for none ID type")

if (
self.dob_year is not None
Expand All @@ -1093,7 +1092,7 @@ def clean(self):
try:
date(self.dob_year, self.dob_month, self.dob_day)
except ValueError as e:
raise ValidationError(f"Invalid date of birth date, {str(e)}")
raise ValidationError(f"Invalid date of birth date, {e!s}") from e


class OpenHIMQueue(models.Model):
Expand Down
4 changes: 2 additions & 2 deletions eventstore/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class MSISDNField(serializers.CharField):

def __init__(self, *args, **kwargs):
self.country = kwargs.pop("country", None)
return super(MSISDNField, self).__init__(*args, **kwargs)
return super().__init__(*args, **kwargs)

def to_representation(self, obj):
number = phonenumbers.parse(obj, self.country)
Expand All @@ -139,7 +139,7 @@ def to_internal_value(self, data):
try:
number = phonenumbers.parse(data, self.country)
except phonenumbers.NumberParseException as e:
raise serializers.ValidationError(str(e))
raise serializers.ValidationError(str(e)) from e
if not phonenumbers.is_possible_number(number):
raise serializers.ValidationError("Not a possible phone number")
if not phonenumbers.is_valid_number(number):
Expand Down
Loading
Loading