Skip to content

fix(api, ETAC): Update Test Setup for BED-7401#2416

Open
mvlipka wants to merge 2 commits intomainfrom
BED-7401/ETAC-Client-Filtering
Open

fix(api, ETAC): Update Test Setup for BED-7401#2416
mvlipka wants to merge 2 commits intomainfrom
BED-7401/ETAC-Client-Filtering

Conversation

@mvlipka
Copy link
Contributor

@mvlipka mvlipka commented Feb 25, 2026

Description

This updates test case setup in order for the related BHE PR to pass tests

Motivation and Context

Resolves: BED-7401

How Has This Been Tested?

NO-OP as this just changes unit tests

Screenshots (optional):

Types of changes

  • Chore (a change that does not modify the application functionality)

Checklist:

Summary by CodeRabbit

  • Tests
    • Enhanced authentication context setup in test cases to improve test coverage and reliability for authentication scenarios.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

This PR adds authenticated context injection to three test cases in the API v2 test suite. It imports authentication and context utilities, then modifies the SortingError, ColumnNotFilterable, and InvalidFilterPredicate test scenarios to construct and apply an authenticated context with an empty user before executing test assertions.

Changes

Cohort / File(s) Summary
Test Context Injection
cmd/api/src/api/v2/apitest/case.go
Added imports for auth, context (bhectx), and model packages. Modified three test cases to inject an authenticated bhctx.Context with an empty model.User via SetContext() prior to test execution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

api, bug

Suggested reviewers

  • stephanieslamb
  • superlinkx
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description covers most required sections but lacks detail in key areas. The Description section is minimal, and while it mentions 'test case setup', it doesn't explain what changes were made or why they're necessary for BED-7401. Enhance the Description section to explain what specific test case changes were made and how they support the BED-7401 API changes. Provide more context about the 'related BHE PR' and the specific test scenarios affected (SortingError, ColumnNotFilterable, InvalidFilterPredicate).
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: updating test setup for a specific issue (BED-7401), which matches the raw summary showing test case modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch BED-7401/ETAC-Client-Filtering

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 and usage tips.

@mvlipka mvlipka changed the title BED-7401 add ETAC to GET clients endpoints fix(api, ETAC): Update Test Setup for BED-7401 Feb 25, 2026
@mvlipka mvlipka marked this pull request as ready for review February 25, 2026 17:28
@coderabbitai coderabbitai bot added api A pull request containing changes affecting the API code. bug Something isn't working labels Feb 25, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
cmd/api/src/api/v2/apitest/case.go (1)

41-47: Consolidate duplicated auth-context setup and align local variable style.

The same context-construction block is repeated in three cases. Please extract a helper and use a richer local name with a hoisted var (...) block.

♻️ Proposed refactor
+func applyEmptyOwnerAuthContext(input *Input) {
+	var (
+		requestContext = bhectx.Context{
+			AuthCtx: auth.Context{
+				Owner: model.User{},
+			},
+		}
+	)
+
+	SetContext(input, requestContext.ConstructGoContext())
+}
+
 func NewSortingErrorCase() Case {
 	return Case{
 		Name: "SortingError",
 		Input: func(input *Input) {
 			AddQueryParam(input, "sort_by", "definitelyInvalidColumn")
-			bhctx := bhectx.Context{
-				AuthCtx: auth.Context{
-					Owner: model.User{},
-				},
-			}
-			SetContext(input, bhctx.ConstructGoContext())
+			applyEmptyOwnerAuthContext(input)
 		},
@@
 func NewColumnNotFilterableCase() Case {
 	return Case{
 		Name: "ColumnNotFilterable",
 		Input: func(input *Input) {
 			AddQueryParam(input, "definitelyInvalidColumn", "gt:0")
-
-			bhctx := bhectx.Context{
-				AuthCtx: auth.Context{
-					Owner: model.User{},
-				},
-			}
-			SetContext(input, bhctx.ConstructGoContext())
+			applyEmptyOwnerAuthContext(input)
 		},
@@
 func NewInvalidFilterPredicateCase(validColumn string) Case {
 	return Case{
 		Name: "InvalidFilterPredicate",
 		Input: func(input *Input) {
 			AddQueryParam(input, validColumn, "definitelyInvalidPredicate:0")
-			bhctx := bhectx.Context{
-				AuthCtx: auth.Context{
-					Owner: model.User{},
-				},
-			}
-
-			SetContext(input, bhctx.ConstructGoContext())
+			applyEmptyOwnerAuthContext(input)
 		},

As per coding guidelines: “Group variable initializations in a var ( ... ) block and hoist them to the top of the function when possible” and “Prefer rich variable names, for example: databaseInterface instead of di or dbi.”

Also applies to: 62-68, 82-89

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/api/src/api/v2/apitest/case.go` around lines 41 - 47, Extract the
repeated auth-context construction into a single helper (e.g.,
newBaseAuthContext or buildAuthGoContext) and replace the three inline blocks
with calls to that helper; hoist any local variables into a var (...) block at
the top of the function and use a descriptive name such as baseAuthContext for
the bhctx.Context instance, then call SetContext(input,
baseAuthContext.ConstructGoContext()) where the current inline
bhctx/auth.Context construction appears (locations referencing bhctx.Context,
auth.Context, SetContext, and ConstructGoContext).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cmd/api/src/api/v2/apitest/case.go`:
- Around line 41-47: Extract the repeated auth-context construction into a
single helper (e.g., newBaseAuthContext or buildAuthGoContext) and replace the
three inline blocks with calls to that helper; hoist any local variables into a
var (...) block at the top of the function and use a descriptive name such as
baseAuthContext for the bhctx.Context instance, then call SetContext(input,
baseAuthContext.ConstructGoContext()) where the current inline
bhctx/auth.Context construction appears (locations referencing bhctx.Context,
auth.Context, SetContext, and ConstructGoContext).

ℹ️ Review info

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e80426f and 97fc93d.

📒 Files selected for processing (1)
  • cmd/api/src/api/v2/apitest/case.go

@mvlipka mvlipka self-assigned this Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api A pull request containing changes affecting the API code. bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant