Skip to content

Commit

Permalink
[#2540] Fix bug by removing a feature
Browse files Browse the repository at this point in the history
Bug was first reported by @elichad in #2551 (review)

The bug was that page didn't render because of some weird
Python inspection stuff. As it turned out, that inspection was happening
in Django template system when trying to display base class for Python
date.

It was not possible to get rid of that error in a sane matter, so
I decided to remove the feature linking models with their documentation.
  • Loading branch information
pbanaszkiewicz committed Nov 19, 2023
1 parent 1623ae6 commit 1f6f873
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 66 deletions.
20 changes: 0 additions & 20 deletions amy/emails/templatetags/emails.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,10 @@
from django import template
from django.db.models import Model

from emails.models import ScheduledEmailStatus, ScheduledEmailStatusActions

register = template.Library()


@register.filter
def model_documentation_link(model: Model) -> str:
# This is a limited mapping of model to its documentation link. It should
# be expanded as needed.
# If our model documentation grows, we should consider including link to model
# documentation inside every model. Then this mapping would become obsolete.
mapping = {
"InstructorRecruitmentSignup": (
"https://carpentries.github.io/amy/design/database_models/"
"#instructorrecruitmentsignup"
),
"Event": "https://carpentries.github.io/amy/amy_database_structure/#events",
"Award": "https://carpentries.github.io/amy/design/database_models/#award",
"Person": "https://carpentries.github.io/amy/amy_database_structure/#persons",
}
model_name = model.__class__.__name__
return mapping.get(model_name, "")


@register.simple_tag
def allowed_actions_for_status(status: ScheduledEmailStatus) -> list[str]:
return [
Expand Down
39 changes: 1 addition & 38 deletions amy/emails/tests/test_template_tags.py
Original file line number Diff line number Diff line change
@@ -1,44 +1,7 @@
from unittest.mock import MagicMock

from django.test import TestCase

from emails.models import ScheduledEmailStatus
from emails.templatetags.emails import (
allowed_actions_for_status,
model_documentation_link,
)


class TestModelDocumentationLink(TestCase):
def test_model_documentation_link__valid_model(self) -> None:
# Arrange
# Real models are replaced with their metaclass BaseModel, so the tests don't
# work with real models and instead mocks are used.
model = MagicMock()
model.__class__.__name__ = "Person"

# Act
link = model_documentation_link(model)

# Assert
self.assertEqual(
link,
"https://carpentries.github.io/amy/amy_database_structure/#persons",
)

def test_model_documentation_link__invalid_model(self) -> None:
# Arrange
# Real models are replaced with their metaclass BaseModel, so the tests don't
# work with real models and instead mocks are used.
model = MagicMock()
# Badge is not in the mapping yet
model.__class__.__name__ = "Badge"

# Act
link = model_documentation_link(model)

# Assert
self.assertEqual(link, "")
from emails.templatetags.emails import allowed_actions_for_status


class TestAllowedActionsForStatusTag(TestCase):
Expand Down
4 changes: 2 additions & 2 deletions amy/emails/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def test_view(self) -> None:
self.assertEqual(
rv.context["body_context_annotations"],
{
"person": Person,
"person": repr(Person),
},
)

Expand Down Expand Up @@ -147,7 +147,7 @@ def test_view_context(self) -> None:
self.assertEqual(
rv.context["body_context_annotations"],
{
"person": Person,
"person": repr(Person),
},
)

Expand Down
8 changes: 6 additions & 2 deletions amy/emails/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ def get_context_data(self, **kwargs):
context["body_context_annotations"] = {}
if signal:
context["body_context_type"] = signal.context_type
context["body_context_annotations"] = signal.context_type.__annotations__
context["body_context_annotations"] = {
k: repr(v) for k, v in signal.context_type.__annotations__.items()
}
return context


Expand Down Expand Up @@ -96,7 +98,9 @@ def get_context_data(self, **kwargs):
context["body_context_annotations"] = {}
if signal:
context["body_context_type"] = signal.context_type
context["body_context_annotations"] = signal.context_type.__annotations__
context["body_context_annotations"] = {
k: repr(v) for k, v in signal.context_type.__annotations__.items()
}
return context


Expand Down
3 changes: 1 addition & 2 deletions amy/templates/emails/email_template_detail.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{% extends "base_nav.html" %}
{% load emails %}

{% block content %}

Expand Down Expand Up @@ -102,7 +101,7 @@
<td colspan="2">
<ul>
{% for key, value in body_context_annotations.items %}
<li><code>{{ key }}</code>: {{ value|model_documentation_link|urlize }}</li>
<li><code>{{ key }}</code>: {{ value }}</li>
{% endfor %}
</ul>
</td>
Expand Down
3 changes: 1 addition & 2 deletions amy/templates/emails/email_template_edit.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{% extends "base_nav.html" %}

{% load crispy_forms_tags %}
{% load emails %}

{% block content %}

Expand All @@ -14,7 +13,7 @@
<div class="col-12 col-lg-10">
<ul>
{% for key, value in body_context_annotations.items %}
<li><code>{{ key }}</code>: {{ value|model_documentation_link|urlize }}</li>
<li><code>{{ key }}</code>: {{ value }}</li>
{% endfor %}
</ul>
</div>
Expand Down

0 comments on commit 1f6f873

Please sign in to comment.