Merge pull request #531 from appuio/edit-be

Edit Billing

cypress/e2e/billingentities.cy.ts

@@ -92,6 +92,15 @@ describe('no permissions', () => {
     cy.get('h1').should('contain.text', 'Billing');
     cy.get('addButton').should('not.exist');
   });
+
+  it('no edit permission', () => {
+    setBillingEntities(cy, billingEntityNxt);
+    cy.setPermission({ verb: 'list', ...BillingEntityPermissions });
+    cy.visit('/billingentities');
+    cy.get('h1').should('contain.text', 'Billing');
+    cy.get('svg[class*="fa-pen-to-square"]').should('not.exist');
+    cy.get('svg[class*="fa-magnifying-glass"]').should('exist');
+  });
 });
 
 describe('Test billing entity details', () => {

cypress/e2e/billingentity-form.cy.ts

@@ -125,6 +125,41 @@ describe('Test billing entity form elements', () => {
       .should('not.contain.text', 'accounting@company');
   });
 
+  it('should prefill existing values', () => {
+    cy.intercept('GET', '/appuio-api/apis/billing.appuio.io/v1/billingentities/be-2345', {
+      body: billingEntityNxt,
+    });
+
+    cy.visit('/billingentities/be-2345?edit=y');
+
+    cy.get('#displayName').should('have.value', '➡️ Engineering GmbH');
+    cy.get('#companyEmail').should('contain.text', '[email protected]');
+    cy.get('#phone').should('have.value', '☎️');
+    cy.get('#line1').should('have.value', '📃');
+    cy.get('#line2').should('have.value', '📋');
+    cy.get('#postal').should('have.value', '🏤');
+    cy.get('#city').should('have.value', '🏙️');
+    cy.get('p-dropdown').should('contain.text', 'Switzerland');
+    cy.get('#accountingName').should('have.value', 'mig');
+    cy.get('#accountingEmail').should('contain.text', '[email protected]');
+
+    cy.get('#companyEmail').find('input').type('{backspace}[email protected]{enter}');
+    cy.get('#accountingEmail')
+      .should('contain.text', '[email protected]')
+      .should('not.contain.text', '[email protected]');
+    cy.get('.p-checkbox').should('have.class', 'p-checkbox-checked').click();
+    cy.get('#accountingEmail')
+      .should('not.contain.text', '[email protected]')
+      .should('contain.text', '[email protected]');
+    cy.get('#accountingEmail').find('input').type('{backspace}[email protected]{enter}');
+
+    cy.get('.p-checkbox').click();
+    cy.get('#accountingEmail')
+      .should('not.contain.text', '[email protected]')
+      .should('contain.text', '[email protected]');
+    cy.get('button[type="submit"]').should('be.enabled');
+  });
+
   it('should cancel editing', () => {
     setBillingEntities(cy);
 
@@ -154,7 +189,7 @@ describe('Test billing entity create', () => {
     cy.setPermission({ verb: 'list', ...BillingEntityPermissions }, { verb: 'create', ...BillingEntityPermissions });
   });
 
-  it('should submit form values', () => {
+  it('should create billing', () => {
     cy.intercept('POST', '/appuio-api/apis/billing.appuio.io/v1/billingentities', {
       body: billingEntityNxt,
     }).as('createBillingEntity');
@@ -202,7 +237,108 @@ describe('Test billing entity create', () => {
       });
 
     cy.get('p-toast').should('contain.text', 'Successfully saved');
+    cy.url().should('include', '/billingentities/be-2345').should('not.include', '?edit=y');
+    cy.get('.flex-wrap > .text-900').eq(0).should('contain.text', '➡️ Engineering GmbH');
+
+    cy.get('#title').should('contain.text', 'be-2345');
+    cy.get('.flex-wrap > .text-900').eq(1).should('contain.text', '[email protected]');
+    cy.get('.flex-wrap > .text-900').eq(2).should('contain.text', '☎️');
+    cy.get('.flex-wrap > .text-900').eq(3).should('contain.text', '📃📋🏤 🏙️Switzerland');
+    cy.get('.flex-wrap > .text-900').eq(4).should('contain.text', 'mig [email protected]');
+    cy.get('.flex-wrap > .text-900').eq(5).should('contain.text', '🇩🇪');
+  });
+});
+
+describe('Test billing entity edit', () => {
+  beforeEach(() => {
+    cy.setupAuth();
+    window.localStorage.setItem('hideFirstTimeLoginDialog', 'true');
+    cy.disableCookieBanner();
+  });
+  beforeEach(() => {
+    // needed for initial getUser request
+    cy.intercept('GET', 'appuio-api/apis/appuio.io/v1/users/mig', {
+      body: createUser({ username: 'mig', defaultOrganizationRef: 'nxt' }),
+    });
+    cy.setPermission(
+      { verb: 'list', ...BillingEntityPermissions },
+      { verb: 'create', ...BillingEntityPermissions },
+      { verb: 'update', ...BillingEntityPermissions, name: 'be-2345' }
+    );
+  });
+
+  it('should update billing', () => {
+    cy.intercept('GET', '/appuio-api/apis/billing.appuio.io/v1/billingentities/be-2345', {
+      body: billingEntityNxt,
+    });
+
+    cy.intercept('PATCH', '/appuio-api/apis/billing.appuio.io/v1/billingentities/be-2345', (req) => {
+      req.reply(req.body);
+    }).as('updateBillingEntity');
+
+    cy.visit('/billingentities/be-2345');
+    cy.get('#title').should('contain.text', 'be-2345');
+
+    cy.get('svg[class*="fa-pen-to-square"]').click();
+
+    cy.get('#displayName').should('have.value', '➡️ Engineering GmbH');
+
+    cy.get('#displayName').type('{selectAll}nxt Engineering');
+    cy.get('#companyEmail').type('[email protected]{enter}');
+    cy.get('#phone').type('{selectAll}1234');
+    cy.get('#line1').type('{selectAll}line1');
+    cy.get('#line2').type('{selectAll}{backspace}').should('have.value', '');
+    cy.get('#postal').type('{selectAll}4321');
+    cy.get('#city').type('{selectAll}Berlin');
+    cy.get('p-dropdown').click().contains('Germany').click();
+
+    cy.get('#accountingName').type('{selectAll}{backspace}crc');
+    cy.get('.p-checkbox').click();
+
+    cy.get('button[type="submit"]').should('be.enabled').click();
+    cy.wait('@updateBillingEntity')
+      .its('request.body')
+      .then((body: BillingEntity) => {
+        expect(body.metadata.name).eq('be-2345');
+        const expected: BillingEntitySpec = {
+          name: 'nxt Engineering',
+          phone: '1234',
+          emails: ['[email protected]', '[email protected]'],
+          address: {
+            line1: 'line1',
+            line2: '',
+            postalCode: '4321',
+            city: 'Berlin',
+            country: 'Germany',
+          },
+          accountingContact: {
+            name: 'crc',
+            emails: ['[email protected]'],
+          },
+          // even we don't use this field yet, ensure it gets sent back.
+          languagePreference: '🇩🇪',
+        };
+        console.debug('expected', expected);
+        console.debug('actual', body.spec);
+        expect(body.spec).deep.eq(expected);
+      });
+
+    cy.get('p-toast').should('contain.text', 'Successfully saved');
+    cy.url().should('include', '/billingentities/be-2345').should('not.include', '?edit=y');
+    // check values
     cy.get('#title').should('contain.text', 'be-2345');
-    cy.url().should('include', '/billingentities/be-2345');
+    cy.get('.flex-wrap > .text-900').eq(0).should('contain.text', 'nxt Engineering');
+    cy.get('.flex-wrap > .text-900')
+      .eq(1)
+      .should('contain.text', '[email protected]')
+      .should('contain.text', '[email protected]');
+    cy.get('.flex-wrap > .text-900').eq(2).should('contain.text', '1234');
+    cy.get('.flex-wrap > .text-900')
+      .eq(3)
+      .should('contain.text', 'line1')
+      .should('contain.text', '4321 Berlin')
+      .should('contain.text', 'Germany');
+    cy.get('.flex-wrap > .text-900').eq(4).should('contain.text', 'crc [email protected]');
+    cy.get('.flex-wrap > .text-900').eq(5).should('contain.text', '🇩🇪');
   });
 });

src/app/billingentity/billing-entity.component.html

@@ -38,6 +38,7 @@ <h1 class="pt-0 mt-0 mb-0" i18n id="billingentities-title">Billing</h1>
               <fa-icon [icon]="faDetails" />
             </a>
             <a
+              *ngIf="p.canEdit"
               [routerLink]="[p.billingEntity.metadata.name]"
               [queryParams]="{edit: 'y'}"
               class="text-blue-500 hover:text-primary cursor-pointer ml-3"

src/app/billingentity/billing-entity.component.ts

@@ -42,11 +42,13 @@ export class BillingEntityComponent implements OnInit {
             of(be),
             // enrich the BE with information about permissions.
             this.billingEntityService.canEditMembers(`billingentities-${be.metadata.name}-admin`),
+            this.billingEntityService.canEditBilling(be.metadata.name),
           ]).pipe(
-            map(([billingEntity, canViewMembers]) => {
+            map(([billingEntity, canViewMembers, canEdit]) => {
               return {
                 billingEntity,
                 canViewMembers,
+                canEdit,
               } satisfies BillingModel;
             })
           );
@@ -77,5 +79,6 @@ interface ViewModel {
 
 interface BillingModel {
   billingEntity: BillingEntity;
+  canEdit: boolean;
   canViewMembers: boolean;
 }

src/app/billingentity/billingentity-detail/billing-entity-detail.component.ts

@@ -1,7 +1,7 @@
 import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core';
-import { ActivatedRoute } from '@angular/router';
+import { ActivatedRoute, Router } from '@angular/router';
 import { BillingEntity } from '../../types/billing-entity';
-import { forkJoin, map, Observable, of, switchMap } from 'rxjs';
+import { combineLatestWith, filter, forkJoin, map, Observable, of, switchMap } from 'rxjs';
 import { faCancel, faClose, faEdit, faWarning } from '@fortawesome/free-solid-svg-icons';
 import { BillingEntityCollectionService } from '../../store/billingentity-collection.service';
 
@@ -21,21 +21,25 @@ export class BillingEntityDetailComponent implements OnInit {
   faCancel = faCancel;
   faClose = faClose;
 
-  constructor(private route: ActivatedRoute, private billingService: BillingEntityCollectionService) {}
+  constructor(
+    private route: ActivatedRoute,
+    private router: Router,
+    private billingService: BillingEntityCollectionService
+  ) {}
 
   ngOnInit(): void {
     this.isEditing$ = this.route.queryParamMap.pipe(map((queryParams) => queryParams.get('edit') === 'y'));
     this.viewModel$ = this.route.paramMap.pipe(
-      switchMap((params) => {
-        const name = params.get('name');
-        if (!name) {
-          throw new Error('name is required');
-        }
+      map((params) => params.get('name') as string),
+      filter((name) => name !== null),
+      switchMap((name) => {
         this.billingEntityName = name;
         if (name === '$new') {
           return forkJoin([of(this.billingService.newBillingEntity()), of(true)]);
         }
-        return forkJoin([this.billingService.getByKeyMemoized(name), this.billingService.canEditBilling(name)]);
+        return this.billingService
+          .streamByKeyMemoized(name)
+          .pipe(combineLatestWith(this.billingService.canEditBilling(name)));
       }),
       map(([billingEntity, canEdit]) => {
         return {

src/app/billingentity/billingentity-form/billing-entity-form.component.html

@@ -108,15 +108,19 @@
       </small>
     </div>
     <button
-      [disabled]="form.invalid"
+      [disabled]="form.invalid || form.pristine"
       [loading]="(billingService.loading$ | ngrxPush) ?? false"
       class="w-auto mr-3 p-button-primary"
       pButton pRipple type="submit"
     >
       <fa-icon [icon]="faSave" class="pr-2"/>
       <span class="pr-2" i18n>Save</span>
     </button>
-    <button class="w-auto mr-3 p-button-secondary p-button-outlined" pButton pRipple type="button" (click)="cancel()">
+    <button
+      class="w-auto mr-3 p-button-secondary p-button-outlined"
+      pButton pRipple type="button"
+      (click)="cancel()"
+    >
       <fa-icon [icon]="faCancel" class="pr-2"/>
       <span class="pr-2" i18n>Cancel</span>
     </button>

src/app/billingentity/billingentity-form/billing-entity-form.component.ts

@@ -40,6 +40,7 @@ export class BillingEntityFormComponent implements OnInit {
 
   ngOnInit(): void {
     this.countryOptions = this.appConfig.getConfiguration().countries;
+    this.billingEntity = structuredClone(this.billingEntity); // make fields writable if editing existing BE.
     const spec = this.billingEntity.spec;
     const companyEmails = spec.emails?.sort((a, b) => a.localeCompare(b, undefined, { sensitivity: 'base' })) ?? [];
     const accountingEmails =
@@ -103,7 +104,7 @@ export class BillingEntityFormComponent implements OnInit {
       return;
     }
     const controls = this.form.controls;
-    const be = structuredClone(this.billingEntity);
+    const be = this.billingEntity;
     be.spec = {
       ...be.spec,
       name: controls.displayName.value,
@@ -149,13 +150,17 @@ export class BillingEntityFormComponent implements OnInit {
   }
 
   private saveOrUpdateSuccess(be: BillingEntity): void {
+    this.billingService.resetMemoization();
     this.messageService.add({
       severity: 'success',
       summary: $localize`Successfully saved`,
     });
-    void this.router.navigate(['..', be.metadata.name], {
+    // TODO: navigating to previous location with fallback might not work correctly.
+    // But since the backend hasn't implemented creating/editing BE yet, it's hard to test.
+    const previous = this.navigationService.previousRoute(`../${be.metadata.name}`);
+    void this.router.navigate([previous.path], {
       relativeTo: this.activatedRoute,
-      queryParams: { edit: undefined },
+      queryParams: { ...previous.queryParams, edit: undefined },
       queryParamsHandling: 'merge',
     });
   }

src/app/billingentity/billingentity-members/billing-entity-members.component.ts

@@ -68,8 +68,8 @@ export class BillingEntityMembersComponent implements OnInit, OnDestroy {
     if (!beName) {
       throw new Error('name is required');
     }
-    const adminClusterRoleBindingName = `billingentities-${beName}-admin`;
-    const viewerClusterRoleBindingName = `billingentities-${beName}-viewer`;
+    const adminClusterRoleBindingName = this.getBindingName(beName, 'admin');
+    const viewerClusterRoleBindingName = this.getBindingName(beName, 'viewer');
 
     // How to read the pipe below:
     //  First, get a set of permissions.
@@ -260,6 +260,10 @@ export class BillingEntityMembersComponent implements OnInit, OnDestroy {
     this.userRefs.removeAt(index);
   }
 
+  getBindingName(beName: string, role: 'admin' | 'viewer'): string {
+    return `billingentities-${beName}-${role}`;
+  }
+
   ngOnDestroy(): void {
     this.subscriptions.forEach((sub) => sub.unsubscribe());
   }

src/app/billingentity/billingentity-view/billing-entity-view.component.html

@@ -7,8 +7,8 @@
     <li *ngIf="billingEntity.spec.emails" class="flex align-items-center py-2 px-2 flex-wrap surface-ground">
       <div class="text-500 w-full md:w-3 font-medium" i18n>Company Email</div>
       <div class="text-900 w-full md:w-9">
-        <ul *ngFor="let email of billingEntity.spec.emails" class="pl-0 list-none">
-          <li>{{ email }}</li>
+        <ul *ngIf="billingEntity.spec.emails" class="pl-0 list-none">
+          <li *ngFor="let email of billingEntity.spec.emails" >{{ email }}</li>
         </ul>
       </div>
     </li>
@@ -30,8 +30,8 @@
       <div class="text-500 w-full md:w-3 font-medium" i18n>Invoice Recipient</div>
       <div class="text-900 w-full md:w-9">
         {{ billingEntity.spec.accountingContact.name }}
-        <ul *ngFor="let email of billingEntity.spec.accountingContact.emails" class="pl-0 list-none">
-          <li>{{ email }}</li>
+        <ul *ngIf="billingEntity.spec.accountingContact.emails" class="pl-0 list-none">
+          <li *ngFor="let email of billingEntity.spec.accountingContact.emails">{{ email }}</li>
         </ul>
       </div>
     </li>