[ADD] balance dashboard to tesote accounts - #13
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a balance history feature for Tesote accounts (model, recording logic, cleanup), UI views and menu, tests and translations, a local API setup script that provisions webhook secret, and a Postman collection with HMAC-SHA256 prerequest signing for API requests. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to c33da72 in 3 minutes and 0 seconds. Click for details.
- Reviewed
1283lines of code in12files - Skipped
0files when reviewing. - Skipped posting
8draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. Tesote_Accounting_API.postman_collection.json:15
- Draft comment:
Replace the secret key placeholder with a secure method (e.g. environment variable) for production. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment is problematic for several reasons: 1) It's making a suggestion about security best practices rather than pointing out a definite bug or issue. 2) The placeholder "YOUR_SECRET_KEY_HERE" is a standard practice in Postman collections - users are expected to replace it with their own values. 3) The suggestion to change it to an empty string doesn't actually solve anything and would make the collection less clear. 4) This is a configuration/documentation file, not production code. 5) The comment is somewhat speculative ("for production") and doesn't point to a clear, actionable code change that must be made. 6) Postman collections commonly use collection variables with placeholder values that users customize. Could this be a legitimate security concern if the file might be committed with actual secrets? Perhaps the comment is trying to prevent accidental secret exposure, which would be a valid concern. While preventing secret exposure is important, the current placeholder "YOUR_SECRET_KEY_HERE" is already a safe placeholder that clearly indicates it needs to be replaced. The file is a Postman collection template, and having a descriptive placeholder is the standard and correct approach. If there were an actual secret hardcoded, that would be different, but this is just a template placeholder. This comment should be deleted. It's commenting on a standard Postman collection placeholder pattern, not a real security issue. The placeholder "YOUR_SECRET_KEY_HERE" is appropriate for a template file, and the suggested change to an empty string would make it less clear. This is not a clear code defect requiring a change.
2. __manifest__.py:118
- Draft comment:
Verify the new view file 'tesote_account_balance_history_views.xml' is correctly referenced and loads without error. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. bin/setup-api-local.py:10
- Draft comment:
This local setup script uses hardcoded credentials; ensure these are only for testing and not deployed in production. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. i18n/es.po:736
- Draft comment:
Consider adding proper accent marks in the Spanish translations (e.g. 'Actualización' instead of 'Actualizacion'). - Reason this comment was not posted:
Marked as duplicate.
5. models/tesote_account.py:592
- Draft comment:
Ensure the floating-point comparison for balance changes (using a threshold of 0.01) is robust and handles rounding issues appropriately. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
6. views/tesote_account_balance_history_views.xml:97
- Draft comment:
Using datetime.timedelta in search view domain expressions may not work as expected in Odoo's safe_eval context; consider verifying or using alternative approaches. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment raises a potential technical issue about usingdatetime.timedeltain Odoo's safe_eval context. However, the comment is speculative - it says "may not work" and asks to "verify" rather than stating definitively that it's broken. This violates the rule about not making speculative comments. The comment also doesn't provide a clear alternative solution - it just says "consider verifying or using alternative approaches" which is vague. If this actually doesn't work, it would be caught when the code is run/tested. The comment is asking the author to verify/confirm rather than stating a definite issue that needs fixing. However, ifdatetime.timedeltagenuinely doesn't work in Odoo's safe_eval context, this could be a real bug that needs fixing. The comment might be highlighting a legitimate technical limitation of Odoo's domain evaluation system that wouldn't necessarily be obvious to all developers. While there might be a real issue here, the comment is phrased speculatively ("may not work") and asks the author to "verify" rather than stating definitively what's wrong. According to the rules, I should not keep speculative comments or comments that ask the author to verify/confirm. If this is a real issue, it would be caught during testing. The comment also doesn't provide a clear, actionable alternative. This comment should be deleted because it's speculative ("may not work"), asks the author to verify rather than stating a definite issue, and doesn't provide a clear actionable alternative. If datetime.timedelta doesn't work in this context, it would be caught during testing.
7. tests/test_balance_history.py:41
- Draft comment:
The balance_history_model fixture uses a Mock; consider adding integration tests with actual record creation to ensure full model behavior. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
8. i18n/es.po:722
- Draft comment:
Typo: 'Registrado En' should likely be 'Registrado en' (the preposition 'en' should be in lower case for consistency in Spanish). - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% Looking at Spanish capitalization rules for titles and labels, prepositions like "en" (meaning "at" or "in") should typically be lowercase in title case, unless they begin the title. The comment appears to be correct from a Spanish grammar perspective. However, I need to check if the existing file has a consistent pattern. Looking at the full file, I see mixed patterns - some entries have prepositions capitalized and some don't. For example, line 680 shows "Sincronización de Saldo" where "de" is lowercase. This suggests the file does follow proper Spanish title case rules where prepositions should be lowercase. The comment appears to be pointing out a legitimate inconsistency. I might be missing context about whether this is a UI label that has specific capitalization requirements in the Odoo framework. The automated tool might not understand that Odoo or the specific module might have its own style guide that differs from standard Spanish grammar rules. Also, this is a very minor stylistic issue that may not warrant a code change. While there might be framework-specific style guides, the comment is about a translation file (.po file) which typically follows the target language's grammar rules. Looking at the file, there is evidence of lowercase prepositions (e.g., "de" in various places), suggesting this is indeed an inconsistency. However, this is an extremely minor issue and the rules state not to make comments that are "obvious or unimportant." A capitalization issue in a translation that doesn't affect functionality is borderline unimportant. This comment points out a minor capitalization inconsistency in Spanish translation. While technically correct according to Spanish grammar rules, it's a very minor stylistic issue. Given the rule to avoid "obvious or unimportant" comments, this falls into the category of unimportant - it doesn't affect functionality and is purely cosmetic.
Workflow ID: wflow_avr68zh5Tb9zkAB8
You can customize 
| #. module: tesote_connector | ||
| #: model:ir.model.fields,selection:tesote_connector.field_tesote_account_balance_history__source | ||
| msgid "Synchronization" | ||
| msgstr "Sincronizacion" |
There was a problem hiding this comment.
Typo: 'Sincronizacion' is missing an accent. It should be 'Sincronización'.
| msgstr "Sincronizacion" | |
| msgstr "Sincronización" |
| #. module: tesote_connector | ||
| #: model:ir.model.fields,selection:tesote_connector.field_tesote_account_balance_history__source | ||
| msgid "Webhook Update" | ||
| msgstr "Actualizacion de Webhook" |
There was a problem hiding this comment.
Typo: 'Actualizacion de Webhook' is missing an accent. Recommend updating to 'Actualización de Webhook'.
| msgstr "Actualizacion de Webhook" | |
| msgstr "Actualización de Webhook" |
| #. module: tesote_connector | ||
| #: model:ir.model.fields,selection:tesote_connector.field_tesote_account_balance_history__source | ||
| msgid "Manual Refresh" | ||
| msgstr "Actualizacion Manual" |
There was a problem hiding this comment.
Typo: 'Actualizacion Manual' is missing an accent. It should be 'Actualización Manual'.
| msgstr "Actualizacion Manual" | |
| msgstr "Actualización Manual" |
| #. module: tesote_connector | ||
| #: model:ir.ui.view,arch_db:tesote_connector.view_tesote_account_balance_history_search | ||
| msgid "Last 90 Days" | ||
| msgstr "Ultimos 90 Dias" |
There was a problem hiding this comment.
Typo: 'Ultimos 90 Dias' is missing accents. It should be 'Últimos 90 Días'.
| msgstr "Ultimos 90 Dias" | |
| msgstr "Últimos 90 Días" |
| #. module: tesote_connector | ||
| #: model:ir.ui.view,arch_db:tesote_connector.view_tesote_account_balance_history_pivot | ||
| msgid "Balance Analysis" | ||
| msgstr "Analisis de Saldo" |
There was a problem hiding this comment.
Typo: 'Analisis de Saldo' is missing an accent. It should be 'Análisis de Saldo'.
| msgstr "Analisis de Saldo" | |
| msgstr "Análisis de Saldo" |
| #. module: tesote_connector | ||
| #: model:ir.ui.view,arch_db:tesote_connector.view_tesote_account_balance_history_form | ||
| msgid "Account Information" | ||
| msgstr "Informacion de Cuenta" |
There was a problem hiding this comment.
Typo: 'Informacion de Cuenta' is missing an accent. It should be 'Información de Cuenta'.
| msgstr "Informacion de Cuenta" | |
| msgstr "Información de Cuenta" |
![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
🧹 Nitpick comments (6)
tests/test_balance_history.py (3)
121-160: Consider testing the actual model's compute method.These tests replicate the computation logic in plain Python rather than invoking
_compute_balance_changeon actual model records. While valid for isolated unit tests, they won't catch regressions if the model's implementation diverges. If you have access to an Odoo test environment (e.g.,TransactionCase), consider testing the computed fields on real records.
194-264: Tests simulate integration logic rather than invoking actual methods.These tests manually replicate the balance change threshold check (
>= 0.01) instead of calling the actualupdate_from_tesoteorcreate_from_tesotemethods. This approach verifies the expected pattern but won't catch regressions if the production code's threshold or logic changes.
270-285: Test doesn't match actual implementation's unlink pattern.The test iterates and calls
record.unlink()individually, but the actualcleanup_old_recordsmethod callsold_records.unlink()on the entire recordset at once. This mismatch means the test isn't verifying the actual code behavior.models/tesote_account.py (2)
591-599: Redundant threshold check; consider removing the duplicate.The
>= 0.01threshold check is duplicated here and insiderecord_balance()(line 185 intesote_account_balance_history.py). Sincerecord_balancealready skips insignificant changes, you could simplify this to just callrecord_balanceunconditionally whennew_balance is not None:Proposed simplification
# Record balance history if balance changed significantly new_balance = vals.get("balance") - if new_balance is not None and abs(new_balance - (self.balance or 0)) >= 0.01: - self.env["tesote.account.balance.history"].record_balance( - account=self, - balance=new_balance, - source="sync", - ) + if new_balance is not None: + self.env["tesote.account.balance.history"].record_balance( + account=self, + balance=new_balance, + source="sync", + )
1-600: File exceeds 500 LOC guideline.This file is approximately 600 lines. Per coding guidelines, consider splitting it into smaller modules. For example, the balance synchronization logic (
sync_balance_to_odoo,_get_odoo_account_balance,_create_balance_adjustment,_get_adjustment_journal) could be extracted to a mixin or separate module.models/tesote_account_balance_history.py (1)
210-225: Movetimedeltaimport to module level.The
from datetime import timedeltainside the method works but is unconventional. Consider moving it to the top of the file with other imports for consistency and slight performance benefit (avoids repeated import lookups).Proposed change
At the top of the file (after line 9):
from datetime import timedeltaThen simplify the method:
@api.model def cleanup_old_records(self, days=365): """ Remove old balance history records beyond retention period. Args: days: Number of days to retain records (default 365) """ - from datetime import timedelta - cutoff_date = fields.Datetime.now() - timedelta(days=days)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
security/ir.model.access.csvis excluded by!**/*.csv
📒 Files selected for processing (11)
Tesote_Accounting_API.postman_collection.json__manifest__.pybin/setup-api-local.pyi18n/es.pomodels/__init__.pymodels/tesote_account.pymodels/tesote_account_balance_history.pytests/test_balance_history.pyviews/tesote_account_balance_history_views.xmlviews/tesote_account_views.xmlviews/tesote_menu.xml
🧰 Additional context used
📓 Path-based instructions (4)
i18n/*.{pot,po}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain translation files:
i18n/tesote_connector.pot(template),i18n/en_US.po(English),i18n/es.po(Spanish)
Files:
i18n/es.po
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use manual capture helperscapture_exception()andcapture_message()for Sentry error tracking
Always store and update cursor after sync operations
Always verify HMAC-SHA256 signatures for webhook events
Convert ISO dates with timezone to Odoo format in date handling
Keep Python files under 500 LOC when possible; split files exceeding 500 lines into smaller modules using OOP principles
Userufffor fast Python linting (replacing flake8, isort partially) with configuration inpyproject.toml
Useblackcode formatter with 100 character line length configured inpyproject.toml
Useisortfor import sorting with Odoo-specific sections, configured inpyproject.toml
Use_()function for all user-facing strings in Python modules
Files:
models/__init__.pymodels/tesote_account.py__manifest__.pybin/setup-api-local.pymodels/tesote_account_balance_history.pytests/test_balance_history.py
models/tesote_account.py
📄 CodeRabbit inference engine (CLAUDE.md)
Track sync cursor in financial accounts model for incremental synchronization
Files:
models/tesote_account.py
tests/test_*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
responseslibrary to mock HTTP calls in unit tests
Files:
tests/test_balance_history.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: tesote/tesote-odoo-api-connector PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:04:17.825Z
Learning: Applies to models/tesote_account.py : Track sync cursor in financial accounts model for incremental synchronization
📚 Learning: 2025-11-25T22:08:03.288Z
Learnt from: sebyx07
Repo: tesote/tesote-odoo-api-connector PR: 0
File: :0-0
Timestamp: 2025-11-25T22:08:03.288Z
Learning: All UI changes in the tesote-odoo-api-connector repository must include updates to translation files: i18n/tesote_connector.pot (template), i18n/en_US.po (English), and i18n/es.po (Spanish). This applies to any new field labels, menu items, view titles, filter labels, help texts, and other user-facing strings.
Applied to files:
views/tesote_account_balance_history_views.xmli18n/es.po__manifest__.py
📚 Learning: 2025-11-24T17:04:17.825Z
Learnt from: CR
Repo: tesote/tesote-odoo-api-connector PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:04:17.825Z
Learning: Applies to i18n/*.{pot,po} : Maintain translation files: `i18n/tesote_connector.pot` (template), `i18n/en_US.po` (English), `i18n/es.po` (Spanish)
Applied to files:
i18n/es.po
📚 Learning: 2025-11-24T17:04:17.825Z
Learnt from: CR
Repo: tesote/tesote-odoo-api-connector PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:04:17.825Z
Learning: Applies to models/tesote_binding.py : Use abstract base model pattern in `tesote_binding.py` as foundation for all tesote.com models
Applied to files:
models/__init__.py
📚 Learning: 2025-11-24T17:04:17.825Z
Learnt from: CR
Repo: tesote/tesote-odoo-api-connector PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:04:17.825Z
Learning: Applies to models/tesote_account.py : Track sync cursor in financial accounts model for incremental synchronization
Applied to files:
models/__init__.pymodels/tesote_account.pymodels/tesote_account_balance_history.pytests/test_balance_history.py
🧬 Code graph analysis (3)
models/tesote_account.py (3)
tests/conftest.py (1)
depends(59-63)run_tests.py (1)
depends(64-68)models/tesote_account_balance_history.py (1)
record_balance(162-208)
models/tesote_account_balance_history.py (1)
utils/colored_logger.py (1)
get_logger(262-355)
tests/test_balance_history.py (2)
models/tesote_account_balance_history.py (2)
TesoteAccountBalanceHistory(20-225)record_balance(162-208)models/tesote_account.py (1)
TesoteAccount(22-600)
🪛 GitHub Actions: lint
bin/setup-api-local.py
[error] 30-30: Ruff: F-string without any placeholders. Remove extraneous f prefix. (Suggested fix: remove 'f' from f"..." since there are no placeholders.)
tests/test_balance_history.py
[error] 9-9: Ruff: F401 'unittest.mock.patch' imported but unused. Remove unused import or use it. (2 fixable with the --fix option.)
🔇 Additional comments (24)
Tesote_Accounting_API.postman_collection.json (2)
20-60: HMAC signing logic looks correct.The prerequest script properly constructs signed payloads and generates HMAC-SHA256 signatures. Query parameter sorting (line 42) ensures signature consistency.
Ensure the server-side signature verification logic matches this payload construction format exactly (timestamp.path.body for POST, timestamp.path.sorted_query_string for other methods).
Based on coding guidelines, HMAC-SHA256 signature verification is required for webhook events. Verify that the server-side implementation uses the same payload format.
61-170: API endpoint definitions are well-structured.The collection includes appropriate CRUD and query operations for accounts with cursor-based pagination and search capabilities.
bin/setup-api-local.py (2)
42-66: Backend setup logic is correct.The script appropriately checks for existing backends before creating new ones, avoiding duplicates.
69-101: Webhook configuration logic is well-implemented.The secret key generation using
secrets.token_urlsafe(32)provides cryptographically secure 256-bit keys suitable for HMAC-SHA256 authentication.views/tesote_account_views.xml (1)
17-19: Balance History stat button correctly integrated.The button follows Odoo UI conventions and properly references the action method and computed count field defined in the model.
models/__init__.py (1)
11-11: New model import correctly positioned.The balance history model import is appropriately placed after tesote_account, following the established import order conventions.
views/tesote_menu.xml (1)
121-127: Balance Dashboard menu item properly configured.The menu is logically placed under Reports with appropriate sequencing and references the dashboard action defined in the balance history views.
__manifest__.py (1)
118-118: Views file correctly registered in manifest.The new balance history views file is properly added to the data list with appropriate load order positioning.
views/tesote_account_balance_history_views.xml (5)
6-35: Form view properly configured as read-only.The form view appropriately restricts create/edit/delete operations for historical balance records.
38-57: List view with effective visual indicators.The color-coded decorations for balance changes (green for increases, red for decreases) enhance data readability, and the sum aggregate on balance provides useful totals.
60-83: Graph and pivot views well-designed for balance analysis.The line chart effectively visualizes balance trends over time, while the pivot table enables multi-dimensional analysis by account and month.
86-121: Comprehensive search filters and grouping options.The search view provides well-thought-out time range presets, balance change filters, and grouping options that support various analysis scenarios.
124-141: Dashboard action well-configured with sensible defaults.The view mode ordering prioritizes visualization (graph/pivot), and the 30-day default filter provides a reasonable initial time window for analysis.
tests/test_balance_history.py (1)
301-321: LGTM!Good structural tests verifying the account model has the expected balance history fields and methods.
models/tesote_account.py (4)
60-65: LGTM!The computed method correctly calculates the balance history count. For accounts with very large history recordsets, consider using
search_countfor better performance, butlen()is acceptable for typical use cases.
66-79: LGTM!The action method is well-implemented with proper
ensure_one()guard, translated title, and sensible defaults including the last 30 days filter.
107-116: LGTM!Field definitions are correct. The One2many establishes the proper relationship with the balance history model.
508-518: LGTM!Initial balance is correctly recorded after account creation. The truthy check on
vals.get("balance")appropriately skips recording for zero balances (which would be filtered by the 0.01 threshold anyway).models/tesote_account_balance_history.py (6)
131-141: LGTM!The computation correctly handles edge cases including zero/None previous balance to avoid division by zero.
143-150: LGTM!The display name computation properly uses the translation function and handles missing data gracefully.
152-159: LGTM!Clean extraction of date portion for graph grouping purposes.
161-208: LGTM!The
record_balancemethod is well-implemented with proper previous balance lookup, threshold filtering, and traceability support.
1-27: LGTM!Well-structured new model with appropriate metadata, ordering, and field definitions for the balance history feature.
87-93: Thetesote.sync.logmodel is properly defined in the codebase atmodels/tesote_sync_log.py. The field reference is valid and requires no changes.
| #. module: tesote_connector | ||
| #: model:ir.model.fields,selection:tesote_connector.field_tesote_account_balance_history__source | ||
| msgid "Synchronization" | ||
| msgstr "Sincronizacion" |
There was a problem hiding this comment.
Correct Spanish accent marks for proper orthography.
Several Spanish translations are missing required accent marks (tildes):
- Line 732: "Sincronizacion" → "Sincronización"
- Line 737: "Actualizacion de Webhook" → "Actualización de Webhook"
- Line 742: "Actualizacion Manual" → "Actualización Manual"
- Line 770: "Ultimos 90 Dias" → "Últimos 90 Días"
- Line 785: "Analisis de Saldo" → "Análisis de Saldo"
- Line 790: "Informacion de Cuenta" → "Información de Cuenta"
🔎 Proposed fix
-msgstr "Sincronizacion"
+msgstr "Sincronización"
-msgstr "Actualizacion de Webhook"
+msgstr "Actualización de Webhook"
-msgstr "Actualizacion Manual"
+msgstr "Actualización Manual"
-msgstr "Ultimos 90 Dias"
+msgstr "Últimos 90 Días"
-msgstr "Analisis de Saldo"
+msgstr "Análisis de Saldo"
-msgstr "Informacion de Cuenta"
+msgstr "Información de Cuenta"Also applies to: 737-737, 742-742, 770-770, 785-785, 790-790
🤖 Prompt for AI Agents
In i18n/es.po around lines 732, 737, 742, 770, 785 and 790, several Spanish
translations lack proper accent marks; update the msgstr values to use correct
orthography: change "Sincronizacion" to "Sincronización", "Actualizacion de
Webhook" to "Actualización de Webhook", "Actualizacion Manual" to "Actualización
Manual", "Ultimos 90 Dias" to "Últimos 90 Días", "Analisis de Saldo" to
"Análisis de Saldo", and "Informacion de Cuenta" to "Información de Cuenta" so
each corresponding msgstr contains the accented characters.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 58e6856 in 30 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. tests/test_balance_history.py:9
- Draft comment:
Removed unused 'patch' import. It cleans up the code since patch isn't used. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_pqKAXVfb9lXot7rh
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 82891e1 in 30 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. bin/setup-api-local.py:30
- Draft comment:
Unnecessary f-string removed; a plain string literal suffices when no interpolation is needed. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_sc0GHp8bI9yMnKY4
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/test_balance_history.py (2)
194-247: Tests simulate logic instead of calling actual methods.Lines 203-208, 221-226, and 239-244 manually replicate the balance change detection logic (threshold check and conditional recording) rather than invoking the actual
update_from_tesotemethod. This means:
- Tests verify the test's simulation, not the implementation
- Changes to the actual method won't be caught by these tests
- False test coverage metrics
Refactor to call the real methods being tested:
def test_update_from_tesote_records_balance_change(self): # Setup actual account with old balance account = self.env['tesote.account'].create({ 'name': 'Test Account', 'balance': 1000.00, }) # Call actual method with new balance account.update_from_tesote({'balance': 1500.00}) # Verify history was recorded self.assertEqual(len(account.balance_history_ids), 1) self.assertEqual(account.balance_history_ids[0].balance, 1500.00)
203-203: Extract hardcoded threshold to a constant.The threshold value
0.01is duplicated across multiple test methods. Extract to a module-level constant or reference the actual constant from the implementation:# At module level BALANCE_CHANGE_THRESHOLD = 0.01 # Or reference from implementation from models.tesote_account import BALANCE_CHANGE_THRESHOLDAlso applies to: 221-221, 239-239
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_balance_history.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use manual capture helperscapture_exception()andcapture_message()for Sentry error tracking
Always store and update cursor after sync operations
Always verify HMAC-SHA256 signatures for webhook events
Convert ISO dates with timezone to Odoo format in date handling
Keep Python files under 500 LOC when possible; split files exceeding 500 lines into smaller modules using OOP principles
Userufffor fast Python linting (replacing flake8, isort partially) with configuration inpyproject.toml
Useblackcode formatter with 100 character line length configured inpyproject.toml
Useisortfor import sorting with Odoo-specific sections, configured inpyproject.toml
Use_()function for all user-facing strings in Python modules
Files:
tests/test_balance_history.py
tests/test_*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
responseslibrary to mock HTTP calls in unit tests
Files:
tests/test_balance_history.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: tesote/tesote-odoo-api-connector PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:04:17.825Z
Learning: Applies to models/tesote_account.py : Track sync cursor in financial accounts model for incremental synchronization
📚 Learning: 2025-11-24T17:04:17.825Z
Learnt from: CR
Repo: tesote/tesote-odoo-api-connector PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:04:17.825Z
Learning: Applies to models/tesote_account.py : Track sync cursor in financial accounts model for incremental synchronization
Applied to files:
tests/test_balance_history.py
🔇 Additional comments (1)
tests/test_balance_history.py (1)
14-179: Refactor to use Odoo's testing framework instead of pytest with mocks.The test class uses pytest fixtures and mocks extensively rather than Odoo's built-in testing framework. For Odoo addons, tests should inherit from
odoo.tests.common.TransactionCaseorSingleTransactionCaseand useself.env['model.name']to access models with a real test database.Current issues:
- Lines 41, 49, 57, 74: Direct imports from
models.*instead of accessing via Odoo registry- Heavy reliance on
Mockobjects that don't test actual implementation behavior- No database transaction isolation provided by Odoo test framework
- Missing Odoo test decorators (e.g.,
@tagged('post_install', '-at_install'))This approach may miss integration issues, database constraints, and Odoo framework interactions.
Example: Standard Odoo test structure
from odoo.tests.common import TransactionCase from odoo.tests import tagged @tagged('post_install', '-at_install') class TestTesoteAccountBalanceHistory(TransactionCase): def setUp(self): super().setUp() # Create real test data using Odoo ORM self.account = self.env['tesote.account'].create({ 'name': 'Test Checking Account', 'tesote_id': 'acc-001', 'balance': 1000.00, }) def test_record_balance_creates_snapshot(self): # Test actual model behavior history = self.env['tesote.account.balance.history'].record_balance( account=self.account, balance=1500.00, source='sync', ) self.assertEqual(history.balance, 1500.00) self.assertEqual(history.balance_change, 500.00)⛔ Skipped due to learnings
Learnt from: CR Repo: tesote/tesote-odoo-api-connector PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-24T17:04:17.825Z Learning: Applies to tests/conftest.py : Mock all Odoo dependencies in `tests/conftest.py` to allow tests to run without Odoo installationLearnt from: CR Repo: tesote/tesote-odoo-api-connector PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-24T17:04:17.825Z Learning: Applies to models/tesote_account.py : Track sync cursor in financial accounts model for incremental synchronization
1 participant
The Balance Dashboard feature is now complete. Here's a summary:
New Files Created
Files Modified
_compute_balance_history_count), plus balance recording in create_from_tesote() and update_from_tesote()
Features
Important
Adds a Balance Dashboard for Tesote accounts with models, views, and tests for tracking and analyzing balance history.
tesote.account.balance.historymodel intesote_account_balance_history.pyfor storing balance snapshots.tesote_account.pywith fieldsbalance_history_idsandbalance_history_count, and methodsaction_view_balance_historyand_compute_balance_history_count.create_from_tesote()andupdate_from_tesote().tesote_account_balance_history_views.xmlfor form, list, graph, pivot, and search.tesote_account_views.xmlfor accessing balance history from account form.tesote_menu.xmlunder Reports.test_balance_history.pywith 18 unit tests for balance history functionality.__manifest__.pyto include new views file.ir.model.access.csv.i18n/es.po.This description was created by
for 82891e1. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.