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

Add enrollment preprocessing CLI tool #2011

Merged
merged 71 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
83995b9
add enrolment_preprocessor and user export endpoint
Kakadus Aug 29, 2023
a2cad5b
enrollment
Kakadus Sep 25, 2023
9c032be
no python fiddling
Kakadus Sep 25, 2023
1fa6d46
add tools to black
Kakadus Oct 9, 2023
ff58fe8
rewrite something
Kakadus Oct 9, 2023
1a70516
remove filter
Kakadus Oct 9, 2023
20bbd79
Merge branch 'main' into enrolment-preprocessor
Kakadus Oct 9, 2023
6353023
remove filter test
Kakadus Oct 9, 2023
09d6bf6
cleanup advanced title logic
Kakadus Oct 9, 2023
6a4c740
enable mypy for evap AND tools
Kakadus Oct 16, 2023
7084323
extract run_preprocessor for tests
Kakadus Oct 16, 2023
f311982
remove import
Kakadus Oct 23, 2023
e77b75b
add user export option for staff index and staff user index
Kakadus Oct 23, 2023
86d73ed
Merge branch 'main' into enrolment-preprocessor
Kakadus Oct 23, 2023
2ac9e40
add tools to pytest config
Kakadus Oct 23, 2023
ead305d
add tools to coverage.run
Kakadus Oct 23, 2023
4f3e437
fix links
Kakadus Oct 23, 2023
43839c4
pylint tools
Kakadus Oct 23, 2023
d81787d
add changes to preprocessor
Kakadus Oct 23, 2023
9db3678
fix test
Kakadus Oct 23, 2023
4937a76
fix spelling and test
Kakadus Oct 30, 2023
37a6caa
dont modify inplace
Kakadus Oct 30, 2023
b67c1c8
add newline for visual separation
Kakadus Oct 30, 2023
ce8c35e
switch to positional
Kakadus Oct 30, 2023
7b693bb
try to delete test
Kakadus Oct 30, 2023
06f0ec9
Merge branch 'main' into enrolment-preprocessor
Kakadus Oct 30, 2023
12f2495
move create_memory_csv_file
Kakadus Oct 30, 2023
4410d0d
no default preprocessor decision
Kakadus Nov 6, 2023
cf1c9b1
fix annotations
Kakadus Nov 13, 2023
1fd0a17
use AttachmentResponse
Kakadus Nov 13, 2023
fd04517
revert breaking changes!
Kakadus Nov 13, 2023
c6a29f7
Update tools/enrollment_preprocessor.py
Kakadus Nov 13, 2023
ac96b31
fix argparse arguments
Kakadus Dec 4, 2023
02ebfee
fix csv format
Kakadus Dec 4, 2023
d4ef115
exclude title row
Kakadus Dec 4, 2023
b920544
Merge remote-tracking branch 'upstream/main' into enrolment-preprocessor
Kakadus Dec 4, 2023
97e6132
fix delimiter in test
Kakadus Dec 4, 2023
f348c6b
use consistent header row and skip them
Kakadus Dec 4, 2023
b45e878
remove j0 pylint setting in action
Kakadus Dec 11, 2023
0d30caf
apply changes from review
Kakadus Dec 22, 2023
1c2d287
add whitespace stripping
Kakadus Dec 22, 2023
5a86e4f
skip empty users with empty emails
Kakadus Dec 22, 2023
a43bda0
add test for new requirements
Kakadus Dec 22, 2023
ee4b6c5
Merge branch 'main' into enrolment-preprocessor
Kakadus Dec 22, 2023
ba7e365
reformat
Kakadus Dec 22, 2023
a67c239
rewrite tests and get cleaning right
Kakadus Dec 22, 2023
f64e813
deduplicate imported users
Kakadus Jan 8, 2024
8fb4ca2
add enrollment preprocessor features
Kakadus Jan 15, 2024
9ff5162
use right delimiter
Kakadus Jan 15, 2024
f5d79f6
ansi to octal
Kakadus Jan 23, 2024
2c31322
format
Kakadus Jan 23, 2024
5fc3e43
ntfs fix
Kakadus Jan 23, 2024
3ee4eaa
do not write file if not changed
Kakadus Jan 23, 2024
70f522c
add latest features: Everything works(tm)
Kakadus Feb 12, 2024
0d74cf2
Merge remote-tracking branch 'upstream' into enrolment-preprocessor
Kakadus Feb 12, 2024
d052cf0
split it up
Kakadus Feb 12, 2024
4adaf88
correctly fix the test
Kakadus Feb 12, 2024
85d6efe
add test that tests everything
Kakadus Feb 12, 2024
f1891d6
reformat
Kakadus Feb 12, 2024
8515bb2
reformat with recent black
Kakadus Feb 12, 2024
4648114
apply changes from code review
Kakadus Feb 19, 2024
3b3a7e8
clean cells directly and reparse xlsx. Refactor as well
Kakadus Feb 19, 2024
a951dfc
simplify
Kakadus Feb 19, 2024
f4b9868
apply review suggestions
Kakadus Feb 26, 2024
a7e2c4d
fix duplicated conflicts
Kakadus Mar 4, 2024
3911607
add test
Kakadus Mar 4, 2024
92647fa
Merge remote-tracking branch 'upstream/main' into enrolment-preprocessor
Kakadus Mar 4, 2024
3b12335
sort for easier tests
Kakadus Mar 4, 2024
3327eba
remove outdated code in tests
Kakadus Mar 4, 2024
ca2400a
notify if changes were necessary
Kakadus Mar 4, 2024
ed54126
f./x path output
Kakadus Mar 4, 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
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ jobs:
uses: ./.github/setup_python

