-
Notifications
You must be signed in to change notification settings - Fork 135
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
Hide app vars tab if user is not a space developer #3247
Changes from 3 commits
2ef9259
5c4c414
a38d1c2
ac74017
1e3bfe5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
# E2E Configuration file | ||
|
||
consoleUsers: | ||
admin: | ||
username: admin | ||
password: changeme | ||
nonAdmin: | ||
username: user | ||
password: changeme | ||
|
||
endpoints: | ||
cf: | ||
- name: cf | ||
url: https://api.10.86.2.89.xip.io | ||
skipSSLValidation: true | ||
testOrg: e2e | ||
testSpace: e2e | ||
testService: app-autoscaler | ||
services: | ||
bindApp: go-env | ||
publicService: | ||
name: public-service | ||
privateService: | ||
name: private-service | ||
invalidOrgName: test-e2e | ||
invalidSpaceName: test-e2e | ||
spaceScopedService: | ||
name: space-scoped-service | ||
invalidOrgName: test-e2e | ||
invalidSpaceName: test-e2e | ||
creds: | ||
admin: | ||
username: admin | ||
password: changeme | ||
nonAdmin: | ||
username: user | ||
password: changeme | ||
- name: regonlycf | ||
url: https://cfinfo.capbristol.com | ||
skipSSLValidation: true | ||
testOrg: e2e | ||
testSpace: e2e | ||
creds: | ||
admin: | ||
username: admin | ||
password: changeme | ||
nonAdmin: | ||
username: e2e | ||
password: changeme | ||
|
||
# Skip SSL validation when talking to the backend API for test setup | ||
skipSSLValidation: true | ||
|
||
#headless: true | ||
#stratosGitHubApiUrl: https://github.capbristol.com:37070 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ import { | |
spaceSchemaKey, | ||
spaceWithOrgKey, | ||
stackSchemaKey, | ||
appEnvVarsSchemaKey, | ||
} from '../../store/helpers/entity-factory'; | ||
import { createEntityRelationKey } from '../../store/helpers/entity-relations/entity-relations.types'; | ||
import { ActionState, rootUpdatingKey } from '../../store/reducers/api-request-reducer/types'; | ||
|
@@ -47,6 +48,7 @@ import { | |
EnvVarStratosProject, | ||
} from './application/application-tabs-base/tabs/build-tab/application-env-vars.service'; | ||
import { getRoute, isTCPRoute } from './routes/routes.helper'; | ||
import { EntityMonitorFactory } from '../../shared/monitors/entity-monitor.factory.service'; | ||
|
||
|
||
export function createGetApplicationAction(guid: string, endpointGuid: string) { | ||
|
@@ -213,6 +215,15 @@ export class ApplicationService { | |
this.appEnvVars = this.appEnvVarsService.createEnvVarsObs(this.appGuid, this.cfGuid); | ||
} | ||
|
||
public getApplicationEnvVarsMonitor() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is better placed in ApplicationEnvVarsHelper alongside createEnvVarsObs |
||
const factory = new EntityMonitorFactory(this.store); | ||
return factory.create<APIResource<IApp>>( | ||
this.appGuid, | ||
appEnvVarsSchemaKey, | ||
entityFactory(appEnvVarsSchemaKey) | ||
); | ||
} | ||
|
||
private constructAmalgamatedObservables() { | ||
// Assign/Amalgamate them to public properties (with mangling if required) | ||
const action = new GetAppStatsAction(this.appGuid, this.cfGuid); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,11 @@ | ||
import { CfUserService } from './../../../../shared/data-services/cf-user.service'; | ||
|
||
import { Component, Inject, NgZone, OnDestroy, OnInit } from '@angular/core'; | ||
import { ActivatedRoute, Router } from '@angular/router'; | ||
import { Store } from '@ngrx/store'; | ||
import { combineLatest as observableCombineLatest, Observable, of as observableOf, Subscription } from 'rxjs'; | ||
import { delay, filter, first, map, mergeMap, tap, withLatestFrom, startWith } from 'rxjs/operators'; | ||
import { delay, filter, first, map, mergeMap, tap, withLatestFrom, startWith, switchMap } from 'rxjs/operators'; | ||
import { IApp, IOrganization, ISpace } from '../../../../core/cf-api.types'; | ||
import { CurrentUserPermissions } from '../../../../core/current-user-permissions.config'; | ||
import { EntityService } from '../../../../core/entity-service'; | ||
import { ConfirmationDialogConfig } from '../../../../shared/components/confirmation-dialog.config'; | ||
import { ConfirmationDialogService } from '../../../../shared/components/confirmation-dialog.service'; | ||
|
@@ -32,6 +32,7 @@ import { | |
getActionsFromExtensions, | ||
StratosActionType | ||
} from '../../../../core/extension/extension-service'; | ||
import { CurrentUserPermissions } from '../../../../core/current-user-permissions.config'; | ||
|
||
// Confirmation dialogs | ||
const appStopConfirmation = new ConfirmationDialogConfig( | ||
|
@@ -110,9 +111,28 @@ export class ApplicationTabsBaseComponent implements OnInit, OnDestroy { | |
} | ||
}); | ||
|
||
// Listen for the response to the env vars API call - if it errors then we hide the variables tab | ||
// This is simpler than the code to check permissions since we make the env vars request anyway | ||
const appDoesNotHaveEnvVars$ = this.applicationService.getApplicationEnvVarsMonitor().entityRequest$.pipe( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think we should stick to the permissions. That way the user can then refresh the list and still see the env vars. To switch would need to
|
||
filter(resp => resp.error || resp.response), | ||
first(), | ||
map(resp => resp.error) | ||
); | ||
|
||
this.tabLinks = [ | ||
{ link: 'summary', label: 'Summary' }, | ||
{ link: 'instances', label: 'Instances' }, | ||
{ link: 'routes', label: 'Routes' }, | ||
{ link: 'log-stream', label: 'Log Stream' }, | ||
{ link: 'services', label: 'Services' }, | ||
{ link: 'variables', label: 'Variables', hidden: appDoesNotHaveEnvVars$ }, | ||
{ link: 'events', label: 'Events' } | ||
]; | ||
|
||
// Add any tabs from extensions | ||
this.tabLinks = this.tabLinks.concat(getTabsFromExtensions(StratosTabType.Application)); | ||
} | ||
|
||
public breadcrumbs$: Observable<IHeaderBreadcrumb[]>; | ||
isFetching$: Observable<boolean>; | ||
applicationActions$: Observable<string[]>; | ||
|
@@ -125,15 +145,7 @@ export class ApplicationTabsBaseComponent implements OnInit, OnDestroy { | |
update => update[this.autoRefreshString] || { busy: false } | ||
)); | ||
|
||
tabLinks: ISubHeaderTabs[] = [ | ||
{ link: 'summary', label: 'Summary' }, | ||
{ link: 'instances', label: 'Instances' }, | ||
{ link: 'routes', label: 'Routes' }, | ||
{ link: 'log-stream', label: 'Log Stream' }, | ||
{ link: 'services', label: 'Services' }, | ||
{ link: 'variables', label: 'Variables' }, | ||
{ link: 'events', label: 'Events' } | ||
]; | ||
tabLinks: ISubHeaderTabs[]; | ||
|
||
private getBreadcrumbs( | ||
application: IApp, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,9 +9,6 @@ | |
position: relative; | ||
height: 46px; | ||
} | ||
&__tab-hidden { | ||
display: none; | ||
} | ||
} | ||
|
||
.tab-nav { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this file be in the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No