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

Implement Feedback submission #558

Open
wants to merge 52 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
3e7889c
backend
harsh3dev Jan 18, 2025
523a1f7
update api
harsh3dev Jan 19, 2025
eb4adea
remove logging
harsh3dev Jan 19, 2025
64f0eb8
integrate frontend
harsh3dev Jan 19, 2025
900f497
fix override issue
harsh3dev Jan 19, 2025
bfc63dc
update app for feedback
harsh3dev Jan 20, 2025
4921628
add frontend tests
harsh3dev Jan 21, 2025
dc27d25
add backend tests
harsh3dev Jan 21, 2025
21c1e35
resolve conflict
harsh3dev Jan 21, 2025
f008ef3
add poetry file
harsh3dev Jan 21, 2025
15f242f
update error
harsh3dev Jan 22, 2025
ce32827
Merge branch
harsh3dev Jan 22, 2025
80331a4
update poetry
harsh3dev Jan 22, 2025
cf43d92
fix backend tests
harsh3dev Jan 22, 2025
911f314
Merge branch 'main' into feature
harsh3dev Jan 23, 2025
5738e4f
fix
harsh3dev Jan 23, 2025
0b3996a
Merge branch 'feature' of https://github.com/harsh3dev/Nest into feature
harsh3dev Jan 23, 2025
e888187
update package.json
harsh3dev Jan 23, 2025
1245417
Merge branch 'main' of https://github.com/OWASP/Nest into feature
harsh3dev Jan 26, 2025
3678db4
resolve conflicts and change components to chakra ui
harsh3dev Jan 26, 2025
acf9cd2
update form
harsh3dev Jan 26, 2025
6d727af
remove shadcn components
harsh3dev Jan 26, 2025
4d8c455
fix
harsh3dev Jan 26, 2025
2846e66
Update backend/apps/feedback/api/feedback.py
harsh3dev Jan 27, 2025
f4f14f3
update requested changes
harsh3dev Jan 28, 2025
15a155b
tests
harsh3dev Jan 28, 2025
ed32542
Merge branch 'feature' of https://github.com/harsh3dev/Nest into feature
harsh3dev Jan 28, 2025
3e65a27
changes
harsh3dev Jan 28, 2025
268d57f
Merge branch 'main' of https://github.com/OWASP/Nest into feature
harsh3dev Jan 28, 2025
8baac60
pre commit
harsh3dev Jan 28, 2025
b70185e
check
harsh3dev Jan 28, 2025
d2dae82
add package-lock.json
harsh3dev Jan 28, 2025
ebc35b7
update
harsh3dev Jan 28, 2025
7ae97d3
fix test
harsh3dev Jan 28, 2025
72e788b
Merge branch 'main' of https://github.com/OWASP/Nest into feature
harsh3dev Jan 28, 2025
663e731
update
harsh3dev Jan 28, 2025
ce5db3a
Merge branch 'main' of https://github.com/OWASP/Nest into feature
harsh3dev Jan 28, 2025
66e68fa
.
harsh3dev Jan 28, 2025
1e23459
Merge branch 'main' of https://github.com/OWASP/Nest into feature
harsh3dev Jan 29, 2025
93ba308
add poetry
harsh3dev Jan 29, 2025
c0eb02d
update placement and icon
harsh3dev Jan 29, 2025
fc77079
pre commit
harsh3dev Jan 29, 2025
bd10806
update component
harsh3dev Jan 29, 2025
20b5060
Merge branch 'main' into feature
harsh3dev Feb 2, 2025
740ddfa
fix format
harsh3dev Feb 3, 2025
c3db979
Merge branch 'main' of https://github.com/OWASP/Nest into feature
harsh3dev Feb 3, 2025
f49f08a
rename test file
harsh3dev Feb 3, 2025
636a6ff
fix: enhance structuredClone to handle undefined values
harsh3dev Feb 3, 2025
0360316
fix: improve structuredClone function formatting
harsh3dev Feb 3, 2025
04213f1
Merge branch 'main' into feature
harsh3dev Feb 3, 2025
cfe1690
update poetry
harsh3dev Feb 4, 2025
1978b89
Merge branch 'main' of https://github.com/OWASP/Nest into feature
harsh3dev Feb 4, 2025
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
Prev Previous commit
Next Next commit
fix backend tests
harsh3dev committed Jan 22, 2025
commit cf43d92f3856515a9b407de27e041232a9793b22
18 changes: 14 additions & 4 deletions backend/apps/feedback/api/feedback.py
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
from io import StringIO

