Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for a new “operacion” field on impuestos to capture tax operation types throughout the UI and database, backed by a new OperacionIVA helper.
- Introduce an “operation” select column in the list and edit XML views, populated from
OperacionIVA::all() - Extend the impuestos table and model (
operacioncolumn/property) to persist the new field - Add a new
OperacionIVAclass to centralize default and custom operation values
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Core/XMLView/ListImpuesto.xml | Add a hidden “operation” column ahead of VAT and bump orders for VAT, surcharge, and subaccounts |
| Core/XMLView/EditImpuesto.xml | Change layout to insert an “operation” select column and shift downstream field orders |
| Core/Table/impuestos.xml | Add a new operacion column of type varchar(50) |
| Core/Model/Impuesto.php | Declare public $operacion; on the Impuesto model |
| Core/Lib/OperacionIVA.php | New class with constants, add(), all(), and default() methods to manage operation labels |
| Core/Data/Codpais/ESP/impuestos.csv | Populate sample Spanish impuestos data with the new operacion values |
| Core/Controller/ListImpuesto.php | Load and inject OperacionIVA::all() into the list view select widget |
| Core/Controller/EditImpuesto.php | Load and inject OperacionIVA::all() into the edit form select widget |
Comments suppressed due to low confidence (4)
Core/XMLView/EditImpuesto.xml:37
- The 'operacion' select widget isn't marked as required or given a default; consider adding required="true" or preselecting the default operation to avoid null values.
<widget type="select" fieldname="operacion" translate="true" >
Core/XMLView/ListImpuesto.xml:38
- This select widget for 'operacion' has no required flag or placeholder; consider enforcing required="true" or adding a default option to prevent blank entries.
<widget type="select" fieldname="operacion" translate="true" >
Core/Table/impuestos.xml:41
- The new 'operacion' column allows nulls by default; consider adding a NOT NULL constraint or specifying a DEFAULT in your migration to backfill existing rows.
<type>character varying(50)</type>
Core/Lib/OperacionIVA.php:55
- Using 'default' as a method name conflicts with PHP's reserved keyword; please rename this method to something like getDefaultOperation().
public static function default(): string
| } | ||
| } | ||
|
|
||
| protected function loadOperations(string $viewName): void |
There was a problem hiding this comment.
[nitpick] The loadOperations method is duplicated in both ListImpuesto and EditImpuesto; extract it into a shared trait or base class to avoid repeating code.
| } | ||
| } | ||
|
|
||
| protected function loadOperations(string $viewName): void |
There was a problem hiding this comment.
[nitpick] This loadOperations logic is the same as in ListImpuesto; consider centralizing it (trait or parent class) to adhere to DRY principles.
Tarea #3989