Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleting chef server message modal window #3478

Merged
merged 4 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ export interface Server {
description: string;
fqdn: string;
ip_address: string;
orgs_count?: string;
orgs_count?: number;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,28 @@
<chef-heading>Chef Servers</chef-heading>
<chef-subheading>Manage Chef Servers with Chef Automate.</chef-subheading>
</chef-page-header>
<app-create-chef-server-modal
[visible]="createModalVisible"
<app-create-chef-server-modal
[visible]="createModalVisible"
[creating]="creatingChefServer"
[createForm]="createChefServerForm"
(close)="closeCreateModal()"
[createForm]="createChefServerForm"
(close)="closeCreateModal()"
[conflictErrorEvent]="conflictErrorEvent"
(createClicked)="createChefServer()">
</app-create-chef-server-modal>
<app-delete-object-modal
[visible]="deleteModalVisible"
objectNoun="server"
<app-delete-object-modal
[visible]="deleteModalVisible"
objectNoun="server"
[objectName]="serverToDelete?.name"
(close)="closeDeleteModal()"
(deleteClicked)="deleteServer()"
(close)="closeDeleteModal()"
(deleteClicked)="deleteServer()"
objectAction="Delete">
</app-delete-object-modal>
<app-message-modal
[title]="'Could Not Delete Server'"
[visible]="messageModalVisible"
(close)="closeMessageModal()">
Before you can delete this server, delete all organizations attached to it.
</app-message-modal>
<div class="page-body">
Copy link
Contributor

@scottopherson scottopherson Apr 27, 2020

Choose a reason for hiding this comment

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

@chef/ui-team is there consensus on listing many attributes/prop bindings on their own line or only wrapping them at a certain line length?

<app-create-chef-server-modal                                                                 |
  [visible]="createModalVisible"                                                              |
  [creating]="creatingChefServer"                                                             |
  (close)="closeCreateModal()"                                                                |
   ...etc...>                                                                                 |
</app-create-chef-server-modal>                                                               |

vs

<app-create-chef-server-modal [visible]="createModalVisible" [creating]="creatingChefServer"  |
   (close)="closeCreateModal()" ...etc...>                                                    |
</app-create-chef-server-modal>                                                               |

I've usually preferred to keep longer lists of attributes/prop bindings on their own line but don't mind either really. By default vscode wraps them by line length whenever you copy/paste html blocks

Copy link
Contributor

Choose a reason for hiding this comment

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

I used to be indifferent, but lately I am leaning towards one-per-line; it greatly improves readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed as one-per-line πŸ‘

Copy link
Contributor

Choose a reason for hiding this comment

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

@msorens yea same. Better readability in file and in git diffs. Thx @sachin-msys πŸ‘

<chef-toolbar>
<chef-button primary (click)="openCreateModal()">Add Chef Server</chef-button>
Expand All @@ -37,17 +43,18 @@
</chef-tr>
</chef-thead>
<chef-tbody>
<chef-tr *ngFor= "let server of sortedChefServers$ | async">
<chef-td >
<chef-tr *ngFor="let server of sortedChefServers$ | async">
<chef-td>
<a [routerLink]="['/infrastructure/chef-servers', server.id]">{{ server.name }}</a>
</chef-td>
<chef-td >{{ server.fqdn }}</chef-td>
<chef-td >{{ server.ip_address }}</chef-td>
<chef-td >{{ server.description }}</chef-td>
<chef-td >{{ server.orgs_count }}</chef-td>
<chef-td>{{ server.fqdn }}</chef-td>
<chef-td>{{ server.ip_address }}</chef-td>
<chef-td>{{ server.description }}</chef-td>
<chef-td>{{ server.orgs_count }}</chef-td>
<chef-td class="three-dot-column">
<mat-select panelClass="chef-control-menu" id="menu-{{server.id}}">
<mat-option (onSelectionChange)="startServerDelete($event, server)" data-cy="remove-server">Remove Chef Server</mat-option>
<mat-option (onSelectionChange)="startServerDelete($event, server)" data-cy="remove-server">Remove
Chef Server</mat-option>
</mat-select>
</chef-td>
</chef-tr>
Expand All @@ -56,4 +63,4 @@
</div>
</main>
</div>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { async, ComponentFixture, TestBed } from '@angular/core/testing';
import { HttpErrorResponse } from '@angular/common/http';
import { RouterTestingModule } from '@angular/router/testing';
import { FormsModule, ReactiveFormsModule } from '@angular/forms';
import { MatOptionSelectionChange } from '@angular/material/core/option';
import { MockComponent } from 'ng2-mock-component';
import { StoreModule, Store } from '@ngrx/store';

Expand All @@ -21,15 +22,19 @@ describe('ChefServersListComponent', () => {
TestBed.configureTestingModule({
declarations: [
MockComponent({
selector: 'app-create-chef-server-modal',
inputs: ['visible', 'creating', 'conflictErrorEvent', 'createForm'],
outputs: ['close', 'createClicked']
selector: 'app-create-chef-server-modal',
inputs: ['visible', 'creating', 'conflictErrorEvent', 'createForm'],
outputs: ['close', 'createClicked']
}),
MockComponent({
selector: 'app-delete-object-modal',
inputs: ['default', 'visible', 'objectNoun', 'objectName'],
outputs: ['close', 'deleteClicked']
}),
MockComponent({
selector: 'chef-button',
inputs: ['disabled', 'routerLink']
}),
MockComponent({ selector: 'app-delete-object-modal',
inputs: ['default', 'visible', 'objectNoun', 'objectName'],
outputs: ['close', 'deleteClicked'] }),
MockComponent({ selector: 'chef-button',
inputs: ['disabled', 'routerLink'] }),
MockComponent({ selector: 'chef-error' }),
MockComponent({ selector: 'chef-form-field' }),
MockComponent({ selector: 'chef-heading' }),
Expand Down Expand Up @@ -58,9 +63,9 @@ describe('ChefServersListComponent', () => {
RouterTestingModule,
StoreModule.forRoot(ngrxReducers, { runtimeChecks })
],
schemas: [ CUSTOM_ELEMENTS_SCHEMA ]
schemas: [CUSTOM_ELEMENTS_SCHEMA]
})
.compileComponents();
.compileComponents();
}));

beforeEach(() => {
Expand All @@ -75,13 +80,13 @@ describe('ChefServersListComponent', () => {

describe('create server', () => {
let store: Store<NgrxStateAtom>;
const server = <Server> {
id: '1',
name: 'new server',
description: 'new server description',
fqdn: 'xyz.com',
ip_address: '1.1.1.1'
};
const server = <Server>{
id: '1',
name: 'new server',
description: 'new server description',
fqdn: 'xyz.com',
ip_address: '1.1.1.1'
};

beforeEach(() => {
store = TestBed.inject(Store);
Expand Down Expand Up @@ -112,7 +117,7 @@ describe('ChefServersListComponent', () => {
component.createChefServerForm.controls['ip_address'].setValue(server.ip_address);
component.createChefServer();

store.dispatch(new CreateServerSuccess({'server': server}));
store.dispatch(new CreateServerSuccess({ 'server': server }));

component.sortedChefServers$.subscribe(servers => {
expect(servers).toContain(server);
Expand Down Expand Up @@ -208,5 +213,59 @@ describe('ChefServersListComponent', () => {

});
});

describe('delete modal', () => {
const mockEvent = { isUserInput: true } as MatOptionSelectionChange;

it('With no orgs, selecting delete from control menu opens the delete modal', () => {
expect(component.deleteModalVisible).toBe(false);
component.startServerDelete(mockEvent, genServer('uuid-111', 0));
expect(component.deleteModalVisible).toBe(true);
});

it('closes upon sending request to back-end', () => {
component.startServerDelete(mockEvent, genServer('uuid-111', 0));
expect(component.deleteModalVisible).toBe(true);
component.deleteServer();
expect(component.deleteModalVisible).toBe(false);
});

});

describe('message modal', () => {
const mockEvent = { isUserInput: true } as MatOptionSelectionChange;

it('With one org, selecting delete from control menu opens the message modal', () => {
expect(component.messageModalVisible).toBe(false);
component.startServerDelete(mockEvent, genServer('uuid-111', 1));
expect(component.messageModalVisible).toBe(true);
});

it('With multiple orgs, selecting delete from control menu opens the message modal', () => {
expect(component.messageModalVisible).toBe(false);
component.startServerDelete(mockEvent, genServer('uuid-111', 1));
expect(component.messageModalVisible).toBe(true);
});

it('closes upon request', () => {
component.startServerDelete(mockEvent, genServer('uuid-111', 4));
expect(component.messageModalVisible).toBe(true);
component.closeMessageModal();
expect(component.messageModalVisible).toBe(false);
});

});

function genServer(id: string, orgs_count: number): Server {
return {
id,
orgs_count,
name: 'Demo Server',
description: 'Demo Description',
fqdn: 'http://demo.com/',
ip_address: '192.168.2.1'
};
}

});

Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { Component, OnInit, OnDestroy, EventEmitter } from '@angular/core';
import { FormBuilder, FormGroup, Validators } from '@angular/forms';
import { MatOptionSelectionChange } from '@angular/material/core/option';
import { Store, select } from '@ngrx/store';
import { filter, takeUntil, map } from 'rxjs/operators';
import { Regex } from 'app/helpers/auth/regex';
import { Observable, Subject, combineLatest } from 'rxjs';
import { isNil } from 'lodash/fp';

import { HttpStatus } from 'app/types/types';
import { ChefKeyboardEvent } from 'app/types/material-types';
import { NgrxStateAtom } from 'app/ngrx.reducers';
import { LayoutFacadeService, Sidebar } from 'app/entities/layout/layout.facade';
import { loading, EntityStatus, pending } from 'app/entities/entities';
Expand All @@ -30,6 +31,7 @@ export class ChefServersListComponent implements OnInit, OnDestroy {
private isDestroyed = new Subject<boolean>();
public serverToDelete: Server;
public deleteModalVisible = false;
public messageModalVisible = false;

constructor(
private store: Store<NgrxStateAtom>,
Expand Down Expand Up @@ -120,10 +122,14 @@ export class ChefServersListComponent implements OnInit, OnDestroy {
this.conflictErrorEvent.emit(false);
}

public startServerDelete($event: ChefKeyboardEvent, server: Server): void {
public startServerDelete($event: MatOptionSelectionChange, server: Server): void {
if ($event.isUserInput) {
this.serverToDelete = server;
this.deleteModalVisible = true;
if (server.orgs_count > 0) {
this.messageModalVisible = true;
} else {
this.serverToDelete = server;
this.deleteModalVisible = true;
}
}
}

Expand All @@ -135,4 +141,8 @@ export class ChefServersListComponent implements OnInit, OnDestroy {
public closeDeleteModal(): void {
this.deleteModalVisible = false;
}

public closeMessageModal(): void {
this.messageModalVisible = false;
}
}