Skip to content
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
6 changes: 4 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
name: Python CI
# This workflow can be troubleshooted with act https://github.com/nektos/act
# For example: act --job run_tests --matrix toxenv:e2e
name: Run tests

on:
push:
Expand All @@ -20,7 +22,7 @@ jobs:
matrix:
os: [ubuntu-latest]
python-version: ['3.11', '3.12']
toxenv: [py, quality, django42]
toxenv: [py, quality, django42, e2e]

steps:
- uses: actions/checkout@v4
Expand Down
11 changes: 10 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ piptools: ## install pinned version of pip-compile and pip-sync
requirements: clean_tox piptools ## install development environment requirements
pip-sync -q requirements/dev.txt requirements/private.*

test-all: clean test test-quality test-pii selfcheck ## run all tests
test-all: selfcheck clean test test-quality test-pii test-e2e ## run all tests

test: ## run unit tests
pytest
Expand All @@ -94,6 +94,15 @@ test-pii: export DJANGO_SETTINGS_MODULE=forum.settings.test
test-pii: ## # check for PII annotations on all Django models
code_annotations django_find_annotations --config_file .pii_annotations.yml --lint --report --coverage

test-e2e: e2e-stop-services e2e-start-services # run end-to-end tests
pytest tests/e2e

e2e-start-services: # Start dependency containers necessary for e2e tests
docker compose -f tests/e2e/docker-compose.yml --project-name forum_e2e up -d

e2e-stop-services: # Stop dependency containers necessary for e2e tests
docker compose -f tests/e2e/docker-compose.yml --project-name forum_e2e down

selfcheck: ## check that the Makefile is well-formed
@echo "The Makefile is well-formed."

Expand Down
18 changes: 16 additions & 2 deletions forum/models/model_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1237,9 +1237,23 @@ def build_course_stats(author_id: str, course_id: str) -> None:
"$group": {
"_id": {"type": "$_type", "is_reply": "$is_reply"},
"count": {"$sum": 1},
"active_flags": {"$sum": {"$gt": [{"$size": "$abuse_flaggers"}, 0]}},
"active_flags": {
"$sum": {
"$cond": {
"if": {"$gt": [{"$size": "$abuse_flaggers"}, 0]},
"then": 1,
"else": 0,
}
}
},
"inactive_flags": {
"$sum": {"$gt": [{"$size": "$historical_abuse_flaggers"}, 0]}
"$sum": {
"$cond": {
"if": {"$gt": [{"$size": "$historical_abuse_flaggers"}, 0]},
"then": 1,
"else": 0,
}
}
},
"latest_update_at": {"$max": "$updated_at"},
}
Expand Down
1 change: 0 additions & 1 deletion forum/search/comment_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from typing import Any, Optional

from forum.constants import FORUM_MAX_DEEP_SEARCH_COMMENT_COUNT

from forum.models import CommentThread
from forum.search.es import ElasticsearchModelMixin

Expand Down
2 changes: 1 addition & 1 deletion forum/search/es.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from django.conf import settings
from elasticsearch import Elasticsearch

from forum.models import BaseContents, MODEL_INDICES
from forum.models import MODEL_INDICES, BaseContents

__all__ = ["Elasticsearch", "ElasticsearchModelMixin"]

Expand Down
2 changes: 1 addition & 1 deletion forum/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def root(*args: str) -> str:
FORUM_MONGODB_CLIENT_PARAMETERS: dict[str, str] = {}
FORUM_MONGODB_AUTH_PARAMETERS: dict[str, str] = {}

