Fix some aspects of endpoint

[whisper67265]

Close #99, close #103, close #102.

Summary by CodeRabbit

• New Features

  • Structured error reporting with stable machine-readable codes and metadata; validation now returns structured per-field error payloads in HTTP 400 responses.

• Bug Fixes

  • Improved error handling and recovery for git operations and component processing to avoid data loss on failures.
  • Background task errors now carry typed codes for clearer monitoring and predictable failure semantics.

• Tests

  • Expanded coverage for structured errors, serializer behavior, services, tasks, and error-wrapping.

• Documentation

  • API docs updated to describe the unified structured error format and error-code table.

• Chores

  • Example configuration default changed to open registrations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6cf96a6f-0cda-4508-a12e-1e781ccf3598

📥 Commits

Reviewing files that changed from the base of the PR and between 843f6ae and eb011d2.

📒 Files selected for processing (1)

• docs/boost-endpoint-api.md

---

📝 Walkthrough

Walkthrough

Adds a typed error taxonomy and helpers, integrates structured error payloads into serializer validation, views, services (git/file removal and process_submodule/process_all), and Celery tasks, and updates tests and an example env var to assert and consume these typed errors.

Changes

Structured Error Handling Integration

Layer / File(s)	Summary
Error infrastructure: codes, exceptions, and helpers src/boost_weblate/endpoint/errors.py, tests/endpoint/test_errors.py	Defines BoostEndpointErrorCode enum, BoostEndpointError subclass, and helpers to_error_dict, append_error, boost_validation_errors, wrap_task_error; tests validate construction, serialization, wrapping, and stable codes.
Serializer validation integration with structured errors src/boost_weblate/endpoint/serializers.py, src/boost_weblate/endpoint/views.py, tests/endpoint/test_serializers.py, tests/endpoint/test_views.py	AddOrUpdateRequestSerializer stores _custom_validation_errors, exposes structured_errors, overrides is_valid() to build typed per-error payloads, and validate_add_or_update() yields typed entries; AddOrUpdateView.post returns structured_errors on 400. Tests assert codes and metadata.
Celery task error reporting src/boost_weblate/endpoint/tasks.py, tests/endpoint/test_views.py	boost_add_or_update_task raises BoostEndpointError(TASK_USER_NOT_FOUND) when user lookup fails, re-raises BoostEndpointError, and wraps unexpected exceptions as TASK_INTERNAL_ERROR after reporting. Tests adjusted to expect typed exceptions.
Git operation helpers with error reporting src/boost_weblate/endpoint/services.py (helpers), tests/endpoint/test_services.py	Adds _git_commit_and_push_removals and _git_restore_removed_files to stage/commit/push removals and restore on failure; returns typed git errors (GIT_PUSH_FAILED, GIT_PUSH_TIMEOUT) with metadata. Tests mock subprocesses and assert codes/restore behavior.
Component deletion with git coordination src/boost_weblate/endpoint/services.py (delete flow), tests/endpoint/test_services.py	Defers DB deletion until git commit/push succeed, collects FILE_REMOVE_FAILED and git errors as typed payloads, restores files on failure, and deletes inside a transaction on success. Tests assert transactional deletion and error codes.
Temp directory parameter refactor src/boost_weblate/endpoint/services.py, tests/endpoint/test_services.py	Removes self.temp_dir; process_submodule requires temp_dir, validates per-submodule path stays inside temp root; process_all creates and cleans the local temp directory and passes it into processing calls. Tests updated to pass explicit temp_dir.
Submodule processing with structured errors src/boost_weblate/endpoint/services.py, tests/endpoint/test_services.py	process_submodule converts invalid submodule names, clone failures, missing docs, permission denials, project-creation failures, and all-components-failed cases into typed append_error entries with metadata. Tests assert codes and metadata.
Tests and helpers updated tests/endpoint/*	Adds _error_codes() and _has_error_code() helpers; updates many tests to assert structured error code, message, and metadata instead of raw strings; converts mixed-success failure payloads to structured dicts.
Environment configuration update .env.example	Updates WEBLATE_REGISTRATION_OPEN from 0 to 1.

Sequence Diagram(s)

sequenceDiagram
  participant View as AddOrUpdateView.post
  participant Serializer as AddOrUpdateRequestSerializer
  participant Task as boost_add_or_update_task
  participant Service as BoostComponentService
  participant Errors as boost_weblate.endpoint.errors
  View->>Serializer: is_valid(raise_exception=True)
  alt Invalid request
    Serializer->>Errors: map DRF ErrorDetail -> BoostEndpointErrorCode
    Errors-->>Serializer: structured error dicts
    Serializer-->>View: raises ValidationError with structured_errors
    View-->>Client: HTTP 400 { "errors": [...] }
  else Valid request
    View->>Task: apply_async() / start
    Task->>Service: process_all(temp_dir)
    Service->>Errors: append_error() for per-submodule failures
    Service-->>Task: result with structured errors
    Task->>Errors: wrap_task_error() on unexpected exception
    Errors-->>Task: BoostEndpointError
    Task-->>Broker: raise BoostEndpointError (typed)
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• cppalliance/cppa-weblate-plugin#50: Prior changes to Boost endpoint logic (process_submodule/process_all/task) that this PR extends with structured error handling.
• cppalliance/cppa-weblate-plugin#48: Prior work on AddOrUpdateRequestSerializer/AddOrUpdateView validation flow that this change evolves to emit structured_errors.

Suggested reviewers

• henry0816191
• wpak-ai

Poem

🐰 I nibbled bugs and tidied codes,

I stitched my hops with careful modes,
Errors now wear labels bright,
Metadata tucked in tight,
Hooray — the logs now sleep at night!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title 'Fix some aspects of endpoint' is vague and generic, failing to convey the primary change (component deletion atomicity and structured error handling).	Use a descriptive title like 'Fix component deletion atomicity and add structured error handling to endpoint' to clearly summarize the main changes.
Out of Scope Changes check	❓ Inconclusive	While most changes align with issue #99, the PR introduces a comprehensive error-handling framework (errors.py, error codes, structured error payloads across views/tasks/services) that extends beyond the stated atomic deletion objective.	Clarify whether the structured error taxonomy and endpoint-wide error refactoring are part of the intended scope or should be separated into a follow-up PR.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	All code changes comprehensively address issue #99 requirements: deferred component deletion post-git-success, structured error recording, git rollback helpers, and atomic transaction wrapping.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.env.example:
- Line 50: There is a duplicate environment variable definition for
WEBLATE_REGISTRATION_OPEN; remove the redundant entry so the variable is defined
only once (keep the intended value and delete the other line), ensuring the
single remaining WEBLATE_REGISTRATION_OPEN entry reflects the correct
boolean/value used by the application.

In `@src/boost_weblate/endpoint/serializers.py`:
- Around line 87-115: The _flatten_field_errors function in
AddOrUpdateRequestSerializer currently stringifies DRF ErrorDetail objects which
breaks stable error-code mapping; modify _flatten_field_errors to propagate each
error's drf_code (use getattr(err, "code", None)) and return tuples like
(subfield, message, drf_code) instead of just (subfield, message), update any
call sites (including _code_for_drf_error) to accept the new triplet shape, and
change _code_for_drf_error to map DRF codes directly (e.g., ListField codes
"not_a_list" and "empty" → INVALID_SUBMODULE_LIST, missing field "required" →
REQUIRED_FIELD) rather than doing substring matching on message text. Ensure
nested branches (lists, dicts, ErrorDetail instances) preserve and propagate the
drf_code and adjust tests/consumers to handle the new tuple form.

In `@src/boost_weblate/endpoint/services.py`:
- Around line 131-138: The code currently runs subprocess.run(...) and uses
git_status.stdout without checking git_status.returncode, so add an explicit
check after the subprocess call (inspect git_status.returncode) and treat any
non-zero return code as a failure: log the error including git_status.stderr and
git_status.returncode (using the existing logger), and abort/raise (do not
proceed with DB deletion) when returncode != 0; update the block around the
subprocess.run invocation and the git_status variable usage to enforce this
explicit failure path (reference: git_status, subprocess.run call that uses
["git", "-C", base_path, "status", "--porcelain"]).
- Around line 146-156: The git commit call in subprocess.run (the one building
args with "git", "-C", base_path, "commit", "-m", f"Remove translation files for
deleted component: {name}", "--author", author) is not path-scoped and may
include unrelated staged changes; change the command to explicitly limit the
commit to the removed translation file paths by appending "--" followed by the
list of removed file paths (e.g., a variable like removed_paths or
removed_files) to the args passed to subprocess.run so only those files are
committed (ensure you pass the exact relative paths as arguments after the
"--").

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 29945e07-71dd-402e-89f4-18b3aa1b5a2c

📥 Commits

Reviewing files that changed from the base of the PR and between b85bc5f and a825283.

📒 Files selected for processing (10)

• .env.example
• src/boost_weblate/endpoint/errors.py
• src/boost_weblate/endpoint/serializers.py
• src/boost_weblate/endpoint/services.py
• src/boost_weblate/endpoint/tasks.py
• src/boost_weblate/endpoint/views.py
• tests/endpoint/test_errors.py
• tests/endpoint/test_serializers.py
• tests/endpoint/test_services.py
• tests/endpoint/test_views.py

[henry0816191]

docs/boost-endpoint-api.md still documents the old error shapes

[whisper67265]

@coderabbitai, resume

[coderabbitai]

@whisper67265: Resuming reviews for this PR.

✅ Action performed

Reviews resumed.