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

feat(listAll): add listAll and account filter #535

Merged
merged 26 commits into from
Sep 29, 2017
Merged

feat(listAll): add listAll and account filter #535

merged 26 commits into from
Sep 29, 2017

Conversation

ksendart
Copy link
Contributor

@@ -40,6 +40,7 @@ export abstract class BaseBackendService<M extends BaseModel> {
}

public getList(params?: {}, customApiFormat?: ApiFormat): Observable<Array<M>> {
params = Object.assign(params || {}, { 'listAll': 'true' });
Copy link
Contributor

Choose a reason for hiding this comment

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

params equal to {} by default. no need to check params || {}
use Object.assign({}, params, {listall...})

@@ -90,10 +91,20 @@ export class VmListComponent implements OnInit {
this.outputs = {
onClick: this.showDetail
};

if (this.authService.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 it's better to add this item to groupings and then filter out

return user ? user.name: account;
}

private sortByAccount(accounts) {
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 this function should take volumes list and accounts

Copy link
Contributor

Choose a reason for hiding this comment

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

Use accounts = [] by default

private sortByAccount(accounts) {
let result: Array<Volume> = [];
if (accounts && accounts.length != 0) {
accounts.forEach(account => {
Copy link
Contributor

Choose a reason for hiding this comment

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

do return visibleVolumes.filter(volume=> accounts.find(a=>a.name === volume.account)

@zolotyx
Copy link
Contributor

zolotyx commented Sep 26, 2017

@ksendart
1 - Please merge master
2 - add account names to domain names when grouping by Account
3 - fix this bug when a group is selected
selected grouping

@ksendart ksendart removed the request for review from andrewbents September 26, 2017 07:32
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 ROOT/ prefix in Account filter select

@ksendart ksendart merged commit f950015 into master Sep 29, 2017
@ksendart ksendart deleted the 305-list-all branch September 29, 2017 06:25
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