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 error suffix to exceptions #2063

Merged
merged 1 commit into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions evap/evaluation/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@
logger = logging.getLogger(__name__)


class NotArchiveable(Exception):
"""An attempt has been made to archive something that is not archiveable."""
class NotArchivableError(Exception):
"""An attempt has been made to archive something that is not archivable."""


class Semester(models.Model):
Expand Down Expand Up @@ -106,7 +106,7 @@
@transaction.atomic
def archive(self):
if not self.participations_can_be_archived:
raise NotArchiveable()
raise NotArchivableError()
for evaluation in self.evaluations.all():
evaluation._archive()
self.participations_are_archived = True
Expand All @@ -119,14 +119,14 @@
from evap.grades.models import GradeDocument

if not self.grade_documents_can_be_deleted:
raise NotArchiveable()
raise NotArchivableError()

Check warning on line 122 in evap/evaluation/models.py

View check run for this annotation

Codecov / codecov/patch

evap/evaluation/models.py#L122

Added line #L122 was not covered by tests
GradeDocument.objects.filter(course__semester=self).delete()
self.grade_documents_are_deleted = True
self.save()

def archive_results(self):
if not self.results_can_be_archived:
raise NotArchiveable()
raise NotArchivableError()

Check warning on line 129 in evap/evaluation/models.py

View check run for this annotation

Codecov / codecov/patch

evap/evaluation/models.py#L129

Added line #L129 was not covered by tests
self.results_are_archived = True
self.save()