- name: Run MyPy
run: mypy -p evap
run: mypy

linter:
runs-on: ubuntu-22.04
Expand All @@ -89,7 +89,7 @@ jobs:
uses: ./.github/setup_python

- name: Run linter
run: pylint evap -j 0
run: pylint evap tools


formatter:
Expand Down
2 changes: 1 addition & 1 deletion evap/evaluation/management/commands/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ class Command(BaseCommand):
requires_migrations_checks = False

def handle(self, *args, **options):
subprocess.run(["pylint", "evap"], check=False) # nosec
subprocess.run(["pylint", "evap", "tools"], check=False) # nosec
2 changes: 1 addition & 1 deletion evap/evaluation/management/commands/typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ class Command(BaseCommand):
requires_migrations_checks = False

def handle(self, *args, **options):
subprocess.run(["mypy", "-p", "evap"], check=True) # nosec
subprocess.run(["mypy"], check=True) # nosec
4 changes: 2 additions & 2 deletions evap/evaluation/tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ class TestLintCommand(TestCase):
@patch("subprocess.run")
def test_pylint_called(mock_subprocess_run):
management.call_command("lint")
mock_subprocess_run.assert_called_once_with(["pylint", "evap"], check=False)
mock_subprocess_run.assert_called_once_with(["pylint", "evap", "tools"], check=False)


class TestFormatCommand(TestCase):
Expand All @@ -392,7 +392,7 @@ class TestTypecheckCommand(TestCase):
def test_mypy_called(self, mock_subprocess_run):
management.call_command("typecheck")
self.assertEqual(len(mock_subprocess_run.mock_calls), 1)
mock_subprocess_run.assert_has_calls([call(["mypy", "-p", "evap"], check=True)])
mock_subprocess_run.assert_has_calls([call(["mypy"], check=True)])


class TestPrecommitCommand(TestCase):
Expand Down
18 changes: 18 additions & 0 deletions evap/staff/fixtures/excel_files_test_data.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import csv
import io
from typing import TextIO

import openpyxl

Expand Down Expand Up @@ -198,6 +200,14 @@
]
}

# user.title, user.last_name, user.first_name, user.email
Kakadus marked this conversation as resolved.
Show resolved Hide resolved
valid_user_courses_import_users = [
['', 'Quid', 'Bastius', '[email protected]'],
['', 'Sed', 'Diam', '[email protected]'],
['', 'Manilium', 'Lucilia', '[email protected]'],
['Prof. Dr.', 'Prorsus', 'Christoph', '[email protected]']
]

