Skip to content

ECOPROJECT-5132 | feat: add assessment enhancement data endpoints - #1404

Open
turjemantal wants to merge 1 commit into
kubev2v:mainfrom
turjemantal:ECOPROJECT-5132
Open

ECOPROJECT-5132 | feat: add assessment enhancement data endpoints#1404
turjemantal wants to merge 1 commit into
kubev2v:mainfrom
turjemantal:ECOPROJECT-5132

Conversation

@turjemantal

@turjemantal turjemantal commented Aug 6, 2026

Copy link
Copy Markdown

What problem does this PR solve?

VMA enhancement data cannot be auto-collected by Agent or RVTools, leaving missing assessment data without a dedicated mechanism for users to provide it manually.

How was it solved?

Introduced POST and GET /api/v1/assessments/{id}/enhancement-data endpoints to persist and retrieve user-provided VMA enhancement data.

Summary by CodeRabbit

  • New Features

    • Added the ability to save and retrieve assessment enhancement data.
    • Supports environment, VMware, vSphere, NSX, Aria, subscription, and customer details.
    • Added validation for assessment identifiers and structured input values.
    • Saved data persists across requests and can be updated or replaced.
  • Bug Fixes

    • Added clear handling for invalid requests, missing assessments, unauthorized access, and server errors.
  • Tests

    • Added coverage for saving, retrieving, updating, validation, authorization, persistence, and database behavior.

@turjemantal
turjemantal requested a review from a team as a code owner August 6, 2026 08:31
@turjemantal
turjemantal requested review from AvielSegev and tupyy and removed request for a team August 6, 2026 08:31
@openshift-ci

openshift-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nirarg for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@turjemantal, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4abc8df4-88ff-49c2-b6c2-5af9b3b65eb8

📥 Commits

Reviewing files that changed from the base of the PR and between 5dd47d4 and b6a12c2.

📒 Files selected for processing (29)
  • api/v1alpha1/openapi.yaml
  • api/v1alpha1/spec.gen.go
  • api/v1alpha1/types.gen.go
  • internal/api/client/client.gen.go
  • internal/api/server/server.gen.go
  • internal/api_server/server.go
  • internal/handlers/v1alpha1/accounts_test.go
  • internal/handlers/v1alpha1/assessment_enhancement_data.go
  • internal/handlers/v1alpha1/assessment_enhancement_data_test.go
  • internal/handlers/v1alpha1/assessment_test.go
  • internal/handlers/v1alpha1/estimation_test.go
  • internal/handlers/v1alpha1/handler.go
  • internal/handlers/v1alpha1/job_test.go
  • internal/handlers/v1alpha1/partner_test.go
  • internal/handlers/v1alpha1/sizer_test.go
  • internal/handlers/v1alpha1/source_test.go
  • internal/service/assessment_enhancement_data.go
  • internal/service/assessment_enhancement_data_test.go
  • internal/service/eventwrap/dispatcher_test.go
  • internal/service/sizer_test.go
  • internal/service/source_test.go
  • internal/store/assessment_enhancement_data.go
  • internal/store/assessment_enhancement_data_test.go
  • internal/store/model/assessment_enhancement_data.go
  • internal/store/model/stringarray.go
  • internal/store/model/stringarray_test.go
  • internal/store/store.go
  • pkg/migrations/migrations_test.go
  • pkg/migrations/sql/20260730120000_assessment_enhancement_data.sql
📝 Walkthrough

Walkthrough

Changes

Added GET and POST assessment enhancement-data endpoints with OpenAPI schemas and generated client/server support. Added PostgreSQL persistence, typed array conversion, service mappings, authorization-aware handlers, runtime wiring, and tests.

Assessment enhancement data

