Skip to content

Comments

Nativo: Add optional placementId parameter#4679

Open
rafataveira wants to merge 3 commits intoprebid:masterfrom
rafataveira:master
Open

Nativo: Add optional placementId parameter#4679
rafataveira wants to merge 3 commits intoprebid:masterfrom
rafataveira:master

Conversation

@rafataveira
Copy link
Contributor

This pull request updates the Nativo adapter's test and supplemental JSON files to include a placementId parameter in the ext.bidder object for all impression types (banner, native, and video) across both app and web examples. Additionally, the Nativo adapter's parameter schema (nativo.json) is updated to formally define placementId as a supported parameter.

Key changes:

Test and Example Data Updates:

  • Added a placementId field (set to 12345678) to the ext.bidder object in all impression objects within the Nativo adapter's exemplary and supplemental JSON files, covering banner, native, and video formats for both app and web. This ensures test coverage for the new parameter in all relevant scenarios. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20]

Schema Definition:

  • Updated static/bidder-params/nativo.json to define placementId as an accepted parameter (integer or string) for the Nativo adapter, providing validation and documentation for this field.

@ChrisHuie
Copy link
Contributor

AGENT TESTING

PR #4679 Review: Nativo: Add optional placementId parameter

Author: rafataveira | Type: Adapter Modification | CI: Clean (all 4 checks passed)


Triage Manifest

Field Value
PR Type adapter-modification
Files 11 files (10 test fixtures + 1 schema)
Bidder nativo
Skills activated bidder-params-pr-review, adapter-code-pr-review
CI Status Clean (validate-merge, notify, validate 1.24.x, validate 1.23.x all passed)

File Routing

Skill Files
bidder-params static/bidder-params/nativo.json [modified]
adapter-code 6 exemplary test fixtures [modified], 4 supplemental test fixtures [modified]

Issues Found

WARN: No test fixture without placementId (backward compatibility gap)

All 10 test fixtures have been updated to include "placementId": 12345678 in ext.bidder. The 4 supplemental fixtures that previously had "bidder": {} now all have "bidder": {"placementId": 12345678}.

Since placementId is optional in the schema, there should be at least one test fixture that sends an empty "bidder": {} or omits ext.bidder entirely. Without this, there is no test coverage for the backward-compatible case where publishers do not send placementId. If someone accidentally added placementId to the required array, all current tests would still pass.

Suggestion: Keep at least one supplemental fixture (e.g., 204-response-from-nativo.json or 400-response-from-nativo.json) with the original "bidder": {} to ensure empty params remain valid.

Files: All test fixtures in adapters/nativo/nativotest/


WARN: No params_test.go for schema validation

The PR adds a new parameter to nativo.json but there is no adapters/nativo/params_test.go file (it doesn't exist on master either). This means the schema change has zero unit test coverage for validation. A params_test.go should be added to verify:

  • {"placementId": 12345678} is accepted (integer)
  • {"placementId": "12345678"} is accepted (string)
  • {} is accepted (optional param, empty is valid)
  • {"placementId": true} is rejected (wrong type)

File: adapters/nativo/params_test.go (missing)


WARN: No imp_nativo.go struct for bidder params

There is no openrtb_ext/imp_nativo.go file (doesn't exist on master). The adapter passes the entire bid request through transparently without parsing bidder ext, so this technically works. However, the PBS convention for adapters with bidder params is to define a Go struct. If one is created in the future, the placementId field should use type jsonutil.StringInt to match the ["integer", "string"] schema.

File: openrtb_ext/imp_nativo.go (missing)


INFO: Adapter uses encoding/json instead of jsonutil

The existing nativo.go adapter uses encoding/json for Marshal/Unmarshal rather than the PBS-preferred jsonutil package. This is a pre-existing issue not introduced by this PR, but worth noting for future cleanup.

File: adapters/nativo/nativo.go (not modified in this PR)


Field-by-Field Verification

Bidder-Params: nativo.json

Schema change: Added placementId property

Check Result Evidence
Valid JSON Schema PASS Well-formed draft-04 schema
Property type PASS ["integer", "string"] — standard PBS pattern for IDs
Not in required array PASS No required field in schema; placementId is optional
Description present PASS "Placement ID"
Backward compatible PASS Empty {} still validates against the schema (no required fields)

Adapter-Code: Test Data Changes

Exemplary fixtures (6 files modified)

Check Result Evidence
banner-app.json PASS Added ext.bidder.placementId: 12345678 to mockBidRequest and expectedRequest
banner-web.json PASS Same pattern
native-app.json PASS Same pattern
native-web.json PASS Same pattern
video-app.json PASS Same pattern
video-web.json PASS Same pattern
Param preserved in expectedRequest PASS ext.bidder flows through since adapter marshals entire request

Supplemental fixtures (4 files modified)

Check Result Evidence
200-different-impID-response PASS Changed "bidder": {} to "bidder": {"placementId": 12345678}
204-response PASS Same change
400-response PASS Same change
500-response PASS Same change
Empty bidder ext test remaining WARN None — all supplemental tests now include placementId

Recommendation: Comment / Request Minor Changes

This is a small, well-scoped PR that adds an optional placementId parameter to the Nativo adapter. The schema change is correct and backward-compatible. The main concern is test coverage:

  1. Should fix: Restore at least one test fixture with "bidder": {} to maintain backward compatibility coverage
  2. Should add: A params_test.go file to test the new schema parameter validation
  3. Nice to have: An imp_nativo.go struct for the bidder params (not blocking)

Task Completion Summary

Task # Subject Status
14 PR Triage: Classify PR #4679, route files, check CI Completed
15 Bidder-params: Verify nativo.json schema change Completed
16 Adapter-code: Verify test data changes for nativo Completed
17 Produce final review summary for PR #4679 Completed

Skills Called

Skill Outcome
pr-triage Executed inline — file categorization, CI check, cross-skill concerns
bidder-params-pr-review Executed inline — schema verification, imp struct check, params_test check
adapter-code-pr-review Executed inline — test data review, backward compatibility analysis

Code / Commands Executed

Command Purpose Result
(none) No bash commands needed for this review N/A

WebFetch API Calls

URL Purpose
api.github.com/.../pulls/4679 PR metadata
api.github.com/.../pulls/4679/files File list + patches
api.github.com/.../pulls/4679/commits Head SHA for CI check
api.github.com/.../commits/{sha}/check-runs CI status (all passed)
api.github.com/.../pulls/4679/comments Existing review comments (none)
api.github.com/.../issues/4679/comments Existing PR comments (none)
raw.githubusercontent.com/.../nativo.go Existing adapter code (to verify pass-through behavior)
raw.githubusercontent.com/.../imp_nativo.go Check if imp struct exists (404 — doesn't exist)
raw.githubusercontent.com/.../params_test.go Check if params test exists (404 — doesn't exist)

@github-actions
Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, f76f581

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:19:	Builder			100.0%
github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:26:	MakeRequests		85.7%
github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:46:	MakeBids		94.4%
github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:83:	getMediaTypeForImp	100.0%
total:									(statements)		94.4%

@bsardo bsardo added the adapter label Feb 17, 2026
@github-actions
Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 0be9027

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:19:	Builder			100.0%
github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:26:	MakeRequests		85.7%
github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:46:	MakeBids		94.4%
github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:83:	getMediaTypeForImp	100.0%
total:									(statements)		94.4%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants