Skip to content

ESD-901: Add product pricing to ProductService - #149

Merged
Phil-Browne merged 12 commits into
mainfrom
feature/ESD-889-product-pricing
May 28, 2026
Merged

ESD-901: Add product pricing to ProductService#149
Phil-Browne merged 12 commits into
mainfrom
feature/ESD-889-product-pricing

Conversation

@Phil-Browne

@Phil-Browne Phil-Browne commented Apr 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Adds GetProductPricing to ProductService, backed by POST /v4/pricebook/product. Supports pricing requests for VXC, MCR, Port (Megaport), MVE, IX, NAT Gateway, and IP Address product types via the PriceBookRequest interface, and returns per-product monthly rates, price elements, and applied discounts.

Also adds GetProductPricingForCompany for partner/reseller pricing scoped to another company via mutually exclusive companyId / companyUid query parameters.

Part of ESD-889.

@Phil-Browne Phil-Browne changed the title ESD-889: Add product pricing to ProductService ESD-901: Add product pricing to ProductService Apr 25, 2026
@Phil-Browne
Phil-Browne requested a review from Copilot April 26, 2026 12:16
Copilot AI reviewed Apr 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new pricing lookup capability to ProductService by introducing GetProductPricing, request/response pricing types, and unit tests to validate basic behavior against POST /v4/pricebook/product.

Changes:

  • Added GetProductPricing(ctx, req) to ProductService / ProductServiceOp backed by POST /v4/pricebook/product.
  • Introduced pricing_types.go containing pricebook request/response DTOs and enums.
  • Added unit tests for the happy-path pricing response and nil-request handling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
product.go Adds GetProductPricing implementation and wires it into the ProductService interface.
pricing_types.go Defines request/response types, enums, and error values for pricebook pricing.
product_test.go Adds tests for GetProductPricing success and nil request behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread product.go Outdated
Comment thread pricing_types.go
Comment thread pricing_types.go
Comment thread product_test.go
Comment thread product.go Outdated
Comment thread product.go
@Phil-Browne
fix(pricing): fix DiscountDetails nesting; add IPAddress type; add ad…
d1dcd3e 
…dOns field; auto-inject productType; per-type validation; companyId/companyUid support; full test coverage
@Phil-Browne
Phil-Browne marked this pull request as draft April 26, 2026 12:39
@Phil-Browne
fix(pricing): address Copilot review comments and linter failure
e8b815e 
- Fix gofmt alignment in pricing_types.go (struct tag spacing)
- Guard typed-nil PriceBookRequest via reflect.ValueOf(req).IsNil();
  req == nil only catches a nil interface, not (*T)(nil)
- Capture *http.Response from Client.Do and defer resp.Body.Close()
  in both GetProductPricing and GetProductPricingForCompany to avoid
  leaking the connection
- Return error when envelope.Data == nil so callers get a clear error
  instead of a silent (nil, nil) return on a 2xx with empty body
- Add TestGetProductPricingTypedNilRequest to lock in typed-nil guard
@Phil-Browne
Phil-Browne requested a review from Copilot April 26, 2026 17:04
Copilot AI reviewed Apr 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread product.go Outdated
Comment thread product.go
Comment thread product.go
Comment thread pricing_types.go
@Phil-Browne
fix: guard typed-nil pricing requests and validate mutually exclusive…
7df8e2a 
… company identifiers

- Push the nil / typed-nil interface guard down into validatePriceBookRequest
  so both GetProductPricing and GetProductPricingForCompany are covered with
  a single check, preventing nil-pointer panics when any *XPriceBookRequest
  case dereferences r.Field.
- Reject GetProductPricingForCompany requests that set both CompanyID and
  CompanyUID, since the docs and API treat them as mutually exclusive.
- Add ErrPricingCompanyIDAndUIDSet and tests covering nil wrapper, nil inner
  request, typed-nil inner request, and the mutually exclusive identifier case.
@Phil-Browne

Phil-Browne commented Apr 26, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Copilot review feedback in 7df8e2a:

  • Pushed the nil / typed-nil interface guard down into validatePriceBookRequest so both GetProductPricing and GetProductPricingForCompany are covered consistently. This also protects every case *XPriceBookRequest: in the validator from dereferencing a nil concrete pointer.
  • Removed the redundant req == nil || reflect.ValueOf(req).IsNil() check in GetProductPricing (now handled by the validator).
  • Added validation in GetProductPricingForCompany that rejects requests with both CompanyID and CompanyUID set, since they are mutually exclusive. Added ErrPricingCompanyIDAndUIDSet for this.
  • Added tests covering: nil wrapper, nil inner request, typed-nil inner request, and the mutually exclusive identifier case.
  • Updated PR description to include IP Address in the supported product types list.

@Phil-Browne
Phil-Browne requested a review from Copilot April 26, 2026 17:14
Copilot AI reviewed Apr 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread product.go Outdated
@Phil-Browne

Phil-Browne commented Apr 26, 2026

Copy link
Copy Markdown
Contributor Author

Adding GetProductPricing and GetProductPricingForCompany to ProductService is intentional — this follows the same pattern used in PR #150 where telemetry methods were added directly to the Port, MCR, MVE, VXC, and IX service interfaces. Downstream mocks will need stubs added, which is acceptable for a pre-v1 library.

@Phil-Browne
Phil-Browne requested a review from Copilot April 26, 2026 17:23
Copilot AI reviewed Apr 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread product.go Outdated
Comment thread pricing_types.go Outdated
@Phil-Browne
fix: guard reflect IsNil with kind check and rename PriceBookDto to P…
45a9362 
…riceBookDTO

Extract isNilPriceBookRequest helper that checks Kind before calling IsNil to
prevent panics on non-nilable types. Rename PriceBookDto to PriceBookDTO to
follow Go initialism conventions.
@Phil-Browne

Phil-Browne commented Apr 26, 2026

Copy link
Copy Markdown
Contributor Author

Both Copilot issues addressed in 45a9362:

1. Safe reflect nil check — Extracted isNilPriceBookRequest helper that guards the Kind before calling IsNil, preventing panics on non-nilable kinds (struct, int, etc.). validatePriceBookRequest now delegates to this helper.

2. PriceBookDto → PriceBookDTO — Renamed to follow Go initialism conventions, touching pricing_types.go (struct definition + productPricingResponse field) and product.go (interface methods + implementations). All 51 product suite tests pass.

@Phil-Browne
Phil-Browne requested a review from Copilot April 26, 2026 17:35
Copilot AI reviewed Apr 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread product.go
@Phil-Browne

Phil-Browne commented Apr 26, 2026

Copy link
Copy Markdown
Contributor Author

Addressed: added case *MVEPriceBookRequest: (validates LocationID != 0, returning ErrPricingMVELocationRequired) and a default: case returning fmt.Errorf("unsupported pricing request type: %T", req) in validatePriceBookRequest. Also added the sentinel error ErrPricingMVELocationRequired in pricing_types.go alongside the other ErrPricing* errors, and a test assertion in TestGetProductPricingValidation. All pricing tests pass.

@Phil-Browne
Phil-Browne requested a review from Copilot April 26, 2026 17:49
Copilot AI reviewed Apr 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread product.go
Comment thread product.go
Comment thread product.go
@Phil-Browne
fix: sentinel error for empty pricing response and add CompanyUID test
c40b3d2 
Replace inline fmt.Errorf for missing pricing data with package-level
ErrProductPricingResponseEmpty sentinel in pricing_types.go. Add
TestGetProductPricingForCompanyUID to assert companyUid query param is
sent when CompanyUID is set (and companyId is absent).
@Phil-Browne

Phil-Browne commented Apr 26, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both review points in c40b3d2:

  • Sentinel error: replaced both inline fmt.Errorf("product pricing response missing data") calls with a new package-level ErrProductPricingResponseEmpty sentinel defined alongside the other ErrPricing* vars in pricing_types.go.
  • CompanyUID test: added TestGetProductPricingForCompanyUID to product_test.go — registers a handler on suite.mux, asserts companyUid is present in the query string and companyId is absent, following the existing TestGetProductPricingForCompany pattern.

All product tests pass.

@Phil-Browne
Phil-Browne requested a review from Copilot April 26, 2026 17:56
Copilot AI reviewed Apr 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pricing_types.go Outdated
Comment thread pricing_types.go Outdated
Comment thread product.go Outdated
@Phil-Browne
refactor: lowercase error strings, remove omitempty from required fie…
a2ec5e3 
…ld, extract body builder helper

- Lowercase all ErrPricing* error message strings per Go convention
- Remove omitempty from MVEPriceBookRequest.LocationID (field is required)
- Extract buildPriceBookRequestBody helper to eliminate duplication between GetProductPricing and GetProductPricingForCompany
@Phil-Browne

Phil-Browne commented Apr 26, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Copilot feedback: lowercased all ErrPricing* error strings per Go convention, removed omitempty from MVEPriceBookRequest.LocationID (field is required by validation), and extracted a buildPriceBookRequestBody helper to eliminate the marshal→unmarshal→inject-productType duplication between GetProductPricing and GetProductPricingForCompany. All tests pass.

@Phil-Browne
Phil-Browne marked this pull request as ready for review April 26, 2026 18:08
@Phil-Browne
Phil-Browne requested a review from Copilot April 27, 2026 00:37
Copilot AI reviewed Apr 27, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pricing_types.go
Comment thread product.go
@Phil-Browne
fix: correct AddOnType constant value in pricing comment and add Pric…
ecd246f 
…ingAddOnType constants

Replace the incorrect "IPSEC" string in the ProductAddOnPriceBookRequest comment
with the correct "IP_SEC" value (matching AddOnTypeIPsec in mcr_types.go), and
introduce exported PricingAddOnTypeCrossConnect and PricingAddOnTypeIPSec constants.
@Phil-Browne

Phil-Browne commented Apr 27, 2026

Copy link
Copy Markdown
Contributor Author

The addition of GetProductPricing and GetProductPricingForCompany to the ProductService interface is an intentional breaking change. This SDK is pre-v1 and breaking changes to interfaces are expected as we expand functionality. Consumers that embed or mock ProductService will need to implement the new methods, which is acceptable at this stage of the SDK's lifecycle.

@Phil-Browne
Phil-Browne requested a review from Copilot April 27, 2026 00:51
Copilot AI reviewed Apr 27, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Phil-Browne
Phil-Browne requested review from a team and megasergeyd and removed request for a team May 28, 2026 12:42
@Phil-Browne
Phil-Browne merged commit 306826f into main May 28, 2026
2 checks passed
@Phil-Browne
Phil-Browne deleted the feature/ESD-889-product-pricing branch May 28, 2026 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@megasergeyd megasergeyd megasergeyd approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Phil-Browne @megasergeyd