feat(vm-snapshots): implement vm snapshots details sidebar #1454

tamazlykar · 2018-11-30T09:46:23Z

Implements #1332

coveralls · 2018-11-30T09:53:15Z

Pull Request Test Coverage Report for Build 1426

80 of 136 (58.82%) changed or added relevant lines in 24 files are covered.
62 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.07%) to 53.665%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/app/account/account/account-item.component.ts	2	3	66.67%
src/app/snapshot/components/vm-snapshot-card-view/vm-snapshot-card-view.component.ts	5	6	83.33%
src/app/snapshot/components/vm-snapshot-list-view/vm-snapshot-list-view.component.ts	5	6	83.33%
src/app/snapshot/store/snapshot-page.reducer.ts	0	1	0.0%
src/app/template/template/template.component.ts	1	2	50.0%
src/app/vm/vm-list-item/vm-list-item.component.ts	2	3	66.67%
src/app/snapshot/containers/snapshots-page/snapshots-page.container.ts	1	3	33.33%
src/app/snapshot/snapshots-page/snapshot-list-item/snapshot-item.component.ts	1	3	33.33%
src/app/snapshot/store/snapshot-page.actions.ts	3	5	60.0%
src/app/shared/models/vm-snapshot.model.ts	2	5	40.0%

Files with Coverage Reduction	New Missed Lines	%
src/app/shared/components/day-period/day-period.component.ts	7	45.45%
src/app/vm-logs/redux/vm-logs.reducers.ts	11	71.76%
src/app/vm-logs/redux/vm-logs.actions.ts	44	64.8%

Totals
Change from base Build 1414:	0.07%
Covered Lines:	8505
Relevant Lines:	13966

💛 - Coveralls

wowshakhov · 2018-12-03T04:15:36Z

Consider fixing in this PR:

cloudstack-ui/src/app/snapshot/components/vm-snapshot-action-menu/vm-snapshot-action-menu.component.html

Line 1 in 1b59e0c

src/app/snapshot/components/vm-snapshot-card-view/vm-snapshot-card-view.component.ts

src/app/snapshot/components/vm-snapshot-list-view/vm-snapshot-list-view.component.ts

src/app/snapshot/components/vm-snapshot-sidebar/vm-snapshot-sidebar.component.html

src/app/snapshot/components/vm-snapshot-sidebar/vm-snapshot-sidebar.component.scss

wowshakhov · 2018-12-03T04:27:33Z

src/app/snapshot/containers/snapshots-page/snapshots-page.container.ts

