Skip to content
Open
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
61 changes: 61 additions & 0 deletions labellerr/core/projects/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -643,3 +643,64 @@ def import_users(self, from_project: "LabellerrProject"):
"POST", url, extra_headers={"Content-Type": "application/json"}
)
return response.get("response")

def archive(self, unarchive=False):
"""
Archives or unarchives the project.

:param unarchive: If True, unarchives the project. Default is False (archive).
:return: Response from the server.
:raises LabellerrError: If the operation fails.
"""
unique_id = str(uuid.uuid4())
action = "unarchive" if unarchive else "archive"
url = f"{constants.BASE_URL}/projects/{action}?client_id={self.client.client_id}&uuid={unique_id}"

payload = json.dumps({"project_id": self.project_id})

headers = {"content-type": "application/json"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inconsistent header casing: The codebase predominantly uses lowercase content-type (see lines 42, 128, 211, 263, 576, 613). Recommend changing to lowercase for consistency:

Suggested change
headers = {"content-type": "application/json"}
headers = {"content-type": "application/json"}

if self.client.api_key:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no need for this check, it will always be there

headers["email_id"] = self.client.api_key

return self.client.make_request(
"POST",
url,
extra_headers=headers,
request_id=unique_id,
data=payload,
)
Comment on lines +665 to +671

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inconsistent response handling: This method returns the raw response, but other similar methods like import_users (line 645) return response.get(\"response\"). Should clarify and document the expected return format. Consider either:

  1. Extracting nested response for consistency, or
  2. Documenting why this returns full response object



Comment on lines +672 to +673

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code style: Extra blank line here is inconsistent with other methods in this file. Should be removed for consistency.

def unarchive(self):
"""
Unarchives the project.
Alias for archive(unarchive=True).

:return: Response from the server.
:raises LabellerrError: If the operation fails.
"""
return self.archive(unarchive=True)


def delete(self):
Comment on lines +684 to +685

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code style: Extra blank line should be removed for consistency with other methods.

"""
Deletes the project.

:return: Response from the server.
:raises LabellerrError: If the operation fails.
"""
unique_id = str(uuid.uuid4())

url = f"{constants.BASE_URL}/projects/project/{self.project_id}?client_id={self.client.client_id}&uuid={unique_id}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think the url is incorrect


Comment on lines +694 to +695

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code style: Extra blank line should be removed for consistency.


headers = {}
if self.client.api_key:
headers["email_id"] = self.client.api_key

return self.client.make_request(
"DELETE",
url,
extra_headers=headers,
request_id=unique_id,
)
Comment on lines +701 to +706

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing validation: Consider validating:

  1. Project exists before attempting deletion
  2. User has permission to delete
  3. Project isn't already deleted

Also, response handling: Same issue as archive() - should document if full response or nested data should be returned.

Comment on lines +700 to +706

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

since we don't allow delete http method, lets use POST

4 changes: 2 additions & 2 deletions labellerr/core/schemas/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
"""

import os
from enum import StrEnum
from enum import Enum
from typing import List, Literal
from uuid import UUID

from .base import DatasetDataType
from pydantic import BaseModel, Field, field_validator


class DataSetScope(StrEnum):
class DataSetScope(str, Enum):
Comment on lines +6 to +14

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good fix: Changing from StrEnum (Python 3.11+) to str, Enum is correct for maintaining Python 3.7+ compatibility as specified in pyproject.toml. The behavior is equivalent and this ensures backward compatibility. ✅

project = "project"
client = "client"
public = "public"
Expand Down
28 changes: 28 additions & 0 deletions tests/integration/test_create_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,31 @@ def test_create_project(create_project_fixture):

assert project.project_id is not None
assert isinstance(project.project_id, str)


def test_archive_and_delete_project(create_project_fixture):

project = create_project_fixture
assert project.project_id is not None

# 1. Archive
print(f"Archiving project {project.project_id}...")
try:
project.archive()
except Exception as e:
pytest.fail(f"Failed to archive project: {e}")
Comment on lines +58 to +63

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Weak assertions: The test should verify:

  1. Response status/success indicator
  2. Response structure matches expected format
  3. Consider checking project state via a GET request to confirm it's actually archived

Current implementation only checks that no exception was raised, which is insufficient for integration testing.


# 2. Unarchive (test the alias)
print(f"Unarchiving project {project.project_id}...")
try:
project.unarchive()
except Exception as e:
pytest.fail(f"Failed to unarchive project: {e}")

# 3. Archive again (to test delete on archived project if valid, or just normal delete)
# Let's delete it while active
print(f"Deleting project {project.project_id}...")
try:
project.delete()
except Exception as e:
pytest.fail(f"Failed to delete project: {e}")
Comment on lines +74 to +78

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test completeness: After deleting, the test should:

  1. Verify the delete response indicates success
  2. Optionally attempt to fetch the project and verify it returns a 404 or "not found" error
  3. Check that subsequent operations on the deleted project fail appropriately

This ensures the delete actually worked and wasn't just a no-op.

86 changes: 86 additions & 0 deletions tests/unit/test_projects.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import pytest
from unittest.mock import patch, Mock
import json
from labellerr.core.projects.image_project import ImageProject
from labellerr.core.users.base import LabellerrUsers

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unused import: LabellerrUsers is imported but never used in this file. Should be removed.


@pytest.fixture
def client():
"""Create a mock client"""
from labellerr.client import LabellerrClient
client = Mock(spec=LabellerrClient)
client.client_id = "test-client-id"
client.api_key = "test-api-key"
client.api_secret = "test-api-secret"
return client

@pytest.fixture
def project(client):
"""Create a test project instance"""
project_data = {
"project_id": "test_project_id",
"data_type": "image",
"attached_datasets": [],
}
# Use __new__ to avoid initialization logic if needed, or just mock it
proj = ImageProject.__new__(ImageProject)
proj.client = client
proj._LabellerrProject__project_id_input = "test_project_id"
proj._LabellerrProject__project_data = project_data
return proj

@pytest.mark.unit
class TestProjectLifecycle:
"""Tests for project lifecycle methods: archive, unarchive, delete"""

def test_archive_project(self, project, client):
"""Test archiving a project"""
mock_response = {"status": "success", "msg": "Project archived"}

with patch.object(client, "make_request", return_value=mock_response) as mock_req:
response = project.archive()

assert response == mock_response

# Verify the request
mock_req.assert_called_once()
args, kwargs = mock_req.call_args
assert args[0] == "POST"
assert "/projects/archive" in args[1]
assert kwargs["data"] is not None
assert '"project_id": "test_project_id"' in kwargs["data"]
Comment on lines +36 to +51

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test coverage gap: The test only verifies the request was made, but should also verify:

  1. The request includes the correct uuid in the URL
  2. The request_id parameter matches the uuid
  3. The payload is properly formatted JSON

Consider adding these assertions:

assert kwargs.get(\"request_id\") is not None
assert f\"uuid={kwargs['request_id']}\" in args[1]


def test_unarchive_project(self, project, client):
"""Test unarchiving a project"""
mock_response = {"status": "success", "msg": "Project unarchived"}

with patch.object(client, "make_request", return_value=mock_response) as mock_req:
# Test direct call to archive(unarchive=True)
response = project.archive(unarchive=True)
assert response == mock_response

args, kwargs = mock_req.call_args
assert "/projects/unarchive" in args[1]

# Test alias unarchive()
mock_req.reset_mock()
response = project.unarchive()
assert response == mock_response

args, kwargs = mock_req.call_args
assert "/projects/unarchive" in args[1]

def test_delete_project(self, project, client):
"""Test deleting a project"""
mock_response = {"status": "success", "msg": "Project deleted"}

with patch.object(client, "make_request", return_value=mock_response) as mock_req:
response = project.delete()

assert response == mock_response

# Verify the request
mock_req.assert_called_once()
args, kwargs = mock_req.call_args
assert args[0] == "DELETE"
assert "/projects/project/test_project_id" in args[1]
Loading