FORUM_ENABLE_ELASTIC_SEARCH = False
FORUM_ENABLE_ELASTIC_SEARCH = True
if FORUM_ENABLE_ELASTIC_SEARCH:
FORUM_ELASTIC_SEARCH_CONFIG = [
{
Expand Down
1 change: 1 addition & 0 deletions forum/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
comment_updated = Signal()
comment_thread_updated = Signal()

# TODO this setting should not exist. We need elasticsearch in production, and there is no way around it.
if settings.FORUM_ENABLE_ELASTIC_SEARCH:
# Connect the handlers when FORUM_ENABLE_ELASTIC_SEARCH is enabled.
comment_deleted.connect(handle_comment_deletion)
Expand Down
38 changes: 18 additions & 20 deletions forum/views/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,52 +31,49 @@ def _validate_and_extract_params(self, request: Request) -> dict[str, Any]:
"""
Validate and extract query parameters from the request.
"""
data = request.query_params
params: dict[str, Any] = {}

# Required parameters
text = request.GET.get("text")
text = data.get("text")
if not text:
raise ValueError("text is required")
params["text"] = text

# Sort key validation
VALID_SORT_KEYS = ("activity", "comments", "date", "votes")
sort_key = request.GET.get("sort_key", "date")
sort_key = data.get("sort_key", "date")
if sort_key not in VALID_SORT_KEYS:
raise ValueError("invalid sort_key")
params["sort_key"] = sort_key

# Pagination handling
page = request.GET.get("page", FORUM_DEFAULT_PAGE)
page = data.get("page", FORUM_DEFAULT_PAGE)
try:
params["page"] = int(page)
except ValueError as exc:
raise ValueError("Invalid page value.") from exc

per_page = request.GET.get("per_page", FORUM_DEFAULT_PER_PAGE)
per_page = data.get("per_page", FORUM_DEFAULT_PER_PAGE)
try:
params["per_page"] = int(per_page)
except ValueError as exc:
raise ValueError("Invalid per_page value.") from exc

# Optional parameters with default values and type conversion
params["context"] = request.GET.get("context", "course")
params["user_id"] = request.GET.get("user_id", "")
params["course_id"] = request.GET.get("course_id", "")
params["author_id"] = request.GET.get("author_id")
params["thread_type"] = request.GET.get("thread_type")
params["flagged"] = request.GET.get("flagged", "false").lower() == "true"
params["unread"] = request.GET.get("unread", "false").lower() == "true"
params["unanswered"] = request.GET.get("unanswered", "false").lower() == "true"
params["unresponded"] = (
request.GET.get("unresponded", "false").lower() == "true"
)
params["count_flagged"] = (
request.GET.get("count_flagged", "false").lower() == "true"
)
params["context"] = data.get("context", "course")
params["user_id"] = data.get("user_id", "")
params["course_id"] = data.get("course_id", "")
params["author_id"] = data.get("author_id")
params["thread_type"] = data.get("thread_type")
params["flagged"] = data.get("flagged", "false").lower() == "true"
params["unread"] = data.get("unread", "false").lower() == "true"
params["unanswered"] = data.get("unanswered", "false").lower() == "true"
params["unresponded"] = data.get("unresponded", "false").lower() == "true"
params["count_flagged"] = data.get("count_flagged", "false").lower() == "true"

# Group IDs extraction
params["group_ids"] = get_group_ids_from_params(request.GET)
params["group_ids"] = get_group_ids_from_params(data)

return params

Expand Down Expand Up @@ -122,13 +119,14 @@ def get(self, request: Request) -> Response:
Returns:
Response: A JSON response containing the search results, corrected text (if any), and total results.
"""

try:
params = self._validate_and_extract_params(request)
except ValueError as error:
return Response({"error": str(error)}, status=status.HTTP_400_BAD_REQUEST)

thread_ids, corrected_text = self._get_thread_ids_from_indexes(
params["context"], params["group_ids"], request.GET, params["text"]
params["context"], params["group_ids"], request.query_params, params["text"]
)

data: dict[str, Any] = handle_threads_query(
Expand Down
13 changes: 7 additions & 6 deletions forum/views/threads.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from rest_framework.serializers import ValidationError
from rest_framework.views import APIView

from forum.models.comments import Comment
from forum.models.model_utils import (
delete_comments_of_a_thread,
delete_subscriptions_of_a_thread,
Expand Down Expand Up @@ -166,9 +165,10 @@ def delete(self, request: Request, thread_id: str) -> Response:
result = CommentThread().delete(thread_id)
delete_subscriptions_of_a_thread(thread_id)
if result:
update_stats_for_course(
thread["author_id"], thread["course_id"], threads=-1
)
if not (thread["anonymous"] or thread["anonymous_to_peers"]):
update_stats_for_course(
thread["author_id"], thread["course_id"], threads=-1
)

return Response(serialized_data, status=status.HTTP_200_OK)

Expand All @@ -185,7 +185,7 @@ def put(self, request: Request, thread_id: str) -> Response:
The details of the thread that is updated.
"""
try:
thread = validate_object(Comment, thread_id)
thread = validate_object(CommentThread, thread_id)
except ObjectDoesNotExist:
return Response(
{"error": "thread does not exist"},
Expand Down Expand Up @@ -280,7 +280,8 @@ def post(self, request: Request) -> Response:
)

thread = self.create_thread(data)
update_stats_for_course(thread["author_id"], thread["course_id"], threads=1)
if not (thread["anonymous"] or thread["anonymous_to_peers"]):
update_stats_for_course(thread["author_id"], thread["course_id"], threads=1)
try:
serialized_data = prepare_thread_api_response(thread, True, data)
except ValidationError as error:
Expand Down
12 changes: 6 additions & 6 deletions requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
#
asgiref==3.8.1
# via django
certifi==2024.7.4
certifi==2024.8.30
# via
# elasticsearch
# requests
charset-normalizer==3.3.2
# via requests
django==4.2.15
django==4.2.16
# via
# -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt
# -r requirements/base.in
Expand All @@ -25,17 +25,17 @@ elasticsearch==7.13.4
# via
# -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt
# -r requirements/base.in
idna==3.7
idna==3.10
# via requests
openedx-atlas==0.6.1
openedx-atlas==0.6.2
# via -r requirements/base.in
pymongo==4.8.0
pymongo==4.9.1
# via -r requirements/base.in
requests==2.32.3
# via -r requirements/base.in
sqlparse==0.5.1
# via django
urllib3==1.26.19
urllib3==1.26.20
# via
# elasticsearch
# requests
Loading