feat: add protobuf definitions for raw material cost calculations and history tracking#22
Conversation
… history tracking
ilramdhan
added
documentation
There was a problem hiding this comment.
Pull request overview
Adds new finance.v1 protobuf APIs to support raw material grouping and landed-cost calculation workflows, including async recalculation triggering and history tracking.
Changes:
- Introduces
RMGroupServicewith CRUD, item assignment/removal, ungrouped-items reporting, and import/export/template RPCs. - Introduces
RMCostServicewith async trigger + synchronous admin calculation, listing/fetching costs, listing history, and period/export helpers. - Adds new domain enums/messages to model RM group configuration, per-stage rates, computed costs, and append-only calculation history.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| finance/v1/rm_group.proto | New RM group domain messages + RMGroupService endpoints for managing groups and memberships, plus import/export/template and reporting APIs. |
| finance/v1/rm_cost.proto | New RM cost + history domain messages and RMCostService endpoints for recalculation trigger/calc and querying/exporting results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // How to handle existing groups by `group_code`: "skip" (default) or "update". | ||
| // Detail rows always additive: items already active in ANOTHER group are | ||
| // reported as skipped errors; items already in the target group are ignored. | ||
| string duplicate_action = 3 [(buf.validate.field).string = { | ||
| in: [ |
There was a problem hiding this comment.
duplicate_action is documented as defaulting to "skip", but the validation requires min_len: 1 (so callers must always supply it). Either allow empty string and treat it as "skip", or update the comment to remove the default claim and mark the field as required.
| message GetRMGroupItemRatesResponse { | ||
| // Standard response envelope. | ||
| common.v1.BaseResponse base = 1; | ||
| // Per-item rates, one row per active detail. |
There was a problem hiding this comment.
The comment says this returns "one row per active detail", but RMGroupItemRates includes is_active/is_dummy flags, implying inactive/dummy rows may be included too. Please clarify whether the response includes only active details or all details.
| // Per-item rates, one row per active detail. | |
| // Per-item rates, one row per detail; use `is_active` and `is_dummy` | |
| // to distinguish active/inactive and dummy rows. |
| package finance.v1; | ||
|
|
||
| import "buf/validate/validate.proto"; | ||
| import "common/v1/common.proto"; | ||
| import "finance/v1/uom.proto"; |
There was a problem hiding this comment.
This new finance/v1 proto file is missing the standard header note used elsewhere ("Note: go_package is managed by buf.gen.yaml managed mode") right after the package declaration. Add it for consistency with other finance/v1 protos.
| package finance.v1; | ||
|
|
||
| import "buf/validate/validate.proto"; | ||
| import "common/v1/common.proto"; | ||
| import "finance/v1/rm_group.proto"; |
There was a problem hiding this comment.
This new finance/v1 proto file is missing the standard header note used elsewhere ("Note: go_package is managed by buf.gen.yaml managed mode") right after the package declaration. Add it for consistency with other finance/v1 protos.
| RMGroupFlag flag_valuation_used = 17; | ||
| // Stage actually used after cascade / INIT resolution. | ||
| RMGroupFlag flag_marketing_used = 18; | ||
| // Stage actually used after cascade / INIT resolution. | ||
| RMGroupFlag flag_simulation_used = 19; |
There was a problem hiding this comment.
In RMCost, tag ordering is confusing: fields with tags 17–19 are declared before tag 16 (audit, declared later in the message). Reorder the field declarations to follow ascending tag order (and keep audit = 16 before any higher-numbered fields) to match other finance protos and reduce maintenance mistakes.
| // ListRMCostPeriods returns distinct periods from cost rows (newest first). | ||
| rpc ListRMCostPeriods(ListRMCostPeriodsRequest) returns (ListRMCostPeriodsResponse) { | ||
| option (google.api.http) = {get: "/api/v1/finance/rm-costs/periods"}; | ||
| } | ||
|
|
There was a problem hiding this comment.
ListRMCostPeriods references request/response types that are declared later in the file (after the service). This works, but it’s inconsistent with other finance/v1 protos where messages are declared before the service; consider moving the Periods/Export message blocks above the service for easier navigation.
| "", | ||
| "code", | ||
| "group_name", | ||
| "sort_order", |
There was a problem hiding this comment.
ListRMGroupsRequest.sort_by includes "sort_order" in the allowed values, but this is a group-head listing and RMGroupHead doesn’t expose any sort_order-like field. If unsupported server-side, clients may send an allowed value that errors or no-ops. Consider removing "sort_order" from the allowed list or documenting what it sorts by.
| "sort_order", |
Description
Add protobuf definitions for raw material cost calculations and history tracking
Change Type
Proto Files Changed
finance/v1/rm_cost.protofinance/v1/rm_group.protoPre-merge Checklist
buf format -wappliedbuf lintpassesbuf breakingpassesImpact Assessment