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

Add restage to application page. #2828

Merged
merged 11 commits into from
Sep 11, 2018
Merged

Add restage to application page. #2828

merged 11 commits into from
Sep 11, 2018

Conversation

KlapTrap
Copy link
Contributor

fixes #2577

@cfdreddbot
Copy link

Hey KlapTrap!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@codecov
Copy link

codecov bot commented Aug 13, 2018

Codecov Report

Merging #2828 into v2-master will decrease coverage by 24.38%.
The diff coverage is 52.77%.

@@              Coverage Diff               @@
##           v2-master    #2828       +/-   ##
==============================================
- Coverage      71.16%   46.77%   -24.39%     
==============================================
  Files            607      607               
  Lines          26131    26140        +9     
  Branches        5915     5917        +2     
==============================================
- Hits           18596    12228     -6368     
- Misses          7535    13912     +6377

nwmac
nwmac previously requested changes Aug 14, 2018
Copy link
Contributor

@nwmac nwmac left a comment

Choose a reason for hiding this comment

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

LGTM - couple of things.

Unit tests will need updating for the app actions.

<button mat-icon-button name="cli" routerLink="/applications/{{applicationService.cfGuid}}/{{applicationService.appGuid}}/cli" matTooltip="CLI Info">
<mat-icon>keyboard</mat-icon>
<ng-template #startButton>
<button mat-icon-button name="start" [disabled]="busy.updating || !appState.actions.start && !appState.actions.stop" (click)="startApplication()" matTooltip="Start">
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 start button should only be disabled if start is not allowed - [disabled]="busy.updating || !appState.actions.start".

@@ -1,12 +1,5 @@
<app-page-header [breadcrumbs]="breadcrumbs$ | async">
<h1>CLI Info</h1>
<div class="page-header-right">
Copy link
Contributor

Choose a reason for hiding this comment

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

There are other places where we have the close button, so we should be consistent and remove them there too.

@@ -62,7 +59,7 @@ export class CliInfoApplicationComponent implements OnInit {
this.route$ = this.store.select(getPreviousRoutingState).pipe(
map(route => {
return {
url: route && route.state ? route.state.url : defaultBackLink,
Copy link
Contributor

Choose a reason for hiding this comment

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

With the close button removed, you can remove the back() method, setupRouteObservable, defaultBackLink etc - these were only there to find the back URL - we are always going back to the summary tab now, so not needed.

@richard-cox richard-cox added the needs attention This PR needs attention label Aug 16, 2018
* v2-master: (149 commits)
  Test bump [e2e]
  Enable app deply e2e test
  Fix application deploy For request params ensure xGUID -> xGuid
  Temporarily disable app deploy test - see #2930
  Add additional logging
  Fix jetstream output
  Updates following review
  Fix README buttons after merge
  Fix code cov button
  Fix CF Console client [e2e]
  Check in travis E2E config for jetstream [e2e]
  Update package.json [e2e]
  Update docker-compose & Travis e2e script [e2e]
  Increase mem for standard `build`
  Enabled secure cookie for VCapApplication apps
  Pin dependency to new version of sqlitestore - cookie fix
  Update cache-control values - Static files served via backed (AIO + cf push world) should have `cache-control: no-cache` - IE caches some https content, therefore jetstream shouldn't cache any requests `cache-control: no-store`
  Weekly update
  Harden the service wall instance card - Ensure we only show the template, which requires serviceInstanceEntity, when we have it - Removed ServicesWallService provider, it's not used (ctor never called) - Removed/Sorted imports
  Ensure golint is installed
  ...
* v2-master:
  Remove fdescribe
  Fix broken e2e tests [e2e]
  Update build.sh
  Update concourse pipelines
  Update helm chart
  Add readme
  Update requiremens
  Add Metrics chart
@KlapTrap KlapTrap added ready for review and removed needs attention This PR needs attention labels Aug 31, 2018
@nwmac nwmac merged commit 028cef2 into v2-master Sep 11, 2018
@nwmac nwmac deleted the redeploy-app branch September 11, 2018 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for restaging an application
4 participants