Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

ui: mutable deployments #1549

Merged
merged 3 commits into from
Jun 3, 2021
Merged

ui: mutable deployments #1549

merged 3 commits into from
Jun 3, 2021

Conversation

jgwhite
Copy link
Contributor

@jgwhite jgwhite commented May 27, 2021

What is this?

Addresses #1239

Adds the UI to display mutable deployments.

Mutable deployments are groups of deployment versions that utilize the same resources, where newer deployments in the same generation overwrite previous deployments (ie only latest version is deployed at the same deployment URL). This functionality was added to accommodate users' existing workflows.

What’s the plan?

  • Create good fixture data in Mirage
  • Implement the designs
  • Ensure immutable deployments still render well
  • Ensure destroyed deployments still render well
  • Use the {{t ...}} helper for all text
  • Write some light tests (we can probably borrow the dev Mirage config)
  • Changelog entry

What does it look like?

Screen Shot 2021-06-02 at 1 17 39 PM

Deployments list without destroyed deployments, note the grayed background of overwritten deployments + link to deployment only on latest

Screen Shot 2021-06-02 at 1 17 47 PM

Deployments list with destroyed deployment

How do I verify it?

  1. Check out this branch
  2. yarn ember serve
  3. Visit http://localhost:4200/tests
  4. Verify all tests pass
  5. Visit http://localhost:4200
  6. Check the deployments under mutable-application in mutable-project
  7. Verify there's no buggy behavior and everything else is untouched (check the builds section)

@jgwhite jgwhite added the ui label May 27, 2021
@jgwhite jgwhite self-assigned this May 27, 2021
@sabrinako sabrinako marked this pull request as ready for review June 2, 2021 17:25
@sabrinako sabrinako assigned jgwhite and unassigned jgwhite Jun 2, 2021
@sabrinako sabrinako requested a review from almonk June 2, 2021 17:28
@sabrinako sabrinako requested a review from a team June 2, 2021 17:41
</Pds::Button>
</Pds::HelpText>
</div>
{{/if}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very glad to see this move outside of the <ul>.

@jgwhite
Copy link
Contributor Author

jgwhite commented Jun 2, 2021

Technically I can’t approve this PR because I opened it, but it gets my 👍 nonetheless.

Copy link
Member

@briancain briancain left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍🏻 I was able to build my own waypoint server container with the latest changes to test. Ran through a nomad jobspec and a k8s apply project and did a few waypoint ups for both.

Copy link
Contributor

@almonk almonk left a comment

Choose a reason for hiding this comment

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

Looks great and the attention to detail is :chefskiss:

Some minor changes highlighted that I think we should make

{{#if (or this.isShowingDestroyed (not-eq deployment.state 4))}}
<AppItem::Deployment @deployment={{deployment}} @latest={{group.deployments.[0]}} />
{{/if}}
{{/each}}
{{else}}
<EmptyState>
<p>There are no deployments to display for this app yet</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this text be in the translations/ file?

Copy link
Contributor

Choose a reason for hiding this comment

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

I started moving some of the text in this deployments.hbs file to the translations files, but @jgwhite said the checklist item about it on this PR was meant for the single deployment item template, not this overarching deployments template.

Copy link
Contributor

Choose a reason for hiding this comment

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

I added the other text to the translation file. Jamie and I discussed this specific block of text yesterday and agreed the paragraph breaks make it a bit more of a complex situation. It's a little out of scope for this PR, so we can add this text to the localization texts some other time.

ui/app/styles/_app-item.scss Outdated Show resolved Hide resolved
ui/translations/en-us.yaml Outdated Show resolved Hide resolved
@sabrinako sabrinako merged commit 1a3c3d0 into main Jun 3, 2021
@sabrinako sabrinako deleted the ui-mutable-deployments branch June 3, 2021 15:02
@sabrinako sabrinako linked an issue Jun 29, 2021 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mutable Deployment Support in the browser UI
4 participants