@@ -25,7 +25,7 @@ import { snapshotPageSelectors, vmSnapshotsSelectors } from '../../store';
    ></cs-snapshots-page>`,
 })
 export class SnapshotsPageContainerComponent implements OnInit, AfterViewInit {
-  readonly snapshots$ = this.store.pipe(select(fromSnapshots.selectFilteredSnapshots));
+  readonly snapshots$ = this.store.pipe(select(snapshotPageSelectors.getFilteredSnapshots));


Why did you rename select -> get? I think we should keep "select", because it's the default prefix in NgRx Entity:
export const { selectIds, selectEntities, selectAll, selectTotal } = adapter.getSelectors(state);

src/app/snapshot/store/snapshot-page.selectors.ts

src/app/snapshot/snapshots-page/snapshot-sidebar/snapshot-sidebar.container.ts

src/app/snapshot/store/snapshot-page.selectors.ts

tamazlykar · 2018-12-03T06:49:47Z

Consider fixing in this PR:
cloudstack-ui/src/app/snapshot/components/vm-snapshot-action-menu/vm-snapshot-action-menu.component.html

Line 1 in 1b59e0c

<button mat-menu-item *ngIf="false" (click)="onCreateVolumeSnapshot()">

This function is not working on the server now, we enable it after it starts working.

wowshakhov · 2018-12-04T02:20:42Z

src/app/snapshot/components/vm-snapshot-card-view/vm-snapshot-card-view.component.ts

+  @Output()
+  public clicked = new EventEmitter<VmSnapshotViewModel>();
+  @ViewChild(MatMenuTrigger)
+  public matMenuTrigger: MatMenuTrigger;


Remove matMenuTrigger if it is not used

wowshakhov · 2018-12-04T02:20:55Z

src/app/snapshot/components/vm-snapshot-list-view/vm-snapshot-list-view.component.ts

+  @Output()
+  public clicked = new EventEmitter<VmSnapshotViewModel>();
+  @ViewChild(MatMenuTrigger)
+  public matMenuTrigger: MatMenuTrigger;


Remove matMenuTrigger if it is not used

wowshakhov · 2018-12-04T02:23:02Z

src/app/snapshot/snapshots-page/snapshot-list-item/snapshot-card-item.component.ts

  @Output()
-  public onClick = new EventEmitter<Snapshot>();
+  public clicked = new EventEmitter<Snapshot>();
  @ViewChild(MatMenuTrigger)
  public matMenuTrigger: MatMenuTrigger;


Remove matMenuTrigger if it is not used

wowshakhov · 2018-12-04T02:23:59Z

src/app/snapshot/snapshots-page/snapshot-list-item/snapshot-item.component.ts

@@ -11,7 +11,7 @@ export class SnapshotItemComponent {
  public virtualMachines: Dictionary<VirtualMachine>;
  public isSelected: (snapshot: Snapshot) => boolean;
  public query: string;
-  public onClick = new EventEmitter<Snapshot>();
+  public clicked = new EventEmitter<Snapshot>();
  public matMenuTrigger: MatMenuTrigger;


Remove matMenuTrigger if it is not used

wowshakhov · 2018-12-04T02:30:29Z

src/app/snapshot/store/snapshot-page.selectors.ts

+        state: snapshot.state,
+        name: snapshot.displayname,
+        created: moment(snapshot.created).toDate(),
+        // account and domain necessary for grouping


account and domain are necessary for grouping

wowshakhov · 2018-12-04T02:30:57Z

src/app/snapshot/store/snapshot-page.selectors.ts

+      filter.volumeSnapshotTypes.includes(snapshot.snapshottype);
+
+    const filterByAccount = (snapshot: Snapshot) =>
+      !filter.accounts.length || !!filter.accounts.find(id => id === snapshot.account);


Replace !!find by includes

wowshakhov · 2018-12-04T02:49:51Z

src/app/volume/volume-item/volume-item.component.ts

-    if (!this.matMenuTrigger.menuOpen) {
-      this.onClick.emit(this.item);
-    }
+  public handleClick(): void {


Remove matMenuTrigger in this component if it's not used

wowshakhov · 2018-12-04T02:50:35Z

src/app/account/account/account-item.component.ts

-    if (!this.matMenuTrigger || !this.matMenuTrigger.menuOpen) {
-      this.onClick.emit(this.item);
-    }
+  public handleClick(): void {


Remove matMenuTrigger in this component if it's not used.

wowshakhov · 2018-12-04T02:51:05Z

src/app/security-group/sg-list-item/security-group-list-item.component.ts

-    if (!this.matMenuTrigger.menuOpen) {
-      this.onClick.emit(this.item);
-    }
+  public handleClick(): void {


Remove matMenuTrigger in this component if it's not used.

feat(vm-snapshots): implement vm snapshots details sidebar

9ecbab7

tamazlykar assigned wowshakhov Nov 30, 2018

tamazlykar requested a review from wowshakhov November 30, 2018 09:46

wowshakhov suggested changes Dec 3, 2018

View reviewed changes

tamazlykar commented Dec 3, 2018

View reviewed changes

src/app/snapshot/store/snapshot-page.selectors.ts Outdated Show resolved Hide resolved

fixes after review

a68e41f

tamazlykar requested a review from wowshakhov December 4, 2018 02:11

wowshakhov suggested changes Dec 4, 2018

View reviewed changes

tamazlykar added 2 commits December 4, 2018 11:12

fixes after review 2

2534704

fixes after review 3

010a5c9

wowshakhov approved these changes Dec 4, 2018

View reviewed changes

tamazlykar merged commit c040801 into bwsw:master Dec 4, 2018

tamazlykar deleted the 1332-vm-snapshot-sidebar branch December 4, 2018 06:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(vm-snapshots): implement vm snapshots details sidebar #1454

feat(vm-snapshots): implement vm snapshots details sidebar #1454

tamazlykar commented Nov 30, 2018

coveralls commented Nov 30, 2018 •

edited

Loading

wowshakhov commented Dec 3, 2018 •

edited

Loading

wowshakhov Dec 3, 2018

tamazlykar Dec 3, 2018

tamazlykar commented Dec 3, 2018

wowshakhov Dec 4, 2018

wowshakhov Dec 4, 2018

wowshakhov Dec 4, 2018

wowshakhov Dec 4, 2018

wowshakhov Dec 4, 2018

wowshakhov Dec 4, 2018

wowshakhov Dec 4, 2018

wowshakhov Dec 4, 2018

wowshakhov Dec 4, 2018

feat(vm-snapshots): implement vm snapshots details sidebar #1454

feat(vm-snapshots): implement vm snapshots details sidebar #1454

Conversation

tamazlykar commented Nov 30, 2018

coveralls commented Nov 30, 2018 • edited Loading

Pull Request Test Coverage Report for Build 1426

💛 - Coveralls

wowshakhov commented Dec 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tamazlykar commented Dec 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 30, 2018 •

edited

Loading

wowshakhov commented Dec 3, 2018 •

edited

Loading