Test/delegate all

[Nyeng]

bør kanskje ikke merges før onsdag, siden testene kjører schedulert mot tt02

Summary by CodeRabbit

• New Features

  • Added a one-click "add all customers" action in the delegation UI.

• Bug Fixes

  • Removed extraneous debug logging from delegation API calls to reduce noise.

• Tests

  • Restructured and expanded delegation end-to-end tests into clearer, multi-step flows.
  • Adjusted cleanup strategy: some tests now perform local post-test cleanup while a global automatic cleanup hook was removed.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR removes debug console logs from the delegation API, moves per-test cleanup into a specific E2E spec (and removes a global fixture cleanup), restructures and consolidates client-delegation E2E flows, and adds bulk-customer UI actions to the ClientDelegation page object.

Changes

Cohort / File(s)	Summary
Delegation API logs playwright/api-requests/delegation-tilgangspakke/delegationApiRequest.ts	Removed three console.log debug statements from addOrgForDelegation, delegateAccessPkg, and cleanupDelegations. No control-flow or error-handling changes.
Per-test cleanup added to spec playwright/e2eTests/accessPackageDelegation/accessPackageDirectdelegatation.spec.ts	Added test.afterEach that calls DelegationApiUtil.cleanupAllDelegations(title) wrapped with withTimeout(..., 15000, ...); errors/timeouts are caught and logged as warnings (do not fail tests).
Global fixture cleanup removed playwright/fixture/pomFixture.ts	Removed imports withTimeout and DelegationApiUtil and the global afterEach cleanup hook; cleanup responsibility moved to suites.
Client delegation E2E refactor playwright/e2eTests/systemuser/clientDelegation.spec.ts	Restructured tests: separated describe blocks per facilitator role, consolidated flows into shared beforeEach setup, moved setup into API/system creation steps, and converted many individual tests into multi-step role-specific sequences including explicit navigation and cleanup.
Page object: bulk customer actions playwright/pages/systemuser/ClientDelegation.ts	Added locators addAllCustomersButton, addAllCustomersSuccessText; renamed openAccessPackage → openSystemUser; added addAllCustomers() method that opens the customers modal, triggers "add all", and verifies success/confirm visibility.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

✨ Logs hushed, flows rearranged,
Tests split into roles and stage,
One click adds all customers in view,
Cleanup now belongs where tests ensue,
A small refactor, tidy and new.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is vague and generic, using abbreviated terms ('Test/delegate all') that lack clarity about the actual changes or primary objective.	Consider a more descriptive title such as 'Add cleanup and multi-step delegation tests' or 'Refactor delegation tests with cleanup hooks' to better communicate the key changes.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches

• 📝 Generate docstrings

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@playwright/e2eTests/accessPackageDelegation/accessPackageDirectdelegatation.spec.ts`:
- Line 6: Replace the empty destructuring in the test.afterEach callback to
satisfy the linter: change the async parameter list used in
accessPackageDirectdelegatation.spec.ts's test.afterEach to either omit fixtures
or use a placeholder for the fixtures object (e.g., use "_" for the first
parameter) so the signature becomes async (_, testInfo) => { ... } (or remove
the first parameter entirely if fixtures are not required) while keeping the use
of testInfo intact.

🧹 Nitpick comments (3)

playwright/pages/systemuser/ClientDelegation.ts (1)

98-107: addAllCustomers() doesn't close the modal—intentional?

This method verifies confirmAndCloseButton is visible but doesn't click it. Looking at the test usage (line 62-63 in the spec), the caller explicitly clicks confirmAndCloseButton afterward. This differs from addCustomer() (lines 94-95) which closes the modal internally.

Consider either:

1. Closing the modal here for consistency with addCustomer()
2. Adding a comment explaining why the caller is responsible for closing

Option 1: Close modal for consistency

   async addAllCustomers() {
     await expect(this.customersButton).toBeVisible();
     await this.customersButton.click();
 
     await expect(this.addAllCustomersButton).toBeVisible();
     await this.addAllCustomersButton.click();
 
     await expect(this.addAllCustomersSuccessText).toBeVisible();
     await expect(this.confirmAndCloseButton).toBeVisible();
+    await this.confirmAndCloseButton.click();
   }

playwright/e2eTests/systemuser/clientDelegation.spec.ts (2)

66-69: Cleanup as a test step won't run if earlier steps fail.

If any step fails before reaching the cleanup step, the system user remains in the environment. Since these tests run scheduled against tt02 (per PR description), orphaned test data could accumulate.

Consider moving cleanup to test.afterEach with error handling similar to accessPackageDirectdelegatation.spec.ts:

test.afterEach(async () => {
  try {
    await clientDelegationPage.deleteSystemUser(name);
  } catch (err) {
    console.warn(`[afterEach] cleanup failed for ${name}`, err);
  }
});

---

16-70: Significant duplication across role test blocks.

The three test.describe blocks share nearly identical structure—only the role constants and delegation method (addAllCustomers vs addCustomer loop) differ. This could be refactored into a parameterized test or shared helper.

If reducing duplication isn't a priority now, this is fine to defer.

Example: Parameterized approach

const roleConfigs = [
  { role: FacilitatorRole.Revisor, apiName: 'ansvarlig-revisor', displayName: 'Ansvarlig revisor', delegateAll: true },
  { role: FacilitatorRole.Regnskapsfoerer, apiName: 'regnskapsforer-lonn', displayName: 'Regnskapsfører lønn', delegateAll: false },
  { role: FacilitatorRole.Forretningsfoerer, apiName: 'forretningsforer-eiendom', displayName: 'Forretningsforer eiendom', delegateAll: false },
];

for (const config of roleConfigs) {
  test.describe(config.displayName, () => {
    // shared setup and test logic using config
  });
}

Also applies to: 72-134, 136-198