Layer / File(s) Summary
API contracts and generated transport
api/v1alpha1/openapi.yaml, api/v1alpha1/types.gen.go, api/v1alpha1/spec.gen.go, internal/api/client/client.gen.go, internal/api/server/server.gen.go
Defines enhancement-data schemas, enums, request types, routes, UUID validation, client methods, response parsers, and server handlers.
Database schema and persistence
pkg/migrations/..., internal/store/..., internal/store/model/...
Adds the database table, GORM model, PostgreSQL array conversion, store accessor, upsert and retrieval operations, and persistence tests.
Service mapping and runtime wiring
internal/service/assessment_enhancement_data.go, internal/api_server/server.go, internal/service/*_test.go
Maps API data to persistence models and back, handles missing records, and constructs the service from the store.
HTTP handlers and validation
internal/handlers/v1alpha1/assessment_enhancement_data.go, internal/handlers/v1alpha1/*_test.go
Adds save and retrieval handlers, authorization and assessment checks, HTTP error mapping, handler tests, and updated test dependencies.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested reviewers: avielsegev, ronenav, tupyy

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ChiRouter
  participant ServiceHandler
  participant AssessmentEnhancementDataService
  participant AssessmentEnhancementDataStore
  Client->>ChiRouter: POST enhancement data
  ChiRouter->>ServiceHandler: Validate UUID and request body
  ServiceHandler->>AssessmentEnhancementDataService: SaveEnhancementData
  AssessmentEnhancementDataService->>AssessmentEnhancementDataStore: Upsert enhancement data
  AssessmentEnhancementDataStore-->>AssessmentEnhancementDataService: Persisted model
  AssessmentEnhancementDataService-->>ServiceHandler: EnhancementData
  ServiceHandler-->>Client: HTTP 200 EnhancementData
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.32% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding assessment enhancement data endpoints.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@turjemantal
turjemantal requested a review from ronenav August 6, 2026 08:33

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/v1alpha1/openapi.yaml`:
- Around line 3431-3557: Add validation tags to the generated enhancement
request types for all enum fields, nonnegative counters, and unique arrays, plus
maximum-length constraints on VsphereCoreInput.vmEncryptionPolicy and
CustomerDetailsInput.targetHardware using the project’s established limits. In
SaveAssessmentEnhancementData, validate the decoded request body after
assessment ID checks and before mapping or calling SaveEnhancementData,
returning the standard request-validation error without persisting invalid
values.

In `@api/v1alpha1/types.gen.go`:
- Around line 248-257: Add x-enum-varnames under
VMwareSubscriptionInput.level.items.enum in api/v1alpha1/openapi.yaml, assigning
explicit scoped names for each VMwareSubscriptionInputLevel value, then
regenerate api/v1alpha1/types.gen.go so the exported constants use those names
instead of generic identifiers.

In `@internal/handlers/v1alpha1/assessment_enhancement_data_test.go`:
- Around line 136-165: Extend the “overwrites previous data on second save” test
after the second SaveAssessmentEnhancementData call by invoking
GetAssessmentEnhancementData for the same assessmentID. Assert the returned
response contains the persisted deployed environment value “on_cloud,” while
retaining the existing save-response assertion.

In `@internal/service/assessment_enhancement_data.go`:
- Around line 64-213: 添加针对 enhancementDataAPIToModel 和 enhancementDataModelToAPI
的表驱动单元测试,覆盖十个可选数据段、所有 typed enum 字段、nil 字段、nil slice 以及空 slice。验证 API 到 model 及
model 到 API 的字段映射、指针和切片语义,并确保每个可选 section 的存在与缺失行为符合当前转换契约。

In `@internal/store/model/stringarray.go`:
- Around line 52-80: Fix parsePostgresArray so it preserves trailing and
standalone empty elements: track whether an element has started independently of
current.Len(), append the final element unconditionally, and retain empty quoted
values. Add table cases covering StringArray{"a", ""}, StringArray{""}, and
StringArray{"", "b"} in TestStringArrayScan and TestStringArrayRoundTrip.

In `@pkg/migrations/sql/20260730120000_assessment_enhancement_data.sql`:
- Around line 4-5: Change assessment_id in the migration to the PostgreSQL UUID
type and retain its foreign-key cascade behavior, then update the corresponding
Go model field to uuid.UUID so generated SQL and runtime types match. Review
sibling reference columns in the same migration and remove fixed VARCHAR(255)
lengths where the schema standard uses unbounded text or UUID-compatible types.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 38fcdb8e-df0d-4d84-a37b-39bcb9277ea7

📥 Commits

Reviewing files that changed from the base of the PR and between 5dd47d4 and e751585.

📒 Files selected for processing (28)
  • api/v1alpha1/openapi.yaml
  • api/v1alpha1/spec.gen.go
  • api/v1alpha1/types.gen.go
  • internal/api/client/client.gen.go
  • internal/api/server/server.gen.go
  • internal/api_server/server.go
  • internal/handlers/v1alpha1/accounts_test.go
  • internal/handlers/v1alpha1/assessment_enhancement_data.go
  • internal/handlers/v1alpha1/assessment_enhancement_data_test.go
  • internal/handlers/v1alpha1/assessment_test.go
  • internal/handlers/v1alpha1/estimation_test.go
  • internal/handlers/v1alpha1/handler.go
  • internal/handlers/v1alpha1/job_test.go
  • internal/handlers/v1alpha1/partner_test.go
  • internal/handlers/v1alpha1/sizer_test.go
  • internal/handlers/v1alpha1/source_test.go
  • internal/service/assessment_enhancement_data.go
  • internal/service/eventwrap/dispatcher_test.go
  • internal/service/sizer_test.go
  • internal/service/source_test.go
  • internal/store/assessment_enhancement_data.go
  • internal/store/assessment_enhancement_data_test.go
  • internal/store/model/assessment_enhancement_data.go
  • internal/store/model/stringarray.go
  • internal/store/model/stringarray_test.go
  • internal/store/store.go
  • pkg/migrations/migrations_test.go
  • pkg/migrations/sql/20260730120000_assessment_enhancement_data.sql

Comment thread api/v1alpha1/openapi.yaml
Comment thread api/v1alpha1/types.gen.go
Comment thread internal/handlers/v1alpha1/assessment_enhancement_data_test.go
Comment thread internal/service/assessment_enhancement_data.go
Comment thread internal/store/model/stringarray.go
Comment thread pkg/migrations/sql/20260730120000_assessment_enhancement_data.sql
@turjemantal

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/service/assessment_enhancement_data_test.go`:
- Around line 61-68: Update the test cases in
internal/service/assessment_enhancement_data_test.go at lines 61-68, 105-112,
113-120, and 121-128 to use API-supported enum values: replace “staging” with
“dev”, “monitoring” with “performance_analytics”, “orchestration” with
“orchestrator”, and “compliance” with “compliance_monitoring”, including
corresponding expected assertions while preserving the existing coverage.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9f693867-c1f3-498b-a789-3d610fc41d03

📥 Commits

Reviewing files that changed from the base of the PR and between e751585 and 2d3206a.

📒 Files selected for processing (9)
  • api/v1alpha1/openapi.yaml
  • api/v1alpha1/spec.gen.go
  • api/v1alpha1/types.gen.go
  • internal/handlers/v1alpha1/assessment_enhancement_data.go
  • internal/handlers/v1alpha1/assessment_enhancement_data_test.go
  • internal/service/assessment_enhancement_data_test.go
  • internal/service/sizer_test.go
  • internal/store/model/stringarray.go
  • internal/store/model/stringarray_test.go

Comment thread internal/service/assessment_enhancement_data_test.go
Introduce POST and GET /api/v1/assessments/{id}/enhancement-data endpoints
to persist and retrieve user-provided VMA enhancement data that cannot be
auto-collected by Agent or RVTools.

Add request validation for enhancement data fields, fix parsePostgresArray
dropping empty-string elements, and add test coverage for the enhancement
data mapper functions, second-save persistence check, and API-supported
enum values.

Signed-off-by: Tal Turjeman <tturjema@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant