ui: template edit and delete flow #15823

ChaiWithJai · 2023-01-19T16:15:57Z

Resolves #15551

github-actions · 2023-01-19T16:22:55Z

Ember Asset Size action

As of 82d10b0

Files that got Bigger 🚨:

File	raw	gzip
nomad-ui.js	+9.84 kB	+1.18 kB

Files that stayed the same size 🤷‍:

File	raw	gzip
vendor.js	0 B	0 B
nomad-ui.css	0 B	0 B
vendor.css	0 B	0 B

github-actions · 2023-01-19T16:57:32Z

Ember Test Audit comparison

	epic/job-templates	`82d10b0`	change
passes	1458	1459	+1
failures	1	0	-1
flaky	0	0	0
duration	000ms	11m 28s 517ms	+11m 28s 517ms

* bug: clear controllers on exit * test: mirage config changes (#15828)

ui/app/templates/jobs/run/templates/template.hbs

philrenaud · 2023-01-20T14:56:17Z

ui/app/routes/jobs/run/templates/template.js

+import { inject as service } from '@ember/service';
+import notifyForbidden from 'nomad-ui/utils/notify-forbidden';
+
+export default class RunRoute extends Route {


Naming convention request: I think this is borrowed from routes/jobs/run and duplicated here (jobs/run/templates/template) as well as jobs/run/templates.

Could we use the more standard RunTemplatesRoute and RunTemplatesTemplateRoute for easier grepping?

Will patch this when I throw the commit up for deleting inside of a row.

philrenaud · 2023-01-20T14:58:51Z

ui/app/controllers/jobs/run/templates/template.js

+  async save(e, overwrite = false) {
+    if (e.type === 'submit') {
+      e.preventDefault();
+    }
+
+    this.model.set('keyValues', this.keyValues);
+
+    try {
+      await this.model.save({ adapterOptions: { overwrite } });
+
+      this.flashMessages.add({
+        title: 'Job template saved',
+        message: `${this.model.path} successfully editted`,
+        type: 'success',
+        destroyOnClick: false,
+        timeout: 5000,
+      });
+
+      this.router.transitionTo('jobs.run.templates');
+    } catch (e) {
+      this.flashMessages.add({
+        title: 'Job template cannot be editted.',
+        message: e,
+        type: 'error',
+        destroyOnClick: false,
+        timeout: 5000,
+      });
+    }
+  }
+}


Should we prevent save unless the user has added some template? We don't have to do careful hcl parsing or anything, but we should at least ensure they haven't accidentally left this blank.

Preferred place would be as a disabled condition on the `save1 button in template.

I'm unsure if we should guard against these actions. We're expecting minimal use on the feature and I generally don't like overly inhibited UIs. This is not a required action needed to move forward.

HDS advises that:

Disabled buttons should be used sparingly and with intention. Acceptable use cases for disabled buttons include: Incomplete flows (ie. user has not yet completed a required action needed to move forward). Permissions restriction.

ChaiWithJai · 2023-01-20T18:33:24Z

ui/app/controllers/jobs/run/templates/template.js

+import { action } from '@ember/object';
+import { tracked } from '@glimmer/tracking';
+
+export default class JobsRunTemplatesController extends Controller {


Will address naming when I patch delete in row functionality.

Will get this after we confirm that I addressed the real changes. My bad.

philrenaud

It looks like some of the recent changes (z-index changes to account for HDS' modal pattern) are causing breaks in other places

I'd like to strongly suggest not overlapping this feature with HDS inclusion; we're seeing that that deserves its own dedicated effort.

ChaiWithJai · 2023-01-22T02:16:53Z

It looks like some of the recent changes (z-index changes to account for HDS' modal pattern) are causing breaks in other places

I'd like to strongly suggest not overlapping this feature with HDS inclusion; we're seeing that that deserves its own dedicated effort.

OK! Do want me to re-do the PR? I think our z-index woes are fairly trivial here. But I can just re-do this part feature because we're already ahead of schedule. LMK what you want to see.

philrenaud · 2023-01-22T03:49:48Z

OK! Do want me to re-do the PR?

I think you can keep this PR, but I would use the conventions we've typically used in Nomad. The changes in behaviour (buttons, dropdowns) and styles (tables) on just these pages is going to be jarring to users.

This reverts commit d3d87ce.

ui/app/templates/jobs/run/templates/manage.hbs

philrenaud

Looks good! Made some suggestions for making the delete confirmations more understandable for users, and would like to see a few tests here and route name changes. But the logic and the flow feels great!

ui/app/templates/jobs/run/templates/manage.hbs

ui/app/templates/jobs/run/templates/template.hbs

philrenaud · 2023-01-25T19:03:41Z

ui/tests/acceptance/job-run-test.js

@@ -434,6 +434,7 @@ module('Acceptance | job run', function (hooks) {
        path: 'nomad/job-templates/foo',
        namespace: 'default',
        id: 'nomad/job-templates/foo',
+        Items: {},
      });


Would like to see some tests for delete / edit template. Future PR is fine.

We have edit template! We're just missing delete.

Co-authored-by: Phil Renaud <phil.renaud@hashicorp.com>

* Extend variables under the nomad path prefix to allow for job-templates (#15570) * Extend variables under the nomad path prefix to allow for job-templates * Add job-templates to error message hinting * RadioCard component for Job Templates (#15582) * chore: add * test: component API * ui: component template * refact: remove bc naming collission * styles: remove SASS var causing conflicts * Disallow specific variable at nomad/job-templates (#15681) * Disallows variables at exactly nomad/job-templates * idiomatic refactor * Expanding nomad job init to accept a template flag (#15571) * Adding a string flag for templates on job init * data-down actions-up version of a custom template editor within variable * Dont force grid on job template editor * list-templates flag started * Correctly slice from end of path name * Pre-review cleanup * Variable form acceptance test for job template editing * Some review cleanup * List Job templates test * Example from template test * Using must.assertions instead of require etc * ui: add choose template button (#15596) * ui: add new routes * chore: update file directory * ui: add choose template button * test: button and page navigation * refact: update var name * ui: use `Button` component from `HDS` (#15607) * ui: integrate buttons * refact: remove helper * ui: remove icons on non-tertiary buttons * refact: update normalize method for key/value pairs (#15612) * `revert`: `onCancel` for `JobDefinition` The `onCancel` method isn't included in the component API for `JobEditor` and the primary cancel behavior exists outside of the component. With the exception of the `JobDefinition` page where we include this button in the top right of the component instead of next to the `Plan` button. * style: increase button size * style: keep lime green * ui: select template (#15613) * ui: deprecate unused component * ui: deprecate tests * ui: jobs.run.templates.index * ui: update logic to handle templates * refact: revert key/value changes * style: padding for cards + buttons * temp: fixtures for mirage testing * Revert "refact: revert key/value changes" This reverts commit 124e95d. * ui: guard template for unsaved job * ui: handle reading template variable * Revert "refact: update normalize method for key/value pairs (#15612)" This reverts commit 6f5ffc9. * revert: remove test fixtures * revert: prettier problems * refact: test doesnt need filter expression * styling: button sizes and responsive cards * refact: remove route guarding * ui: update variable adapter * refact: remove model editing behavior * refact: model should query variables to populate editor * ui: clear qp on exit * refact: cleanup deprecated API * refact: query all namespaces * refact: deprecate action * ui: rely on collection * refact: patch deprecate transition API * refact: patch test to expect namespace qp * styling: padding, conditionals * ui: flashMessage on 404 * test: update for o(n+1) query * ui: create new job template (#15744) * refact: remove unused code * refact: add type safety * test: select template flow * test: add data-test attrs * chore: remove dead code * test: create new job flow * ui: add create button * ui: create job template * refact: no need for wildcard * refact: record instead of delete * styling: spacing * ui: add error handling and form validation to job create template (#15767) * ui: handle server side errors * ui: show error to prevent duplicate * refact: conditional namespace * ui: save as template flow (#15787) * bug: patches failing tests associated with `pretender` (#15812) * refact: update assertion * refact: test set-up * ui: job templates manager view (#15815) * ui: manager list view * test: edit flow * refact: deprecate column-helper * ui: template edit and delete flow (#15823) * ui: manager list view * refact: update title * refact: update permissions * ui: template edit page * bug: typo * refact: update toast messages * bug: clear selections on exit (#15827) * bug: clear controllers on exit * test: mirage config changes (#15828) * refact: deprecate column-helper * style: update z-index for HDS * Revert "style: update z-index for HDS" This reverts commit d3d87ce. * refact: update delete button * refact: edit redirect * refact: patch reactivity issues * styling: fixed width * refact: override defaults * styling: edit text causing overflow * styling: add inline text Co-authored-by: Phil Renaud <phil.renaud@hashicorp.com> * bug: edit `text` to `template` Co-authored-by: Phil Renaud <phil.renaud@hashicorp.com> Co-authored-by: Phil Renaud <phil.renaud@hashicorp.com> * test: delete flow job templates (#15896) * refact: edit names * bug: set correct ref to store * chore: trim whitespace: * test: delete flow * bug: reactively update view (#15904) * Initialized default jobs (#15856) * Initialized default jobs * More jobs scaffolded * Better commenting on a couple example job specs * Adapter doing the work * fall back to epic config * Label format helper and custom serialization logic * Test updates to account for a never-empty state * Test suite uses settled and maintain RecordArray in adapter return * Updates to hello-world and variables example jobspecs * Parameterized job gets optional payload output * Formatting changes for param and service discovery job templates * Multi-group service discovery job * Basic test for default templates (#15965) * Basic test for default templates * Percy snapshot for manage page * Some late-breaking design changes * Some copy edits to the header paragraphs for job templates (#15967) * Added some init options for job templates (#15994) * Async method for populating default job templates from the variable adapter --------- Co-authored-by: Jai <41024828+ChaiWithJai@users.noreply.github.com>

ChaiWithJai added 4 commits January 17, 2023 20:16

ui: manager list view

d4385f5

refact: update title

0c6a1bd

refact: update permissions

c33e8fc

ui: template edit page

808801f

ChaiWithJai requested a review from philrenaud January 19, 2023 16:15

vercel bot deployed to Preview – nomad-storybook-and-ui January 19, 2023 16:21 View deployment

bug: typo

c26a728

vercel bot deployed to Preview – nomad-storybook-and-ui January 19, 2023 18:17 View deployment

refact: update toast messages

ac77070

vercel bot deployed to Preview – nomad-storybook-and-ui January 19, 2023 19:19 View deployment

ChaiWithJai added this to the 1.5.0 milestone Jan 19, 2023

Base automatically changed from 15550/job-templates-manager to epic/job-templates January 20, 2023 14:52

ChaiWithJai added 2 commits January 20, 2023 09:53

bug: clear selections on exit (#15827)

cc0328f

* bug: clear controllers on exit * test: mirage config changes (#15828)

Merge branch 'epic/job-templates' into 15551/job-template-edit

d43aa12

philrenaud requested changes Jan 20, 2023

View reviewed changes

vercel bot deployed to Preview – nomad-storybook-and-ui January 20, 2023 15:00 View deployment

ChaiWithJai added 2 commits January 20, 2023 12:31

refact: deprecate column-helper

33b7366

style: update z-index for HDS

d3d87ce

ChaiWithJai requested a review from philrenaud January 20, 2023 17:32

vercel bot deployed to Preview – nomad-storybook-and-ui January 20, 2023 17:38 View deployment

ChaiWithJai commented Jan 20, 2023

View reviewed changes

philrenaud requested changes Jan 20, 2023

View reviewed changes

ChaiWithJai added 2 commits January 24, 2023 16:36

Revert "style: update z-index for HDS"

5a5d49f

This reverts commit d3d87ce.

refact: update delete button

eb09d40

ChaiWithJai commented Jan 24, 2023

View reviewed changes

ui/app/templates/jobs/run/templates/manage.hbs Outdated Show resolved Hide resolved

vercel bot deployed to Preview – nomad-storybook-and-ui January 24, 2023 22:25 View deployment

ChaiWithJai added 2 commits January 24, 2023 17:26

refact: edit redirect

1d4c4b8

refact: patch reactivity issues

9d186be

vercel bot deployed to Preview – nomad-storybook-and-ui January 24, 2023 23:12 View deployment

styling: fixed width

47acc7d

vercel bot deployed to Preview – nomad-storybook-and-ui January 24, 2023 23:19 View deployment

refact: override defaults

d2fa063

ChaiWithJai requested a review from philrenaud January 24, 2023 23:43

vercel bot deployed to Preview – nomad-storybook-and-ui January 24, 2023 23:49 View deployment

styling: edit text causing overflow

266b7e3

vercel bot deployed to Preview – nomad-storybook-and-ui January 25, 2023 14:51 View deployment

philrenaud approved these changes Jan 25, 2023

View reviewed changes

ChaiWithJai and others added 2 commits January 25, 2023 14:16

styling: add inline text

7eb8a23

Co-authored-by: Phil Renaud <phil.renaud@hashicorp.com>

bug: edit text to template

82d10b0

Co-authored-by: Phil Renaud <phil.renaud@hashicorp.com>

vercel bot deployed to Preview – nomad-storybook-and-ui January 25, 2023 19:21 View deployment

ChaiWithJai merged commit 6b4ca72 into epic/job-templates Jan 25, 2023

ChaiWithJai deleted the 15551/job-template-edit branch January 25, 2023 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ui: template edit and delete flow #15823

ui: template edit and delete flow #15823

ChaiWithJai commented Jan 19, 2023

github-actions bot commented Jan 19, 2023 •

edited

Loading

github-actions bot commented Jan 19, 2023 •

edited

Loading

philrenaud Jan 20, 2023

ChaiWithJai Jan 20, 2023

ChaiWithJai Jan 20, 2023

philrenaud Jan 20, 2023

ChaiWithJai Jan 20, 2023

ChaiWithJai Jan 20, 2023

ChaiWithJai Jan 24, 2023

philrenaud left a comment

ChaiWithJai commented Jan 22, 2023

philrenaud commented Jan 22, 2023

philrenaud left a comment

philrenaud Jan 25, 2023

ChaiWithJai Jan 25, 2023

ui: template edit and delete flow #15823

ui: template edit and delete flow #15823

Conversation

ChaiWithJai commented Jan 19, 2023

github-actions bot commented Jan 19, 2023 • edited Loading

Ember Asset Size action

github-actions bot commented Jan 19, 2023 • edited Loading

Ember Test Audit comparison

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philrenaud left a comment

Choose a reason for hiding this comment

ChaiWithJai commented Jan 22, 2023

philrenaud commented Jan 22, 2023

philrenaud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 19, 2023 •

edited

Loading

github-actions bot commented Jan 19, 2023 •

edited

Loading