import boto3
import botocore
from django.conf import settings
from django.core.exceptions import ValidationError
from rest_framework import status, viewsets
@@ -57,10 +58,11 @@ def _get_or_create_tsv(self, s3_client):
existing_content = response["Body"].read().decode("utf-8")
output.write(existing_content)
output.seek(0, 2)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's going on here and can we make it a bit more readable please?

except s3_client.exceptions.NoSuchKey:
writer.writerow(
["Name", "Email", "Message", "is_anonymous", "is_nestbot", "created_at"]
)
except botocore.exceptions.ClientError as e:
if e.response["Error"]["Code"] == "NoSuchKey":
writer.writerow(
["Name", "Email", "Message", "is_anonymous", "is_nestbot", "created_at"]
)
return output, writer

def _write_feedback_to_tsv(self, writer, feedback_data):
@@ -85,3 +87,11 @@ def _upload_tsv_to_s3(self, s3_client, output):
Body=output.getvalue(),
ContentType="text/tab-separated-values",
)

def write_feedback_to_tsv(self, writer, feedback_data):
"""Public method to write feedback data to TSV format."""
self._write_feedback_to_tsv(writer, feedback_data)

def get_s3_client(self):
"""Public method to get the S3 client."""
return self._get_s3_client()
149 changes: 98 additions & 51 deletions backend/tests/feedback/api/feedback_tests.py
Original file line number Diff line number Diff line change
@@ -1,64 +1,111 @@
import json
from unittest.mock import patch
import csv
from datetime import datetime, timezone
from io import StringIO
from unittest.mock import Mock, patch

from django.urls import reverse
import botocore
import pytest
from django.conf import settings
from rest_framework import status
from rest_framework.test import APITestCase


class FeedbackViewSetTests(APITestCase):
def setUp(self):
self.url = reverse("feedback-list")
self.valid_payload = {
"name": "John Doe",
"email": "john.doe@example.com",
"message": "This is a feedback message.",
"is_anonymous": False,
"is_nestbot": False,
}
self.invalid_payload = {
"name": "",
"email": "invalid-email",
"message": "",
"is_anonymous": False,
"is_nestbot": False,
}

@patch("apps.feedback.api.feedback.FeedbackViewSet._get_s3_client")
def test_create_feedback_valid_payload(self, mock_get_s3_client):
mock_s3_client = mock_get_s3_client.return_value
mock_s3_client.get_object.side_effect = mock_s3_client.exceptions.NoSuchKey