random_file_content = b"Hallo Welt\n"


Expand All @@ -221,3 +231,11 @@ def create_memory_excel_file(data):
sheet.cell(row=row_num, column=column_num).value = cell_data
workbook.save(memory_excel_file)
return memory_excel_file.getvalue()


def create_memory_csv_file(data) -> TextIO:
memory_csv_file = io.StringIO()
writer = csv.writer(memory_csv_file, delimiter=";", lineterminator="\n")
writer.writerows(data)
memory_csv_file.seek(0)
return memory_csv_file
1 change: 1 addition & 0 deletions evap/staff/templates/staff_index.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ <h4 class="card-title">{% translate 'Users' %}</h4>
<ul>
<li><a href="{% url 'staff:user_list' %}">{% translate 'All users' %}</a></li>
<li><a href="{% url 'staff:user_import' %}">{% translate 'Import users' %}</a></li>
<li><a href="{% url 'staff:user_export' %}">{% trans 'Export users' %}</a></li>
<li><a href="{% url 'staff:user_merge_selection' %}">{% translate 'Merge users' %}</a></li>
<li><a href="{% url 'staff:user_bulk_update' %}">{% translate 'Update users' %}</a></li>
</ul>
Expand Down
1 change: 1 addition & 0 deletions evap/staff/templates/staff_user_index.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<ul>
<li><a href="{% url 'staff:user_list' %}" class="">{% translate 'User list' %}</a></li>
<li><a href="{% url 'staff:user_import' %}" class="">{% translate 'Import users' %}</a></li>
<li><a href="{% url 'staff:user_export' %}" class="">{% trans 'Export users' %}</a></li>
<li><a href="{% url 'staff:user_merge_selection' %}" class="">{% translate 'Merge users' %}</a></li>
<li><a href="{% url 'staff:user_bulk_update' %}" class="">{% translate 'Bulk update users' %}</a></li>
</ul>
Expand Down
87 changes: 87 additions & 0 deletions evap/staff/tests/test_tools.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,31 @@
from io import BytesIO
from itertools import cycle, repeat
from unittest.mock import MagicMock, patch

from django.contrib.auth.models import Group
from django.test import TestCase
from django.utils.html import escape
from django_webtest import WebTest
from model_bakery import baker
from openpyxl import load_workbook

from evap.evaluation.models import Contribution, Course, Evaluation, UserProfile
from evap.evaluation.tests.tools import assert_no_database_modifications
from evap.evaluation.tools import assert_not_none
from evap.rewards.models import RewardPointGranting, RewardPointRedemption
from evap.staff.fixtures.excel_files_test_data import (
create_memory_csv_file,
create_memory_excel_file,
valid_user_courses_import_filedata,
valid_user_courses_import_users,
)
from evap.staff.tools import (
conditional_escape,
merge_users,
remove_user_from_represented_and_ccing_users,
user_edit_link,
)
from tools.enrollment_preprocessor import run_preprocessor


class MergeUsersTest(TestCase):
Expand Down Expand Up @@ -216,3 +230,76 @@ def test_conditional_escape(self):
self.assertEqual(conditional_escape("<script>"), "&lt;script&gt;")
self.assertEqual(conditional_escape(escape("<script>")), "&lt;script&gt;")
self.assertEqual(conditional_escape("safe"), "safe")


class EnrollmentPreprocessorTest(WebTest):
@classmethod
def setUpTestData(cls) -> None:
cls.imported_data = valid_user_courses_import_filedata
cls.csv = create_memory_csv_file(
[["Title", "Last name", "First name", "Email"]] + valid_user_courses_import_users
)

@patch("builtins.input", side_effect=cycle(("i", "e", "invalid")))
def test_xlsx_data_stripped(self, input_patch: MagicMock):
self.imported_data["MA Belegungen"][1][1] = " Accepted "
self.imported_data["MA Belegungen"][1][8] = " conflicts "
self.imported_data["BA Belegungen"][1][2] = " are "
self.imported_data["BA Belegungen"][1][11] = " stripped. "
modified = run_preprocessor(BytesIO(create_memory_excel_file(self.imported_data)), self.csv)
self.assertIsNotNone(modified)
self.assertEqual(input_patch.call_count, 4)
workbook = load_workbook(assert_not_none(modified), read_only=True)
self.assertEqual(workbook["MA Belegungen"]["B2"].value, "Accepted") # stripped conflict used
self.assertEqual(workbook["MA Belegungen"]["I2"].value, None) # existing data kept
self.assertEqual(workbook["BA Belegungen"]["C2"].value, "are") # stripped conflict used
self.assertEqual(workbook["BA Belegungen"]["L2"].value, "stripped.") # different email is no conflict

@patch("builtins.input", side_effect=repeat("i"))
def test_empty_email_ignored(self, input_patch: MagicMock):
self.imported_data["MA Belegungen"][1][1] = " Add "
self.imported_data["MA Belegungen"][1][8] = " some "
self.imported_data["BA Belegungen"][1][2] = " conflicts "
self.imported_data["MA Belegungen"][1][3] = " "
self.imported_data["MA Belegungen"][1][11] = " "
self.imported_data["BA Belegungen"][1][3] = " \t "
self.imported_data["BA Belegungen"][1][11] = " \n "
res = run_preprocessor(BytesIO(create_memory_excel_file(self.imported_data)), self.csv)
self.assertIsNone(res)
input_patch.assert_not_called()

@patch("builtins.input", side_effect=repeat("i"))
def test_deduplication(self, input_patch: MagicMock):
self.imported_data["MA Belegungen"][1][1] = "Some conflicts"
self.imported_data["MA Belegungen"][1][8] = "in all"
self.imported_data["BA Belegungen"][1][2] = "fields"
# copy data and pad with spaces
self.imported_data["MA Belegungen"].append([f" {data} " for data in self.imported_data["MA Belegungen"][1]])

res = run_preprocessor(BytesIO(create_memory_excel_file(self.imported_data)), self.csv)
self.assertIsNone(res)
self.assertEqual(input_patch.call_count, 3) # conflicts are deduplicated.

@patch("builtins.input", side_effect=cycle(("i", "e", "e", "invalid")))
def test_changes_applied_globally(self, input_patch: MagicMock):
self.imported_data["MA Belegungen"][1][1] = "some conflicts"
self.imported_data["MA Belegungen"][1][8] = "in all"
self.imported_data["BA Belegungen"][1][2] = "fields"
# copy data and pad with spaces and add conflict
self.imported_data["MA Belegungen"].append([f" {data} " for data in self.imported_data["MA Belegungen"][1]])
self.imported_data["BA Belegungen"].append([f" {data} " for data in self.imported_data["BA Belegungen"][1]])
self.imported_data["MA Belegungen"][2][1] += "modified"
self.imported_data["MA Belegungen"][2][8] += "modified"
self.imported_data["BA Belegungen"][2][2] += "modified"
self.imported_data["MA Belegungen"].append([f" {data} " for data in self.imported_data["MA Belegungen"][2]])
self.imported_data["BA Belegungen"].append([f" {data} " for data in self.imported_data["BA Belegungen"][2]])
modified = run_preprocessor(BytesIO(create_memory_excel_file(self.imported_data)), self.csv)
self.assertIsNotNone(modified)
self.assertEqual(input_patch.call_count, 7)
workbook = load_workbook(assert_not_none(modified), read_only=True)
self.assertEqual(workbook["MA Belegungen"]["B2"].value, "some conflicts")
self.assertEqual(workbook["MA Belegungen"]["B3"].value, "some conflicts")
self.assertEqual(workbook["MA Belegungen"]["I2"].value, "in all modified")
self.assertEqual(workbook["MA Belegungen"]["I3"].value, "in all modified")
self.assertEqual(workbook["BA Belegungen"]["C2"].value, "Lucilia")
self.assertEqual(workbook["BA Belegungen"]["C3"].value, "Lucilia")
28 changes: 28 additions & 0 deletions evap/staff/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import csv
import datetime
import os
from abc import ABC, abstractmethod
Expand Down Expand Up @@ -569,6 +570,33 @@ def test_wrong_files_dont_crash(self):
self.assertIn("An error happened when processing the file", reply)


