Fix client terms checkbox confirmation

[delano]

This pull request fixes the issue where the client has to confirm the terms checkbox each time a delivery request is made. The following changes have been made:

• Updated models to standardize address, contact info, and status tracking

• Added RequestViewSet and RequestSerializer

• Deleted Dockerfile

• Added unit tests for RequestViewSet

• Regenerated migrations

• Renamed codium config

• Updated test config

Please review and merge this pull request to resolve the issue.

[qodo-merge-pro]

PR Review

⏱️ Estimated effort to review [1-5]	4, due to the extensive changes across multiple files including models, serializers, views, and migrations. The complexity of the changes, especially in the models and migrations, requires careful review to ensure data integrity and application functionality are maintained.
🧪 Relevant tests	Yes
🔍 Possible issues	Possible Bug: The RequestSerializer in apps/api/afbcore/serializers/__init__.py includes fields that do not exist in the Request model, such as profile, food_types_available, safe_drop, request_notes, driver_comments, picture_of_delivery, and needs_review. This discrepancy will cause serialization errors.
	Data Integrity: The Request model in apps/api/afbcore/models/request.py changes the foreign key from profile to user without a corresponding migration to handle existing data. This could lead to data loss or integrity issues.
	Security Concern: The RequestViewSet in apps/api/afbcore/views/__init__.py does not implement any permission classes, potentially allowing unauthorized access to sensitive request data.
🔒 Security concerns	No

[qodo-merge-pro]

PR Code Suggestions

