Skip to content

Refactored Portal offer helper in /e2e/#26991

Merged
9larsons merged 2 commits intomainfrom
rf-portal-helper
Mar 26, 2026
Merged

Refactored Portal offer helper in /e2e/#26991
9larsons merged 2 commits intomainfrom
rf-portal-helper

Conversation

@9larsons
Copy link
Copy Markdown
Contributor

no ref

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f2d2d44f-6fab-4965-bb53-686422ab8403

📥 Commits

Reviewing files that changed from the base of the PR and between ff8beb7 and 0ffa9f2.

📒 Files selected for processing (3)
  • e2e/data-factory/factories/offer-factory.ts
  • e2e/helpers/playwright/flows/offers.ts
  • e2e/tests/public/portal-offers.test.ts
✅ Files skipped from review due to trivial changes (1)
  • e2e/data-factory/factories/offer-factory.ts

Walkthrough

This pull request modifies offer creation patterns across test infrastructure files. The offer factory module's imports are consolidated to import generateId and generateSlug alongside Factory from a single source. A new createPortalSignupOffer helper function and PortalSignupOfferInput type are introduced in the Playwright flows module. The portal-offers test file is updated to use the new helper function instead of maintaining its own implementation.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description provides minimal information ('no ref'), which is vague and does not meaningfully convey what changes were made or why. Provide a brief description of what was refactored and why (e.g., consolidating offer creation logic, extracting shared helpers, or improving test maintainability).
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: refactoring of the Portal offer helper in the /e2e/ directory, which aligns with the changeset's consolidation of offer creation logic.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rf-portal-helper

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.

Copy link
Copy Markdown
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 (2)
e2e/data-factory/factories/offer-factory.ts (1)

1-4: Consolidate duplicate imports from @/data-factory.

SonarCloud correctly flags that @/data-factory is imported multiple times. Merge into a single import statement for cleaner code.

♻️ Proposed fix
-import {Factory} from '@/data-factory';
-import {faker} from '@faker-js/faker';
-import {generateId, generateSlug} from '@/data-factory';
-import type {HttpClient, PersistenceAdapter} from '@/data-factory';
+import {Factory, generateId, generateSlug} from '@/data-factory';
+import type {HttpClient, PersistenceAdapter} from '@/data-factory';
+import {faker} from '@faker-js/faker';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/data-factory/factories/offer-factory.ts` around lines 1 - 4, Merge the
duplicate imports from '@/data-factory' into a single import: combine Factory,
generateId, generateSlug and the type imports HttpClient and PersistenceAdapter
into one import declaration (keeping the separate import for faker unchanged);
update the import line that currently references Factory and the separate line
for generateId/generateSlug and the type import so all symbols come from a
single '@/data-factory' import.
e2e/tests/public/portal-offers.test.ts (1)

116-116: Consider removing unnecessary non-null assertions on stripe.

SonarCloud flags these stripe! assertions as unnecessary. Since test.use({stripeEnabled: true}) guarantees stripe is defined within this describe block, the non-null assertions are redundant. This is a minor cleanup.

♻️ Example fix for one occurrence
-        const offer = await createPortalSignupOffer(page.request, stripe!, {
+        const offer = await createPortalSignupOffer(page.request, stripe, {

Also applies to: 139-139, 167-167, 197-197

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

In `@e2e/tests/public/portal-offers.test.ts` at line 116, Remove the redundant
non-null assertions on the `stripe` variable (the `!` after `stripe`) in the
test calls — e.g., the call to `createPortalSignupOffer(page.request, stripe!,
{...})` and the other occurrences flagged — because `test.use({stripeEnabled:
true})` already guarantees `stripe` is defined; update each call site to pass
`stripe` without `!` (search for `stripe!` in this test file and remove the `!`
in calls like `createPortalSignupOffer` and the other create-portal offer helper
invocations).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@e2e/data-factory/factories/offer-factory.ts`:
- Around line 1-4: Merge the duplicate imports from '@/data-factory' into a
single import: combine Factory, generateId, generateSlug and the type imports
HttpClient and PersistenceAdapter into one import declaration (keeping the
separate import for faker unchanged); update the import line that currently
references Factory and the separate line for generateId/generateSlug and the
type import so all symbols come from a single '@/data-factory' import.

In `@e2e/tests/public/portal-offers.test.ts`:
- Line 116: Remove the redundant non-null assertions on the `stripe` variable
(the `!` after `stripe`) in the test calls — e.g., the call to
`createPortalSignupOffer(page.request, stripe!, {...})` and the other
occurrences flagged — because `test.use({stripeEnabled: true})` already
guarantees `stripe` is defined; update each call site to pass `stripe` without
`!` (search for `stripe!` in this test file and remove the `!` in calls like
`createPortalSignupOffer` and the other create-portal offer helper invocations).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4e5b6609-544b-4245-989f-e83c35ec250b

📥 Commits

Reviewing files that changed from the base of the PR and between 25e1433 and ff8beb7.

📒 Files selected for processing (8)
  • e2e/data-factory/factories/offer-factory.ts
  • e2e/data-factory/index.ts
  • e2e/data-factory/setup.ts
  • e2e/helpers/playwright/flows/offers.ts
  • e2e/helpers/services/offers/index.ts
  • e2e/helpers/services/offers/offers-service.ts
  • e2e/tests/public/portal-offers.test.ts
  • e2e/tests/public/stripe-offer-checkout.test.ts
💤 Files with no reviewable changes (2)
  • e2e/helpers/services/offers/index.ts
  • e2e/helpers/services/offers/offers-service.ts

ref #26991
Kept the rebased PR free of the Sonar duplicate-import warning without changing behavior.
@9larsons 9larsons enabled auto-merge (squash) March 26, 2026 16:44
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
4.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@9larsons 9larsons merged commit a3494d4 into main Mar 26, 2026
33 of 34 checks passed
@9larsons 9larsons deleted the rf-portal-helper branch March 26, 2026 17:06
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