Expand Down Expand Up @@ -636,7 +636,7 @@
def _archive(self):
"""Should be called only via Semester.archive"""
if not self.participations_can_be_archived:
raise NotArchiveable()
raise NotArchivableError()
if self._participant_count is not None:
assert self._voter_count is not None
assert (
Expand Down
6 changes: 3 additions & 3 deletions evap/evaluation/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
CourseType,
EmailTemplate,
Evaluation,
NotArchiveable,
NotArchivableError,
Question,
Questionnaire,
QuestionType,
Expand Down Expand Up @@ -748,9 +748,9 @@ def test_archiving_participations_does_not_change_results(self):

def test_archiving_participations_twice_raises_exception(self):
self.semester.archive()
with self.assertRaises(NotArchiveable):
with self.assertRaises(NotArchivableError):
self.semester.archive()
with self.assertRaises(NotArchiveable):
with self.assertRaises(NotArchivableError):
self.semester.courses.first().evaluations.first()._archive()

def test_evaluation_participations_are_not_archived_if_participant_count_is_set(self):
Expand Down
6 changes: 3 additions & 3 deletions evap/evaluation/tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ def test_respects_stored_language(self):
translation.activate("en") # for following tests


class SaboteurException(Exception):
class SaboteurError(Exception):
"""An exception class used for making sure that our mock is raising the exception and not some other unrelated code"""


class TestLogExceptionsDecorator(TestCase):
@patch("evap.evaluation.models.Evaluation.update_evaluations", side_effect=SaboteurException())
@patch("evap.evaluation.models.Evaluation.update_evaluations", side_effect=SaboteurError())
@patch("evap.evaluation.management.commands.tools.logger.exception")
def test_log_exceptions_decorator(self, mock_logger, __):
"""
Expand All @@ -54,7 +54,7 @@ def test_log_exceptions_decorator(self, mock_logger, __):
One could create a mock management command and call its handle method manually,
but to me it seemed safer to use a real one.
"""
with self.assertRaises(SaboteurException):
with self.assertRaises(SaboteurError):
management.call_command("update_evaluation_states")

self.assertTrue(mock_logger.called)
Expand Down
8 changes: 4 additions & 4 deletions evap/rewards/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,19 @@
from evap.evaluation.models import Semester, UserProfile


class NoPointsSelected(Exception):
class NoPointsSelectedError(Exception):
"""An attempt has been made to redeem <= 0 points."""


class NotEnoughPoints(Exception):
class NotEnoughPointsError(Exception):
"""An attempt has been made to redeem more points than available."""


class OutdatedRedemptionData(Exception):
class OutdatedRedemptionDataError(Exception):
"""A redemption request has been sent with outdated data, e.g. when a request has been sent twice."""


class RedemptionEventExpired(Exception):
class RedemptionEventExpiredError(Exception):
"""An attempt has been made to redeem more points for an event whose redeem_end_date lies in the past."""


Expand Down
16 changes: 8 additions & 8 deletions evap/rewards/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@

from evap.evaluation.models import Evaluation, Semester, UserProfile
from evap.rewards.models import (
NoPointsSelected,
NotEnoughPoints,
OutdatedRedemptionData,
RedemptionEventExpired,
NoPointsSelectedError,
NotEnoughPointsError,
OutdatedRedemptionDataError,
RedemptionEventExpiredError,
RewardPointGranting,
RewardPointRedemption,
RewardPointRedemptionEvent,
Expand All @@ -31,22 +31,22 @@ def save_redemptions(request, redemptions: dict[int, int], previous_redeemed_poi
# check consistent previous redeemed points
# do not validate reward points, to allow receiving points after page load
if previous_redeemed_points != redeemed_points_of_user(request.user):
raise OutdatedRedemptionData()
raise OutdatedRedemptionDataError()

total_points_available = reward_points_of_user(request.user)
total_points_redeemed = sum(redemptions.values())

if total_points_redeemed <= 0:
raise NoPointsSelected()
raise NoPointsSelectedError()

if total_points_redeemed > total_points_available:
raise NotEnoughPoints()
raise NotEnoughPointsError()

for event_id in redemptions:
if redemptions[event_id] > 0:
event = get_object_or_404(RewardPointRedemptionEvent, pk=event_id)
if event.redeem_end_date < date.today():
raise RedemptionEventExpired()
raise RedemptionEventExpiredError()

RewardPointRedemption.objects.create(user_profile=request.user, value=redemptions[event_id], event=event)

Expand Down
23 changes: 14 additions & 9 deletions evap/rewards/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
from evap.rewards.exporters import RewardsExporter
from evap.rewards.forms import RewardPointRedemptionEventForm
from evap.rewards.models import (
NoPointsSelected,
NotEnoughPoints,
OutdatedRedemptionData,
RedemptionEventExpired,
NoPointsSelectedError,
NotEnoughPointsError,
OutdatedRedemptionDataError,
RedemptionEventExpiredError,
RewardPointGranting,
RewardPointRedemption,
RewardPointRedemptionEvent,
Expand All @@ -45,15 +45,20 @@ def redeem_reward_points(request):
try:
save_redemptions(request, redemptions, previous_redeemed_points)
messages.success(request, _("You successfully redeemed your points."))
except (NoPointsSelected, NotEnoughPoints, RedemptionEventExpired, OutdatedRedemptionData) as error:
except (
NoPointsSelectedError,
NotEnoughPointsError,
RedemptionEventExpiredError,
OutdatedRedemptionDataError,
) as error:
status_code = 400
if isinstance(error, NoPointsSelected):
if isinstance(error, NoPointsSelectedError):
error_string = _("You cannot redeem 0 points.")
elif isinstance(error, NotEnoughPoints):
elif isinstance(error, NotEnoughPointsError):
error_string = _("You don't have enough reward points.")
elif isinstance(error, RedemptionEventExpired):
elif isinstance(error, RedemptionEventExpiredError):
error_string = _("Sorry, the deadline for this event expired already.")
elif isinstance(error, OutdatedRedemptionData):
elif isinstance(error, OutdatedRedemptionDataError):
status_code = 409
error_string = _(
"It appears that your browser sent multiple redemption requests. You can see all successful redemptions below."
Expand Down
10 changes: 5 additions & 5 deletions evap/staff/importers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def has_errors(self) -> bool:

def raise_if_has_errors(self) -> None:
if self.has_errors():
raise ImporterException(message="")
raise ImporterError(message="")

def success_messages(self) -> list[ImporterLogEntry]:
return self._messages_with_level_sorted_by_category(ImporterLogEntry.Level.SUCCESS)
Expand Down Expand Up @@ -117,7 +117,7 @@ def add_success(self, message_text, *, category=ImporterLogEntry.Category.GENERA
return self.add_message(ImporterLogEntry(ImporterLogEntry.Level.SUCCESS, category, message_text))


class ImporterException(Exception):
class ImporterError(Exception):
"""Used to abort the import run immediately"""

def __init__(
Expand Down Expand Up @@ -145,7 +145,7 @@ def __enter__(self):
pass

def __exit__(self, exc_type, exc_value, traceback) -> bool:
if isinstance(exc_value, ImporterException):
if isinstance(exc_value, ImporterError):
# Importers raise these to immediately abort with a message
if exc_value.message:
self.importer_log.add_message(exc_value.as_importer_message())
Expand Down Expand Up @@ -202,7 +202,7 @@ def map(self, file_content: bytes):
try:
book = openpyxl.load_workbook(BytesIO(file_content))
except Exception as e:
raise ImporterException(
raise ImporterError(
message=_("Couldn't read the file. Error: {}").format(e),
category=ImporterLogEntry.Category.SCHEMA,
) from e
Expand All @@ -213,7 +213,7 @@ def map(self, file_content: bytes):
continue

if sheet.max_column != self.row_cls.column_count:
raise ImporterException(
raise ImporterError(
message=_("Wrong number of columns in sheet '{}'. Expected: {}, actual: {}").format(
sheet.title, self.row_cls.column_count, sheet.max_column
)
Expand Down
36 changes: 18 additions & 18 deletions evap/staff/importers/enrollment.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def all_fields_valid(course_data: CourseData) -> TypeGuard[ValidCourseData]:


class DegreeImportMapper:
class InvalidDegreeNameException(Exception):
class InvalidDegreeNameError(Exception):
def __init__(self, *args, invalid_degree_name: str, **kwargs):
self.invalid_degree_name = invalid_degree_name
super().__init__(*args, **kwargs)
Expand All @@ -105,11 +105,11 @@ def degree_from_import_string(self, import_string: str) -> Degree:
try:
return self.degrees[lookup_key]
except KeyError as e:
raise self.InvalidDegreeNameException(invalid_degree_name=trimmed_name) from e
raise self.InvalidDegreeNameError(invalid_degree_name=trimmed_name) from e


class CourseTypeImportMapper:
class InvalidCourseTypeException(Exception):
class InvalidCourseTypeError(Exception):
def __init__(self, *args, invalid_course_type: str, **kwargs):
super().__init__(*args, **kwargs)
self.invalid_course_type: str = invalid_course_type
Expand All @@ -126,11 +126,11 @@ def course_type_from_import_string(self, import_string: str) -> CourseType:
try:
return self.course_types[stripped_name.lower()]
except KeyError as e:
raise self.InvalidCourseTypeException(invalid_course_type=stripped_name) from e
raise self.InvalidCourseTypeError(invalid_course_type=stripped_name) from e


class IsGradedImportMapper:
class InvalidIsGradedException(Exception):
class InvalidIsGradedError(Exception):
def __init__(self, *args, invalid_is_graded: str, **kwargs):
super().__init__(*args, **kwargs)
self.invalid_is_graded: str = invalid_is_graded
Expand All @@ -143,7 +143,7 @@ def is_graded_from_import_string(cls, is_graded: str) -> bool:
if is_graded == settings.IMPORTER_GRADED_NO:
return False

raise cls.InvalidIsGradedException(invalid_is_graded=is_graded)
raise cls.InvalidIsGradedError(invalid_is_graded=is_graded)


@dataclass
Expand Down Expand Up @@ -231,21 +231,21 @@ def _map_row(self, row: EnrollmentInputRow) -> EnrollmentParsedRow:
degrees: MaybeInvalid[set[Degree]]
try:
degrees = {self.degree_mapper.degree_from_import_string(row.evaluation_degree_name)}
except DegreeImportMapper.InvalidDegreeNameException as e:
except DegreeImportMapper.InvalidDegreeNameError as e:
degrees = invalid_value
self.invalid_degrees_tracker.add_location_for_key(row.location, e.invalid_degree_name)

course_type: MaybeInvalid[CourseType]
try:
course_type = self.course_type_mapper.course_type_from_import_string(row.evaluation_course_type_name)
except CourseTypeImportMapper.InvalidCourseTypeException as e:
except CourseTypeImportMapper.InvalidCourseTypeError as e:
course_type = invalid_value
self.invalid_course_types_tracker.add_location_for_key(row.location, e.invalid_course_type)

is_graded: MaybeInvalid[bool]
try:
is_graded = self.is_graded_mapper.is_graded_from_import_string(row.evaluation_is_graded)
except IsGradedImportMapper.InvalidIsGradedException as e:
except IsGradedImportMapper.InvalidIsGradedError as e:
is_graded = invalid_value
self.invalid_is_graded_tracker.add_location_for_key(row.location, e.invalid_is_graded)

Expand Down Expand Up @@ -305,15 +305,15 @@ def _log_aggregated_messages(self) -> None:


class CourseMergeLogic:
class MergeException(Exception):
class MergeError(Exception):
def __init__(self, *args, merge_hindrances: list[str], **kwargs):
super().__init__(*args, **kwargs)
self.merge_hindrances: list[str] = merge_hindrances

class NameDeCollisionException(Exception):
class NameDeCollisionError(Exception):
"""Course with same name_de, but different name_en exists"""

class NameEnCollisionException(Exception):
class NameEnCollisionError(Exception):
"""Course with same name_en, but different name_de exists"""

def __init__(self, semester: Semester):
Expand Down Expand Up @@ -370,10 +370,10 @@ def set_course_merge_target(self, course_data: CourseData) -> None:

if course_with_same_name_en != course_with_same_name_de:
if course_with_same_name_en is not None:
raise self.NameEnCollisionException()
raise self.NameEnCollisionError()

if course_with_same_name_de is not None:
raise self.NameDeCollisionException()
raise self.NameDeCollisionError()

assert course_with_same_name_en is not None
assert course_with_same_name_de is not None
Expand All @@ -383,7 +383,7 @@ def set_course_merge_target(self, course_data: CourseData) -> None:

merge_hindrances = self.get_merge_hindrances(course_data, merge_candidate)
if merge_hindrances:
raise self.MergeException(merge_hindrances=merge_hindrances)
raise self.MergeError(merge_hindrances=merge_hindrances)

course_data.merge_into_course = merge_candidate

Expand Down Expand Up @@ -414,15 +414,15 @@ def check_course_data(self, course_data: CourseData, location: ExcelFileLocation
try:
self.course_merge_logic.set_course_merge_target(course_data)

except CourseMergeLogic.MergeException as e:
except CourseMergeLogic.MergeError as e:
self.course_merge_impossible_tracker.add_location_for_key(
location, (course_data.name_en, tuple(e.merge_hindrances))
)

except CourseMergeLogic.NameDeCollisionException:
except CourseMergeLogic.NameDeCollisionError:
self.name_de_collision_tracker.add_location_for_key(location, course_data.name_de)

except CourseMergeLogic.NameEnCollisionException:
except CourseMergeLogic.NameEnCollisionError:
self.name_en_collision_tracker.add_location_for_key(location, course_data.name_en)

if course_data.merge_into_course != invalid_value and course_data.merge_into_course:
Expand Down