Audit logs endpoint - #72
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: lokalise/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/_data/api_items.yml`:
- Around line 14-18: Rename the navigation property from sub_path to sub_paths
in the Audit logs entry so the existing schema recognizes “List audit logs” as a
nested navigation item.
In `@docs/additional_info/changelog.md`:
- Around line 7-12: Update the API v1 cursor pagination entry in the changelog
to explicitly document that Audit Logs pagination reads has_more and next_cursor
from the response body, placing this clarification near the existing cursor
pagination bullet.
In `@lib/elixir_lokalise_api/config.ex`:
- Around line 54-58: Update the configuration module’s documentation text near
the base_url_api_v1 and base_url_oauth2 examples so it explicitly describes API
v1 alongside API v2 and OAuth2. Keep the existing configuration examples and
environment-variable guidance unchanged.
🪄 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: Repository: lokalise/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7095f1a4-6e0e-4695-8646-f0e2745eb3a3
⛔ Files ignored due to path filters (1)
mix.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
config/dev.exsdocs/_data/api_items.ymldocs/additional_info/changelog.mddocs/additional_info/customization.mddocs/api/audit-logs.mddocs/api/getting-started.mdlib/elixir_lokalise_api/collections/v1/audit_logs.exlib/elixir_lokalise_api/config.exlib/elixir_lokalise_api/cursor_pagination.exlib/elixir_lokalise_api/dynamic_resource.exlib/elixir_lokalise_api/endpoints/v1/audit_logs.exlib/elixir_lokalise_api/models/v1/audit_log.exlib/elixir_lokalise_api/processor.exlib/elixir_lokalise_api/request.exlib/elixir_lokalise_api/url_generator.exmix.exstest/elixir_lokalise_api/cursor_pagination_test.exstest/elixir_lokalise_api/endpoints/v1/audit_logs_test.exstest/elixir_lokalise_api/processor_test.exstest/elixir_lokalise_api/request_test.exs
| - title: Audit logs | ||
| path: audit-logs | ||
| sub_path: | ||
| - title: List audit logs | ||
| path: list-audit-logs |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the navigation schema’s sub_paths key.
sub_path does not match the key used by every other section, so “List audit logs” will not be included as a nested navigation item.
Proposed fix
- sub_path:
+ sub_paths:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - title: Audit logs | |
| path: audit-logs | |
| sub_path: | |
| - title: List audit logs | |
| path: list-audit-logs | |
| - title: Audit logs | |
| path: audit-logs | |
| sub_paths: | |
| - title: List audit logs | |
| path: list-audit-logs |
🤖 Prompt for 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.
In `@docs/_data/api_items.yml` around lines 14 - 18, Rename the navigation
property from sub_path to sub_paths in the Audit logs entry so the existing
schema recognizes “List audit logs” as a nested navigation item.
| * Added support for Lokalise API v1 endpoints | ||
| * Added the Audit Logs endpoint through `ElixirLokaliseApi.V1.AuditLogs` | ||
| * Added a separate `:base_url_api_v1` configuration option for API v1 requests | ||
| * Added cursor pagination support for API v1 collections | ||
| * Added `ElixirLokaliseApi.CursorPagination` helpers for checking and retrieving the next cursor | ||
| * Updated URL generation to prevent duplicate slashes when joining base URLs and endpoint paths |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document that API v1 cursor fields come from the response body.
The entry names cursor pagination but omits the required body-level semantics for has_more and next_cursor; add this explicitly near the cursor-pagination bullet. As per path instructions, “API v1 cursor pagination semantics for Audit Logs: has_more/next_cursor come from the response body.”
🤖 Prompt for 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.
In `@docs/additional_info/changelog.md` around lines 7 - 12, Update the API v1
cursor pagination entry in the changelog to explicitly document that Audit Logs
pagination reads has_more and next_cursor from the response body, placing this
clarification near the existing cursor pagination bullet.
Source: Path instructions
| config :elixir_lokalise_api, base_url_api_v1: "YOUR_API_BASE_URL_v1" | ||
| config :elixir_lokalise_api, base_url_oauth2: "YOUR_OAUTH2_BASE_URL" | ||
|
|
||
| You can also use `{:system, "ENV_VAR_NAME"}` to set it via environment variables. | ||
| """ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include API v1 in the base URL documentation.
Line [52] still describes only APIv2/OAuth2 even though Line [54] adds base_url_api_v1. Update the wording so the public documentation accurately covers all supported base URLs.
🤖 Prompt for 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.
In `@lib/elixir_lokalise_api/config.ex` around lines 54 - 58, Update the
configuration module’s documentation text near the base_url_api_v1 and
base_url_oauth2 examples so it explicitly describes API v1 alongside API v2 and
OAuth2. Keep the existing configuration examples and environment-variable
guidance unchanged.
1 participant
No description provided.