Conversation
…on fixes - Add development_status: Beta to both manifests - Replace markdown tables with bullet lists in DESCRIPTION.md (CI compat) - Fix variable shadowing bug in studio router list_variables endpoint - Add unit tests for spp_gis_report_programs (report model + wizard)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #90 +/- ##
==========================================
+ Coverage 69.67% 70.18% +0.50%
==========================================
Files 666 685 +19
Lines 36682 37422 +740
==========================================
+ Hits 25558 26264 +706
- Misses 11124 11158 +34
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request advances the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly promotes spp_gis_report_programs and spp_studio_api_v2 to Beta status, fixes a variable shadowing bug, and adds a valuable test suite. The documentation updates are also beneficial. I've included one suggestion to further refactor the code in spp_studio_api_v2/routers/studio.py for improved conciseness by removing temporary variables.
| var_cel_accessor = var.cel_accessor | ||
| var_value_type = var.value_type | ||
| var_source_type = var.source_type | ||
| var_applies_to = var.applies_to | ||
|
|
||
| if not cel_accessor or not value_type or not source_type or not applies_to: | ||
| if not var_cel_accessor or not var_value_type or not var_source_type or not var_applies_to: | ||
| _logger.warning("Skipping variable ID %s with missing required fields", var.id) | ||
| continue | ||
|
|
||
| # Ensure string type for required fields (some may be False/None from Odoo) | ||
| try: | ||
| items.append( | ||
| VariableInfo( | ||
| name=str(cel_accessor), | ||
| label=str(getattr(var, "label", None) or var.name or cel_accessor), | ||
| name=str(var_cel_accessor), | ||
| label=str(getattr(var, "label", None) or var.name or var_cel_accessor), | ||
| description=str(getattr(var, "description", None) or "") | ||
| if getattr(var, "description", None) | ||
| else None, | ||
| valueType=str(value_type), | ||
| sourceType=str(source_type), | ||
| appliesTo=str(applies_to), | ||
| valueType=str(var_value_type), | ||
| sourceType=str(var_source_type), | ||
| appliesTo=str(var_applies_to), |
There was a problem hiding this comment.
While the fix for variable shadowing is correct, this block can be made more concise by removing the temporary variables (var_cel_accessor, var_value_type, etc.) and accessing the var object's attributes directly. This improves readability and reduces redundancy.
if not var.cel_accessor or not var.value_type or not var.source_type or not var.applies_to:
_logger.warning("Skipping variable ID %s with missing required fields", var.id)
continue
# Ensure string type for required fields (some may be False/None from Odoo)
try:
items.append(
VariableInfo(
name=str(var.cel_accessor),
label=str(getattr(var, "label", None) or var.name or var.cel_accessor),
description=str(getattr(var, "description", None) or "")
if getattr(var, "description", None)
else None,
valueType=str(var.value_type),
sourceType=str(var.source_type),
appliesTo=str(var.applies_to),Add test_coverage_gaps.py with 60+ tests covering monkey-patched service closures, studio field registration edge cases, extension write service conversions, variable value service edge cases, and FastAPI endpoint integration. Coverage: spp_studio_api_v2 95%, spp_gis_report_programs 97%.
emjay0921
deleted the
fix/alpha-to-beta-gis-report-programs-studio-api-v2
branch
2 participants
Why is this change needed?
Promote
spp_gis_report_programsandspp_studio_api_v2from Alpha to Beta status. Fixes bugs and adds missing tests identified during review.How was the change implemented?
development_status: Betato both manifestsoca-gen-addon-readme)spp_studio_api_v2/routers/studio.py—list_variablesendpoint was overwriting function parameterssource_typeandapplies_towith loop variablesspp_gis_report_programs(14 tests covering report model and wizard)New unit tests
spp_gis_report_programs/tests/test_gis_report_programs.py— 14 tests:TestGISReportProgramsModel: field existence, create with/without program,_apply_context_filterswith program/without/non-partner modelTestGISReportWizardPrograms: field existence, validation (required/provided/not-required), context filter vals, code suffixUnit tests executed by the author
spp_gis_report_programs: 0 failed, 0 errors of 14 testsspp_studio_api_v2: 0 failed, 0 errors of 89 testsHow to test manually
spp_gis_report_programswizard shows program field when template requires programspp_studio_api_v2/Studio/variablesendpoint returns correct data without parameter corruptionRelated links