Skip to content
This repository has been archived by the owner on Mar 25, 2023. It is now read-only.

feat(manage-accounts): add accounts view and allow to manage accounts #594

Merged
merged 44 commits into from
Oct 18, 2017

Conversation

ksendart
Copy link
Contributor

@ksendart ksendart commented Oct 6, 2017

# Conflicts:
#	src/app/shared/shared.module.ts
#	src/app/spare-drive/spare-drive-filter/spare-drive-filter.component.ts
#	src/app/vm/vm-filter/vm-filter.component.ts
# Conflicts:
#	src/app/shared/shared.module.ts
@ksendart ksendart requested a review from zolotyx October 6, 2017 09:48
import { Component, OnInit } from '@angular/core';
import { State } from '../../reducers/index';
import { Store } from '@ngrx/store';
import * as accountEvent from '../redux/accounts.actions';
Copy link
Contributor

Choose a reason for hiding this comment

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

please, rename this to accountActions/domainActions/roleActions

}

public ngOnInit() {
this.store.dispatch(new domainEvent.LoadDomainsRequest({}));
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove payload if it's not required or make it optional
remove passing empty object to constructor

@@ -0,0 +1,28 @@
<cs-account-list-filter *ngIf="isAdmin()"
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we should add some kind of top panel as other pages have

@@ -0,0 +1,46 @@
<cs-top-bar>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add grouping widget

</a>
<a
mdTabLink
[routerLink]="['./users']"
Copy link
Contributor

Choose a reason for hiding this comment

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

clicking on this tab redirects me to index page

import { Injectable } from '@angular/core';
import { Actions, Effect } from '@ngrx/effects';
import { Observable } from 'rxjs/Observable';
import * as roleEvent from './roles.actions';
Copy link
Contributor

Choose a reason for hiding this comment

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

rename

.map((roles: Role[]) => {
return new roleEvent.LoadRolesResponse(roles);
})
.catch(() => Observable.of(new roleEvent.LoadRolesResponse([]))):
Copy link
Contributor

Choose a reason for hiding this comment

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

please use optional payload or get rid of it. why do you use [] in constructor?

);

export const roleTypes = createSelector(
getRolesEntitiesState,
Copy link
Contributor

Choose a reason for hiding this comment

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

use list selector

getRolesEntitiesState,
state => {
const allTypes = state.list.map(role => role.type);
return allTypes.filter((value, index) => allTypes.indexOf(value) == index );
Copy link
Contributor

Choose a reason for hiding this comment

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

use another approach to get unique list

readonly selectedStates$ = this.store.select(fromAccounts.filterSelectedStates);
readonly selectedRoleTypes$ = this.store.select(fromAccounts.filterSelectedRoleTypes);

readonly states: Array<string> = ['enabled', 'disabled', 'locked'];
Copy link
Contributor

Choose a reason for hiding this comment

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

please add states constants

Copy link
Contributor

@zolotyx zolotyx left a comment

Choose a reason for hiding this comment

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

Please fix the mentioned issues

Copy link
Contributor

@zolotyx zolotyx left a comment

Choose a reason for hiding this comment

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

Please, remove a single tab on vm account sidebar and remove fab button from the listing page for now

(onGroupingsChange)="onGroupingsChange.emit($event)"
cs-list
></cs-account-list-filter>
<cs-top-bar *ngIf="!isAdmin()"></cs-top-bar>
Copy link
Contributor

Choose a reason for hiding this comment

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

let it be present for all the users but the content should be conditionally appear

@ksendart ksendart merged commit 844f98d into master Oct 18, 2017
@ksendart ksendart deleted the 526-manage-accounts branch October 18, 2017 04:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants