Skip to content

Commit

Permalink
fix: return form error messages (#223)
Browse files Browse the repository at this point in the history
- when the content item type is AUDIO_VIDEO or PDF_DOCUMENT, and the relevant media/document type is not attached,
  return form error message rather than INTERNAL server error(500)

Signed-off-by: Kathurima Kimathi <kathurimakimathi415@gmail.com>
  • Loading branch information
KathurimaKimathi committed Jul 26, 2023
1 parent 15add73 commit 08e7f7e
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 44 deletions.
4 changes: 4 additions & 0 deletions mycarehub/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest
from django.conf import settings
from django.contrib.auth.models import Group, Permission
from django.contrib.messages.storage.fallback import FallbackStorage
from django.utils import timezone
from faker import Faker
from model_bakery import baker
Expand Down Expand Up @@ -72,6 +73,9 @@ def request_with_user(rf, django_user_model, user_with_all_permissions):
url = settings.ADMIN_URL + "/common/organisation/add/"
request = rf.get(url)
request.user = user_with_all_permissions
setattr(request, "session", "session")
messages = FallbackStorage(request)
setattr(request, "_messages", messages)
return request


Expand Down
94 changes: 94 additions & 0 deletions mycarehub/content/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import pytest
from django.contrib import messages
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import override_settings
from django.urls import reverse
Expand All @@ -7,6 +8,7 @@

from mycarehub.clients.models import Client
from mycarehub.content.models import CustomDocument, CustomImage, CustomMedia
from mycarehub.content.wagtail_hooks import before_publish_page

from ..models import (
Author,
Expand Down Expand Up @@ -472,3 +474,95 @@ def test_custom_document_add_existing(admin_user, user_with_all_permissions, cli
print(response.content)

assert response.status_code == status.HTTP_200_OK


def test_content_view_with_no_attached_audio_video_media(
content_item_with_tag_and_category, request_with_user, client
):
client.force_login(request_with_user.user)
url = reverse("api:contentview-list")

page = content_item_with_tag_and_category
page.item_type = "AUDIO_VIDEO"
page.save()
page.refresh_from_db()

# Simulate the message error and handle it with the messages framework
request_with_user._messages = messages.storage.default_storage(request_with_user)
before_publish_page(request_with_user, page)

messages_str = [str(message) for message in request_with_user._messages]

expected_message = (
"an AUDIO_VIDEO content item must have at least one video "
"or audio file before publication"
)

# Strip extra newlines and whitespaces
messages_str = [msg.strip() for msg in messages_str]

assert expected_message in messages_str

client_one = baker.make(Client)

response = client.post(
url,
data={
"client": client_one.id,
"content_item": page.id,
},
content_type="application/json",
accept="application/json",
)

print(response.content)
assert response.status_code == status.HTTP_201_CREATED

response_data = response.json()
assert response_data["id"] != ""


def test_content_view_with_no_attached_pdf_document_media(
content_item_with_tag_and_category, request_with_user, client
):
client.force_login(request_with_user.user)
url = reverse("api:contentview-list")

page = content_item_with_tag_and_category
page.item_type = "PDF_DOCUMENT"
page.save()
page.refresh_from_db()

# Simulate the message error and handle it with the messages framework
request_with_user._messages = messages.storage.default_storage(request_with_user)
before_publish_page(request_with_user, page)

messages_str = [str(message) for message in request_with_user._messages]

expected_message = (
"a PDF_DOCUMENT content item must have at least one document "
"attached before publication"
)

# Strip extra newlines and whitespaces
messages_str = [msg.strip() for msg in messages_str]

assert expected_message in messages_str

client_one = baker.make(Client)

response = client.post(
url,
data={
"client": client_one.id,
"content_item": page.id,
},
content_type="application/json",
accept="application/json",
)

print(response.content)
assert response.status_code == status.HTTP_201_CREATED

response_data = response.json()
assert response_data["id"] != ""
41 changes: 0 additions & 41 deletions mycarehub/content/tests/test_wagtail_hooks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import pytest
from django.core.exceptions import ValidationError
from django.utils import timezone
from model_bakery import baker
from wagtail.models import Page, Site
Expand All @@ -26,46 +25,6 @@ def test_get_global_admin_js():
assert "DOMContentLoaded" in admin_script


def test_before_publish_content_item_audio_video_no_media(
request_with_user,
content_item_with_tag_and_category,
):
page = content_item_with_tag_and_category
page.item_type = "AUDIO_VIDEO"
page.save()
page.refresh_from_db()
assert page.featured_media.count() == 0
assert page.item_type == "AUDIO_VIDEO"

with pytest.raises(ValidationError) as c:
before_publish_page(request_with_user, page)

assert (
"an AUDIO_VIDEO content item must have at least one video "
"or audio file before publication"
) in str(c.value.messages)


def test_before_publish_content_item_pdf_document_no_document(
request_with_user,
content_item_with_tag_and_category,
):
page = content_item_with_tag_and_category
page.item_type = "PDF_DOCUMENT"
page.save()
page.refresh_from_db()
assert page.documents.count() == 0
assert page.item_type == "PDF_DOCUMENT"

with pytest.raises(ValidationError) as c:
before_publish_page(request_with_user, page)

assert (
"a PDF_DOCUMENT content item must have at least one document "
"attached before publication"
) in str(c.value.messages)


def test_before_publish_non_content_item_page(request_with_user):
site = Site.find_for_request(request_with_user)
assert site is not None
Expand Down
9 changes: 6 additions & 3 deletions mycarehub/content/wagtail_hooks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.core.exceptions import ValidationError
from django.shortcuts import redirect
from django.utils.safestring import mark_safe
from wagtail.admin import messages
from wagtail.core import hooks
from wagtail.documents import get_document_model
from wagtail.images import get_image_model
Expand Down Expand Up @@ -46,14 +47,16 @@ def before_publish_page(request, page):
"an AUDIO_VIDEO content item must have at least one video "
"or audio file before publication"
)
raise ValidationError(message)
messages.error(request, message)
return redirect("wagtailadmin_pages:edit", page.pk)

if page.item_type == "PDF_DOCUMENT" and page.documents.count() == 0:
message = (
"a PDF_DOCUMENT content item must have at least one document "
"attached before publication"
)
raise ValidationError(message)
messages.error(request, message)
return redirect("wagtailadmin_pages:edit", page.pk)


@hooks.register("after_create_page")
Expand Down

0 comments on commit 08e7f7e

Please sign in to comment.