Poeditor plugin implementation#223
Conversation
Implements a comprehensive POEditor plugin following the 5-layer architecture pattern and POEditor API v2 specification. Features: - Project management (list, get project details) - Term operations (add, update terms following API spec) - Translation file upload - Multi-language support - Full error handling with ClientResult pattern - Textual TUI integration with workflow steps - Improved error messages for plugin configuration Plugin Architecture: - Models: Network (REST API), View (UI), Mappers - Clients: Network layer, Services, Public facade - Operations: Pure business logic - Steps: UI orchestration for workflows - Workflows: list-projects, import-translations API Compliance: - terms/add: Strict validation, supports all optional fields - terms/update: Full spec support including fuzzy_trigger - projects/upload: File upload with multipart/form-data - Proper statistics parsing (parsed/added/updated) Configuration: - API token via secrets (POEDITOR_API_TOKEN) - Project-level plugin enablement - Enhanced error messages guide users to configure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
83284f9 to
fe4a323
Compare
| try: | ||
| # First, add the terms to the project | ||
| terms_payload = [ | ||
| {"term": term_key} |
There was a problem hiding this comment.
This method advertises terms_map as term_key -> default language value, but the payload only sends {"term": term_key}. That means values like "Home" are never written to POEditor at all. As written, create_terms_with_translations({"home_title": "Home"}, {"es": ...}) creates the term key but drops the source/default translation. Please persist the default-language value as well, or change the API contract so callers do not believe those values are stored.
| ) | ||
|
|
||
| # PoEditor API response format: {"response": {...}, "result": {...}} | ||
| json_response = response.json() if response.content else {} |
There was a problem hiding this comment.
This method currently lets response.json() raise if the server returns a non-JSON body. That means callers can get a raw JSONDecodeError instead of the advertised PoEditorAPIError, and in the HTTPError branch the JSON parse can even hide the original HTTP exception. Please wrap both JSON parsing sites and convert parse failures into PoEditorAPIError (including the raw body/status for debugging).
| project_id, file_path, language_code, updating | ||
| ) | ||
|
|
||
| def get_project_languages(self, project_id: str) -> ClientResult[list[str]]: |
There was a problem hiding this comment.
These methods are being added to the client facade’s public API, but clients/protocols.py still only declares list_projects, get_project, and upload_file. Since the plugin exports PoEditorClientProtocol for DI/testing, the declared interface is now incomplete. Please add get_project_languages, add_terms, and create_terms_with_translations to PoEditorClientProtocol so the public contract matches the implementation.
| data = self.network.make_request("projects/list") | ||
|
|
||
| # Handle response | ||
| projects_data = data.get("projects", []) |
There was a problem hiding this comment.
If projects/list comes back without a projects field, we currently convert that malformed payload into a successful empty list. That loses failure signaling and can show users 0 projects when the server actually returned an invalid response. Please validate that projects is present and of the expected type, and return a ClientError when the payload shape is wrong.
| ) | ||
|
|
||
| # Parse project | ||
| network_project = self._parse_project(project_data.get("project", {})) |
There was a problem hiding this comment.
This fallback turns a missing project object into a blank UIPoEditorProject and still returns success. That makes malformed API responses look like valid data. Please validate that project_data["project"] exists before parsing, and return a ClientError if the payload is missing or invalid.
| try: | ||
| # Validate file exists | ||
| file_path_obj = Path(file_path) | ||
| if not file_path_obj.exists(): |
There was a problem hiding this comment.
This validates only existence, not that the path is an actual file. If a directory is passed here, open() raises and the caller gets INTERNAL_ERROR, which is misleading. Please check is_file() (and ideally return a dedicated validation error) before trying to open the path.
| UIPoEditorProject ready for rendering | ||
| """ | ||
| # Calculate overall progress based on languages | ||
| progress_percentage = 0.0 |
There was a problem hiding this comment.
This mapper treats missing languages as 0% completion. In the current list-projects path, projects are mapped without languages, so every listed project will render with the red/incomplete icon even when the project is actually complete. Please avoid turning “unknown progress” into 0% here, or source the icon from data that is actually present on the list response.
|
|
||
| # Validate file exists | ||
| file_path_obj = Path(file_path) | ||
| if not file_path_obj.exists(): |
There was a problem hiding this comment.
This validation should also require file_path_obj.is_file(). Right now any existing directory passes, and the workflow only fails later when the upload service calls open(...), which surfaces as an internal/upload error instead of a clear file_path validation error.
| """Get project by ID.""" | ||
| ... | ||
|
|
||
| def upload_file( |
There was a problem hiding this comment.
The protocol is now behind the actual client contract: import_translations_step passes updating=..., and the README documents that parameter too. Please add updating: str = "terms_translations" here so typed callers, mocks, and alternate implementations stay compatible with the workflow-facing API.
| ) | ||
|
|
||
| # Invert the map to key -> value | ||
| result = {key: value for value, key in keys_map.items()} |
There was a problem hiding this comment.
Before inverting this mapping, validate that all generated keys are unique. As written, two source strings that get the same AI-generated key will collapse to one entry here, and the caller gets a successful result with silent data loss.
| ) | ||
|
|
||
| @log_client_operation() | ||
| def add_terms(self, project_id: str, terms: list[dict]) -> ClientResult[dict]: |
There was a problem hiding this comment.
Instead of returning dict you could return something like this:
@dataclass class TermsAddResult: parsed: int added: int
| project_id: str, | ||
| terms_map: dict[str, str], | ||
| translations_by_language: dict[str, dict[str, str]] | ||
| ) -> ClientResult[dict]: |
There was a problem hiding this comment.
You could return something like:
@dataclass class TermsWithTranslationsResult: terms_added: int languages_updated: int
| error_code="INTERNAL_ERROR", | ||
| ) | ||
|
|
||
| def _add_terms(self, project_id: str, terms: list[dict]) -> ClientResult[dict]: |
There was a problem hiding this comment.
If this is a new plugin does not make sense to make something deprecated, just implemented it right at first
| file_path: str, | ||
| language_code: str, | ||
| updating: str = "terms_translations", | ||
| ) -> ClientResult[dict]: |
There was a problem hiding this comment.
You could return something like:
@dataclass class UploadStatsResult: added: int updated: int deleted: int
| updating: "terms_translations" | ||
|
|
||
| steps: | ||
| - id: list_projects |
There was a problem hiding this comment.
You could reference a workflow within a workflow. You already have a workflow that do this, so maybe or call this workflow from here, or simply remove the other workflow if you don't use it outside this, nor you don't want to give the option of running the other workflow ouside this one
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
By: finxo
Pull Request
📝 Summary
Adds PoEditor plugin as a core plugin for Titan
🔧 Changes Made
🧪 Testing
poetry run pytest)make test)titan-dev📊 Logs
✅ Checklist
Plugins > Git Plugin,GitHub Plugin,Jira Plugin)