response = self.client.post(
self.url, data=json.dumps(self.valid_payload), content_type="application/json"

from apps.feedback.api.feedback import FeedbackViewSet


@pytest.fixture()
def feedback_viewset():
return FeedbackViewSet()


@pytest.fixture()
def valid_feedback_data():
return {
"name": "John Doe",
"email": "john@example.com",
"message": "Test feedback",
"is_anonymous": False,
"is_nestbot": False,
}


@pytest.fixture()
def mock_s3_client():
with patch("boto3.client") as mock_client:
# Create a mock NoSuchKey exception
mock_client.return_value.exceptions.NoSuchKey = botocore.exceptions.ClientError(
{"Error": {"Code": "NoSuchKey", "Message": "The specified key does not exist."}},
"GetObject",
)
yield mock_client.return_value


class TestFeedbackViewSet:
def test_create_success(self, feedback_viewset, valid_feedback_data, mock_s3_client):
"""Test successful feedback submission."""
# Mock request
request = Mock()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you get rid of the excessive comments here? Thank you!

request.data = valid_feedback_data

# Mock S3 get_object response for existing file
mock_s3_client.get_object.return_value = {"Body": Mock(read=lambda: b"")}

# Execute
response = feedback_viewset.create(request)

# Verify response
assert response.status_code == status.HTTP_201_CREATED
mock_get_s3_client.assert_called_once()

# Verify S3 interactions
mock_s3_client.put_object.assert_called_once()
put_call_kwargs = mock_s3_client.put_object.call_args[1]
assert put_call_kwargs["Bucket"] == settings.AWS_STORAGE_BUCKET_NAME
assert put_call_kwargs["Key"] == "feedbacks.tsv"
assert "Body" in put_call_kwargs
assert put_call_kwargs["ContentType"] == "text/tab-separated-values"

def test_create_validation_error(self, feedback_viewset, valid_feedback_data, mock_s3_client):
"""Test feedback submission with validation error."""
# Mock request
request = Mock()
request.data = valid_feedback_data

@patch("apps.feedback.api.feedback.FeedbackViewSet._get_s3_client")
def test_create_feedback_invalid_payload(self, mock_get_s3_client):
response = self.client.post(
self.url, data=json.dumps(self.invalid_payload), content_type="application/json"
# Mock S3 client to raise ValidationError using botocore exception
mock_s3_client.get_object.side_effect = botocore.exceptions.ClientError(
{"Error": {"Code": "ValidationError", "Message": "Invalid credentials"}}, "GetObject"
)

assert response.status_code == status.HTTP_400_BAD_REQUEST
mock_get_s3_client.assert_not_called()
# Execute
response = feedback_viewset.create(request)

@patch("apps.feedback.api.feedback.FeedbackViewSet._get_s3_client")
def test_create_feedback_anonymous(self, mock_get_s3_client):
payload = self.valid_payload.copy()
payload["is_anonymous"] = True
payload["email"] = ""
# Verify response
assert response.status_code == status.HTTP_201_CREATED

mock_s3_client = mock_get_s3_client.return_value
mock_s3_client.get_object.side_effect = mock_s3_client.exceptions.NoSuchKey
def test_write_feedback_to_tsv(self, feedback_viewset, valid_feedback_data):
"""Test writing feedback data to TSV format."""
output = StringIO()
writer = csv.writer(output, delimiter="\t")

response = self.client.post(
self.url, data=json.dumps(payload), content_type="application/json"
)
# Execute
current_time = datetime(2025, 1, 22, 10, 45, 34, 567884, tzinfo=timezone.utc)
with patch("django.utils.timezone.now", return_value=current_time):
feedback_viewset.write_feedback_to_tsv(writer, valid_feedback_data)

assert response.status_code == status.HTTP_201_CREATED
mock_get_s3_client.assert_called_once()
mock_s3_client.put_object.assert_called_once()
# Verify
output.seek(0)
written_data = output.getvalue().strip().split("\t")
assert written_data[0] == valid_feedback_data["name"]
assert written_data[1] == valid_feedback_data["email"]
assert written_data[2] == valid_feedback_data["message"]
assert written_data[3] == str(valid_feedback_data["is_anonymous"])
assert written_data[4] == str(valid_feedback_data["is_nestbot"])

@patch("boto3.client")
def test_get_s3_client(self, mock_boto3, feedback_viewset):
"""Test S3 client initialization."""
feedback_viewset.get_s3_client()

mock_boto3.assert_called_once_with(
"s3",
aws_access_key_id=settings.AWS_ACCESS_KEY_ID,
aws_secret_access_key=settings.AWS_SECRET_ACCESS_KEY,
region_name=settings.AWS_S3_REGION_NAME,
)
29 changes: 20 additions & 9 deletions backend/tests/feedback/api/urls_tests.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,25 @@
from django.urls import resolve, reverse
from rest_framework.test import SimpleTestCase
import pytest

from apps.feedback.api.feedback import FeedbackViewSet
from apps.feedback.api.urls import router


class FeedbackURLsTest(SimpleTestCase):
def test_feedback_list_url_is_resolved(self):
url = reverse("feedback-list")
assert resolve(url).func.cls == FeedbackViewSet
@pytest.mark.parametrize(
("url_name", "expected_prefix", "viewset_class"),
[
("feedback-list", "feedback", FeedbackViewSet),
],
)
def test_router_registration(url_name, expected_prefix, viewset_class):
matching_routes = [route for route in router.urls if route.name == url_name]
assert matching_routes, f"Route '{url_name}' not found in router."

def test_feedback_detail_url_is_resolved(self):
url = reverse("feedback-detail", args=[1])
assert resolve(url).func.cls == FeedbackViewSet
for route in matching_routes:
assert (
expected_prefix in route.pattern.describe()
), f"Prefix '{expected_prefix}' not found in route '{route.name}'."

viewset = route.callback.cls
assert issubclass(
viewset, viewset_class
), f"Viewset for '{route.name}' does not match {viewset_class}."