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
12 changes: 12 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,15 @@
- **State Management**: MobX stores in `packages/shared-state`, reactive patterns
- **Testing**: All features require unit tests, use existing test framework per package
- **Components**: Build in `@plane/ui` with Storybook for isolated development

## Backend tests (Docker)

The Django/pytest suite for `apps/api` runs in an isolated stack defined by `docker-compose-test.yml` at the repo root.

Prereq (once): `./setup.sh` — generates `apps/api/.env` from `.env.example`.

- Full suite: `docker compose -f docker-compose-test.yml up --build --abort-on-container-exit --exit-code-from api-tests`
- Subset: `docker compose -f docker-compose-test.yml run --rm api-tests pytest -m unit`
- Teardown: `docker compose -f docker-compose-test.yml down -v`

See `apps/api/tests/RUNNING_TESTS.md` for the full walkthrough and troubleshooting; see `apps/api/tests/TESTING_GUIDE.md` for test conventions and fixtures.
6 changes: 4 additions & 2 deletions apps/api/plane/api/serializers/cycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@ class Meta:
]

def validate(self, data):
project_id = self.initial_data.get("project_id") or (
self.instance.project_id if self.instance and hasattr(self.instance, "project_id") else None
project_id = (
self.context.get("project_id")
or self.initial_data.get("project_id")
or (self.instance.project_id if self.instance and hasattr(self.instance, "project_id") else None)
)

if not project_id:
Expand Down
8 changes: 6 additions & 2 deletions apps/api/plane/api/views/cycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,9 @@ def post(self, request, slug, project_id):
if (request.data.get("start_date", None) is None and request.data.get("end_date", None) is None) or (
request.data.get("start_date", None) is not None and request.data.get("end_date", None) is not None
):
serializer = CycleCreateSerializer(data=request.data, context={"request": request})
serializer = CycleCreateSerializer(
data=request.data, context={"request": request, "project_id": project_id}
)
if serializer.is_valid():
if (
request.data.get("external_id")
Expand Down Expand Up @@ -516,7 +518,9 @@ def patch(self, request, slug, project_id, pk):
status=status.HTTP_400_BAD_REQUEST,
)

serializer = CycleUpdateSerializer(cycle, data=request.data, partial=True, context={"request": request})
serializer = CycleUpdateSerializer(
cycle, data=request.data, partial=True, context={"request": request, "project_id": project_id}
)
if serializer.is_valid():
if (
request.data.get("external_id")
Expand Down
4 changes: 2 additions & 2 deletions apps/api/plane/app/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def get(self, request: Request, pk: Optional[str] = None) -> Response:
serializer = APITokenReadSerializer(api_tokens, many=True)
return Response(serializer.data, status=status.HTTP_200_OK)
else:
api_tokens = APIToken.objects.get(user=request.user, pk=pk)
api_tokens = APIToken.objects.get(user=request.user, pk=pk, is_service=False)
serializer = APITokenReadSerializer(api_tokens)
return Response(serializer.data, status=status.HTTP_200_OK)

Expand All @@ -54,7 +54,7 @@ def delete(self, request: Request, pk: str) -> Response:
return Response(status=status.HTTP_204_NO_CONTENT)

def patch(self, request: Request, pk: str) -> Response:
api_token = APIToken.objects.get(user=request.user, pk=pk)
api_token = APIToken.objects.get(user=request.user, pk=pk, is_service=False)
serializer = APITokenSerializer(api_token, data=request.data, partial=True)
if serializer.is_valid():
serializer.save()
Expand Down
8 changes: 4 additions & 4 deletions apps/api/plane/bgtasks/work_item_link_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ def validate_url_ip(url: str) -> None:
ValueError: If the URL points to a private/internal IP
"""
parsed = urlparse(url)
hostname = parsed.hostname

if not hostname:
raise ValueError("Invalid URL: No hostname found")

# Only allow HTTP and HTTPS to prevent file://, gopher://, etc.
if parsed.scheme not in ("http", "https"):
raise ValueError("Invalid URL scheme. Only HTTP and HTTPS are allowed")

hostname = parsed.hostname
if not hostname:
raise ValueError("Invalid URL: No hostname found")

# Resolve hostname to IP addresses — this catches domain names that
# point to internal IPs (e.g. attacker.com -> 169.254.169.254)

Expand Down
2 changes: 1 addition & 1 deletion apps/api/plane/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

# Third party imports
from celery import Celery
from pythonjsonlogger.jsonlogger import JsonFormatter
from pythonjsonlogger.json import JsonFormatter
from celery.signals import after_setup_logger, after_setup_task_logger
from celery.schedules import crontab

Expand Down
1 change: 0 additions & 1 deletion apps/api/plane/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@
# Internationalization
LANGUAGE_CODE = "en-us"
USE_I18N = True
USE_L10N = True

# Timezones
USE_TZ = True
Expand Down
2 changes: 1 addition & 1 deletion apps/api/plane/settings/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"style": "{",
},
"json": {
"()": "pythonjsonlogger.jsonlogger.JsonFormatter",
"()": "pythonjsonlogger.json.JsonFormatter",
"fmt": "%(levelname)s %(asctime)s %(module)s %(name)s %(message)s",
},
},
Expand Down
2 changes: 1 addition & 1 deletion apps/api/plane/settings/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"formatters": {
"verbose": {"format": "%(asctime)s [%(process)d] %(levelname)s %(name)s: %(message)s"},
"json": {
"()": "pythonjsonlogger.jsonlogger.JsonFormatter",
"()": "pythonjsonlogger.json.JsonFormatter",
"fmt": "%(levelname)s %(asctime)s %(module)s %(name)s %(message)s",
},
},
Expand Down
1 change: 1 addition & 0 deletions apps/api/plane/tests/contract/api/test_cycles.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def project(db, workspace, create_user):
identifier="TP",
workspace=workspace,
created_by=create_user,
cycle_view=True,
)
ProjectMember.objects.create(
project=project,
Expand Down
9 changes: 5 additions & 4 deletions apps/api/plane/tests/contract/app/test_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,10 @@ def test_magic_sign_in_with_next_path(self, mock_magic_link, django_client, api_
user_data = json.loads(ri.get("magic_user@plane.so"))
token = user_data["token"]

# Use Django client to test the redirect flow without following redirects
# Use Django client to test the redirect flow without following redirects.
# next_path must start with "/" per validate_next_path (otherwise it's discarded).
url = reverse("magic-sign-in")
next_path = "workspaces"
next_path = "/workspaces"
response = django_client.post(
url,
{"email": "user@plane.so", "code": token, "next_path": next_path},
Expand All @@ -315,8 +316,8 @@ def test_magic_sign_in_with_next_path(self, mock_magic_link, django_client, api_
assert response.status_code == 302
assert "error_code" not in response.url

# Check that the redirect URL contains the next_path
assert next_path in response.url
# Check that the redirect URL contains the next_path (URL-encoded, leading slash → %2F)
assert "workspaces" in response.url

# The user should now be authenticated
assert "_auth_user_id" in django_client.session
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def test_copy_s3_objects_of_description_and_assets(
mock_sync.return_value = {
"description": "test description",
"description_binary": base64.b64encode(b"test binary").decode(),
"description_json": {"type": "doc", "content": []},
}

# Call the actual function (not .delay())
Expand Down
32 changes: 20 additions & 12 deletions apps/api/plane/tests/unit/utils/test_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,20 @@ def test_contains_url_edge_cases(self):
assert contains_url("www.") is False # Incomplete www - needs at least one char after dot

def test_contains_url_length_limit_under_1000(self):
"""Test contains_url with input under 1000 characters containing URLs"""
# Create a string under 1000 characters with a URL
text_with_url = "a" * 970 + " https://example.com" # 970 + 1 + 19 = 990 chars
"""Test contains_url with input under 1000 characters containing URLs.

Note: contains_url also truncates each line to 500 chars (ReDoS protection),
so URLs must fall within the first 500 chars of their line.
"""
# Single line under 500 chars with URL at the end
text_with_url = "a" * 470 + " https://example.com" # 490 chars total
assert len(text_with_url) < 1000
assert contains_url(text_with_url) is True

# Test with exactly 1000 characters
text_exact_1000 = "a" * 981 + "https://example.com" # 981 + 19 = 1000 chars
assert len(text_exact_1000) == 1000
assert contains_url(text_exact_1000) is True
# Multi-line input under 1000 chars total; URL on its own short line
text_multiline = "a" * 480 + "\nhttps://example.com\n" + "b" * 480
assert len(text_multiline) < 1000
assert contains_url(text_multiline) is True

def test_contains_url_length_limit_over_1000(self):
"""Test contains_url with input over 1000 characters returns False"""
Expand All @@ -91,14 +95,17 @@ def test_contains_url_length_limit_over_1000(self):
assert contains_url(long_text_with_url) is False

def test_contains_url_length_limit_exactly_1000(self):
"""Test contains_url with input exactly 1000 characters"""
"""Test contains_url with input exactly 1000 characters.

URLs must fall within the first 500 chars of their line (ReDoS protection).
"""
# Test with exactly 1000 characters without URL
text_no_url = "a" * 1000
assert len(text_no_url) == 1000
assert contains_url(text_no_url) is False

# Test with exactly 1000 characters with URL at the end
text_with_url = "a" * 981 + "https://example.com" # 981 + 19 = 1000 chars
# Multi-line totalling exactly 1000 chars; URL on a short line
text_with_url = "a" * 480 + "\nhttps://example.com\n" + "b" * 499 # 480+1+19+1+499 = 1000
assert len(text_with_url) == 1000
assert contains_url(text_with_url) is True

Expand All @@ -121,8 +128,9 @@ def test_contains_url_total_length_vs_line_length(self):
over_limit_text = "a" * 1001 # No URL, but over total limit
assert contains_url(over_limit_text) is False

# Test that under total limit, line processing works normally
under_limit_with_url = "a" * 900 + "https://example.com" # 919 chars total
# Test that under total limit, line processing works normally.
# URL must be within first 500 chars of its line (ReDoS protection).
under_limit_with_url = "a" * 400 + "https://example.com" # 419 chars total, fits in 500
assert len(under_limit_with_url) < 1000
assert contains_url(under_limit_with_url) is True

Expand Down
21 changes: 8 additions & 13 deletions apps/api/plane/utils/path_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,15 @@ def _contains_suspicious_patterns(path: str) -> bool:

def get_allowed_hosts() -> list[str]:
"""Get the allowed hosts from the settings."""
base_origin = settings.WEB_URL or settings.APP_BASE_URL

allowed_hosts = []
if base_origin:
host = urlparse(base_origin).netloc
allowed_hosts.append(host)
if settings.ADMIN_BASE_URL:
# Get only the host
host = urlparse(settings.ADMIN_BASE_URL).netloc
allowed_hosts.append(host)
if settings.SPACE_BASE_URL:
# Get only the host
host = urlparse(settings.SPACE_BASE_URL).netloc
allowed_hosts.append(host)
# Include every configured base URL; WEB_URL and APP_BASE_URL may differ
# (e.g. WEB_URL points at the API host, APP_BASE_URL at the web app), and
# both need to be allowed for redirects to either origin to pass safety checks.
for setting in (settings.WEB_URL, settings.APP_BASE_URL, settings.ADMIN_BASE_URL, settings.SPACE_BASE_URL):
if setting:
host = urlparse(setting).netloc
if host and host not in allowed_hosts:
allowed_hosts.append(host)
return allowed_hosts


Expand Down
82 changes: 82 additions & 0 deletions apps/api/tests/RUNNING_TESTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# Running the API Test Suite

This guide covers running the Django/pytest suite for `apps/api` inside Docker via `docker-compose-test.yml` at the repo root. The compose file boots an isolated stack — Postgres, Valkey (Redis), RabbitMQ, MinIO — with tmpfs-backed data dirs, so every run begins from a clean slate and a single teardown command removes everything.

For background on the test layout, markers, and fixtures, see [`TESTING_GUIDE.md`](./TESTING_GUIDE.md) and [`README.md`](./README.md).

## Prerequisites

- Docker and Docker Compose v2 (`docker compose ...`)
- Env files generated via the setup script:

```bash
./setup.sh
```

This copies `apps/api/.env.example` → `apps/api/.env` (along with the other app env files). The compose file reads `apps/api/.env`, so this step must run **before** the first `docker compose` invocation.

## Running the suite

All commands are run from the repo root.

### Full suite

```bash
docker compose -f docker-compose-test.yml up \
--build \
--abort-on-container-exit \
--exit-code-from api-tests
```

- `--build` rebuilds the `api-tests` image when `Dockerfile.dev` or `requirements/*.txt` change.
- `--abort-on-container-exit` stops the dependency services as soon as `api-tests` exits.
- `--exit-code-from api-tests` propagates pytest's exit code so this works in CI.

### Filtered runs

Use `docker compose run` to override the default `pytest` command. Anything you pass after the service name is forwarded to pytest.

```bash
# Only unit tests (marker defined in pytest.ini)
docker compose -f docker-compose-test.yml run --rm --build api-tests pytest -m unit

# A single directory, filtered by name
docker compose -f docker-compose-test.yml run --rm api-tests \
pytest plane/tests/unit -k "test_workspace"

# Single file with verbose output
docker compose -f docker-compose-test.yml run --rm api-tests \
pytest plane/tests/unit/models/test_workspace.py -vv
```

The available markers (`unit`, `contract`, `smoke`, `slow`) are declared in `apps/api/pytest.ini`.

### Teardown

```bash
docker compose -f docker-compose-test.yml down -v
```

`-v` removes the ephemeral volumes and the `test_env` network. Because the data directories are tmpfs, no host state survives a teardown — every run starts clean. Run this between unrelated test sessions to free Docker resources.

## How it works

| Service | Image | Purpose |
| ------------ | ------------------------------------ | --------------------------------------------- |
| `test-db` | `postgres:15.7-alpine` | Application database |
| `test-redis` | `valkey/valkey:7.2.11-alpine` | Cache / Celery broker |
| `test-mq` | `rabbitmq:3.13.6-management-alpine` | Task queue |
| `test-minio` | `minio/minio` | S3-compatible object storage |
| `api-tests` | built from `apps/api/Dockerfile.dev` | Installs `requirements/test.txt`, runs pytest |

All four dependencies expose health checks; `api-tests` waits for `service_healthy` on each via `depends_on`, so pytest only starts once the stack is ready.

Test-time env overrides live in the compose file itself (`POSTGRES_HOST=test-db`, `REDIS_URL=redis://test-redis:6379/`, `AWS_S3_ENDPOINT_URL=http://test-minio:9000`, `DJANGO_SETTINGS_MODULE=plane.settings.test`). Everything else is inherited from `apps/api/.env`.

## Troubleshooting

- **`./apps/api/.env: no such file or directory`** — run `./setup.sh` from the repo root.
- **Port already in use** — none of the test services publish host ports; if you see this it's coming from a different compose stack. Stop the local stack (`docker compose -f docker-compose-local.yml down`).
- **Stale image after dependency changes** — rebuild explicitly: `docker compose -f docker-compose-test.yml build --no-cache api-tests`.
- **MinIO bucket missing** — the `test-minio` entrypoint creates the bucket named by `AWS_S3_BUCKET_NAME` (default `uploads`). Change the value in `apps/api/.env` and re-run.
- **Database state leaking between runs** — confirm you ran `down -v` (not just `down`). The tmpfs mounts are torn down with the container, but the network and any externally created volumes need `-v` to clear.
Loading
Loading