Category	Suggestions
Enhancement	Use Django's models.UUIDField for consistency. Consider using models.UUIDField from Django directly instead of model_utils.fields.UUIDField for consistency with other UUID fields in your models. This change will simplify the model field imports and maintain consistency across your model definitions. apps/api/afbcore/migrations/0001_initial.py [70-75] -model_utils.fields.UUIDField( +models.UUIDField( default=uuid.uuid4, editable=False, primary_key=True, serialize=False, )
	Set a descriptive default for address_details JSONField. For the address_details JSONField, consider setting a more descriptive default value that outlines the expected structure of the JSON object. This can help developers understand what kind of data to expect in this field and ensure that the application handles the JSON structure consistently. apps/api/afbcore/models/request.py [54] -address_details = models.JSONField(default=dict) +address_details = models.JSONField(default=lambda: {"floor": "", "unit": "", "notes": ""})
	Implement permission checks for user list view access. Instead of completely disabling the list view by returning User.objects.none() in get_queryset, consider implementing permission checks to allow certain users (e.g., admins) to access the list view. This approach provides more flexibility and control over who can view the list of users. apps/api/afbcore/views/users.py [54-55] if self.action == "list": - return User.objects.none() # disable list view + if request.user.is_admin: + return User.objects.all() + else: + return User.objects.none()
	Add tests for failure cases and edge cases. To ensure the test suite remains relevant and accurate, consider adding tests that cover failure cases and edge cases. For example, tests for unauthorized access, invalid data submissions, and handling of non-existent resources. This will help improve the robustness and reliability of the application. apps/api/afbcore/tests/views/test__requests__view.py [44] -self.assertEqual(response.status_code, status.HTTP_200_OK) +# Example of a test for unauthorized access +def test_list_requests_unauthorized(self): + request = self.factory.get(self.url) + response = self.view(request) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
	Ensure consistent permission handling by overriding update methods. The destroy method in RequestViewSet raises MethodNotAllowed for DELETE requests, which is good for preventing deletion. However, it's recommended to also override the update and partial_update methods if updates are not allowed, or to ensure proper permission checks are in place if they are. apps/api/afbcore/views/init.py [27-28] -def destroy(self, request, *args, **kwargs): - raise exceptions.MethodNotAllowed("DELETE") +def update(self, request, *args, **kwargs): + raise exceptions.MethodNotAllowed("PUT") +def partial_update(self, request, *args, **kwargs): + raise exceptions.MethodNotAllowed("PATCH")
	Expand docstrings for clarity and comprehensive understanding. While adding docstrings to the base classes is a good practice for clarity, it's also important to ensure these docstrings are comprehensive and accurately describe the purpose and usage of the class. Consider expanding the docstrings to provide more detailed information about each class's role and how it should be used within the application. apps/api/afbcore/models/base.py [26-34] -"""A common base class for core model managers""" -"""A common base class for core model querysets""" -"""A common base class for core models +"""Manages database queries for models, providing a layer of abstraction over database operations.""" +"""Represents a collection of database queries for a specific model, allowing for complex filtering and manipulation of data sets.""" +"""Serves as the foundational class for all models, encapsulating common fields and behaviors such as UUIDs, soft deletion, and timestamping."""
	Improve test isolation by mocking database calls. The test case test_retrieve_all_requests does not mock the database calls, which can lead to tests that are dependent on the database state. Consider using Django's TestCase class to wrap each test method in a transaction or use mocking libraries like unittest.mock to mock the Request.objects.count() and RequestViewSet.list method calls. apps/api/afbcore/tests/views/test_requests.py [16-28] -class TestRequestViewSet: - def test_retrieve_all_requests(self): +from unittest.mock import patch +class TestRequestViewSet(TestCase): + @patch('apps.api.afbcore.views.RequestViewSet.list') + @patch('apps.api.afbcore.models.Request.objects.count') + def test_retrieve_all_requests(self, mock_list, mock_count): + mock_count.return_value = 10 + mock_list.return_value = Response(data=list(range(10)), status=200) viewset = RequestViewSet() request = RequestFactory().get("/") response = viewset.list(request) assert response.status_code == 200 - assert len(response.data) == Request.objects.count() + assert len(response.data) == mock_count.return_value
Best practice	Use APIClient for simpler and cleaner tests. To improve code readability and maintainability, consider refactoring the test cases to use Django's APIClient instead of APIRequestFactory and force_authenticate. APIClient provides a higher-level API that simplifies making requests and automatically handles authentication, making the tests cleaner and easier to understand. apps/api/afbcore/tests/views/test_userviewset.py [24] -self.factory = APIRequestFactory() -force_authenticate(request, user=self.user) +from rest_framework.test import APIClient +self.client = APIClient() +self.client.force_authenticate(user=self.user)
Bug	Correct a typo in the field name. It appears there's a typo in the field name modeified which should likely be modified. Correcting this typo will ensure the serializer accurately reflects the model fields. apps/api/afbcore/serializers/init.py [27] -"modeified", +"modified",
	Correct an incorrect import statement. The import statement from blobfile import Request seems incorrect as it shadows the Request model import from Django. If the intention was to use Django's HttpRequest, the correct import should be from django.http import HttpRequest. apps/api/afbcore/tests/views/test_requests.py [13] -from blobfile import Request +from django.http import HttpRequest

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[qodo-merge-pro]

CI Failure Feedback

(Checks updated until commit 22871a8)

Action: install-and-cache (ubuntu-latest, 3.11, 5)

Failed stage: Install django 5 [❌]

Failure summary: The action failed because the virtual environment activation script .venv/bin/activate was not found. This likely means that the virtual environment was not properly set up or the path specified is incorrect.

Relevant error logs: 1: ##[group]Operating System 2: Ubuntu ... 625: -rw-r--r-- 1 runner docker 1709 Mar 31 18:57 .env.example 626: -rw-r--r-- 1 runner docker 0 Mar 31 18:57 __init__.py 627: drwxr-xr-x 3 runner docker 4096 Mar 31 18:57 afb 628: drwxr-xr-x 10 runner docker 4096 Mar 31 18:57 afbcore 629: -rwxr-xr-x 1 runner docker 659 Mar 31 18:57 manage.py 630: -rw-r--r-- 1 runner docker 108574 Mar 31 18:57 poetry.lock 631: drwxr-xr-x 3 runner docker 4096 Mar 31 18:57 static 632: /home/runner/work/_temp/dc3296db-2757-4a7d-910b-160db3b19b22.sh: line 2: .venv/bin/activate: No such file or directory 633: ##[error]Process completed with exit code 1.

---

✨ CI feedback usage guide:

---

The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:

• Failed stage
• Failed test name
• Failure summary
• Relevant error logs

In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

/checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"

where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

Configuration options

• enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
• excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
• enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
• persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
• final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

See more information about the checks tool in the docs.