class TestUserExportView(WebTestStaffMode):
url = reverse("staff:user_export")

@classmethod
def setUpTestData(cls) -> None:
cls.manager = make_manager()
# titles are not filled by baker because it has a default, see https://github.com/model-bakers/model_bakery/discussions/346
baker.make(
UserProfile,
_quantity=5,
_fill_optional=["first_name_given", "last_name", "email"],
title=iter(("", "Some", "Custom", "Titles", "")),
)

def test_export_all(self):
user_objects = {
(user.title or "", user.last_name or "", user.first_name or "", user.email or "")
for user in UserProfile.objects.iterator()
}
response = self.app.get(self.url, user=self.manager)

reader = csv.reader(response.text.strip().split("\n"), delimiter=";", lineterminator="\n")
# skip header
next(reader)
self.assertEqual({tuple(row) for row in reader}, user_objects)


class TestUserImportView(WebTestStaffMode):
url = "/staff/user/import"

Expand Down
1 change: 1 addition & 0 deletions evap/staff/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
path("user/", views.user_index, name="user_index"),
path("user/create", views.UserCreateView.as_view(), name="user_create"),
path("user/import", views.user_import, name="user_import"),
path("user/export", views.user_export, name="user_export"),
path("user/<int:user_id>/edit", views.user_edit, name="user_edit"),
path("user/list", views.user_list, name="user_list"),
path("user/delete", views.user_delete, name="user_delete"),
Expand Down
12 changes: 12 additions & 0 deletions evap/staff/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2136,6 +2136,18 @@ def user_list(request):
return render(request, "staff_user_list.html", {"users": users, "filter_users": filter_users})


@manager_required
def user_export(request):
response = AttachmentResponse("exported_users.csv")
writer = csv.writer(response, delimiter=";", lineterminator="\n")
header_row = (_("Title"), _("Last name"), _("First name"), _("Email"))
writer.writerow(header_row)
writer.writerows(
(user.title, user.last_name, user.first_name, user.email) for user in UserProfile.objects.iterator()
)
return response


@manager_required
class UserCreateView(SuccessMessageMixin, CreateView):
model = UserProfile
Expand Down
9 changes: 5 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ requires-python = ">=3.10"

[tool.black]
line-length = 120
include = 'evap.*\.pyi?$'
include = '(evap|tools).*\.pyi?$'
extend-exclude = """\
.*/urls\\.py|\
.*/migrations/.*\
Expand All @@ -15,7 +15,7 @@ extend-exclude = """\

[tool.isort]
profile = "black"
src_paths = ["evap"]
src_paths = ["evap", "tools"]
line_length = 120
skip_gitignore = true
extend_skip_glob = ["**/migrations/*"]
Expand Down Expand Up @@ -74,14 +74,15 @@ disable = [
[tool.coverage.run]
branch = true
omit = ["*migrations*", "*__init__.py"]
source = ["evap"]
source = ["evap", "tools"]

[tool.coverage.report]
omit = ["*/migrations/*", "*__init__.py"]

##############################################

[tool.mypy]
packages = ["evap", "tools"]
plugins = ["mypy_django_plugin.main"]
exclude = 'evap/.*/migrations/.*\.py$'
no_implicit_optional = true
Expand Down Expand Up @@ -112,6 +113,6 @@ ignore_missing_imports = true
# pytest-xdist worked for parallelizing tests.
DJANGO_SETTINGS_MODULE = "evap.settings"
python_files = ["tests.py", "test_*.py", "*_tests.py"]
testpaths = ["evap"]
testpaths = ["evap", "tools"]
norecursedirs=["locale", "logs", "static", "static_collected", "upload"]
addopts = "--reuse-db"
Empty file added tools/__init__.py
Empty file.
Loading
Loading