ui: job templates manager view #15815

ChaiWithJai · 2023-01-18T01:17:51Z

Resolves #15550

github-actions · 2023-01-18T01:25:05Z

Ember Asset Size action

As of f795c3e

Files that got Bigger 🚨:

File	raw	gzip
nomad-ui.js	+5.91 kB	+515 B

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-18T01:58:50Z

Ember Test Audit comparison

	epic/job-templates	`f795c3e`	change
passes	1458	1458	0
failures	0	1	+1
flaky	0	0	0
duration	10m 48s 970ms	000ms	-10m 48s 970ms

philrenaud · 2023-01-18T04:48:03Z

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

+    const jobTemplateVariables = await this.store.query('variable', {
+      prefix: 'nomad/job-templates',
+      namespace: '*',
+    });
+    const recordsToQuery = jobTemplateVariables.map((template) =>
+      this.store.findRecord('variable', template.id)
+    );


Seeing all variables here in the Mirage mocks:
- is prefix the correct property, or is path?

Note: also seeing more variables than expected on the non-management list page (templates index)

In mirage mocks you will see all the variables here because we're using the /vars endpoint which does not have any logic for accounting for the prefix. Touching our Mirage setup would spill over into the variables view so I'm not going to touch it until after all the work is complete and we don't have more feature requests or updates on the feature.

Its annoying but we'll have to rely on a Nomad server here.

Understood! Let's definitely tackle this at some point in the process, if not in this PR.

Hey! Get back to being on vacation and recharge your batteries!!!

philrenaud · 2023-01-18T04:54:10Z

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

+        <Hds::Button class="button-group" @text="Back to editor" @route="jobs.run" @icon="arrow-left" data-test-cancel />
+    {{else}}
+    <main class="radio-group" data-test-template-list>
+        <Hds::Table @model={{this.model}} @columns={{ create-columns "Name" "Namespace" "Description"}}>


Hm, the extra abstraction (new helper just for columns) and ergonomics (<ListTable><t.head> at least used real <th> elements) makes me think that this is the wrong time to bring Hds::Table in.

philrenaud · 2023-01-18T04:54:53Z

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

+    {{else}}
+    <main class="radio-group" data-test-template-list>
+        <Hds::Table @model={{this.model}} @columns={{ create-columns "Name" "Namespace" "Description"}}>
+            <:body as |B|>


Could we change B to Template or something more indicative of what we're iterating over?

This isn't a template. Rather it's a contextual components that is yielding out TableRow and TableData components. We could change the name to Body to reflect this. And its a pattern used by other teams, in our own codebase and in line with the official documentation.

Changing this to Body could make sense. But that would defeat the purpose of the alias and we could just write :body.Tr instead.

Your call. I'm open to suggestions!

This makes a ton of sense. Thanks for clearing it up for me!

philrenaud · 2023-01-18T04:57:45Z

ui/app/helpers/create-columns.js

+import { helper } from '@ember/component/helper';
+
+function createColumns(positional) {
+  return positional.map((column) => ({ label: column }));
+}
+
+export default helper(createColumns);


I'm not convinced we want a helper just to make tables work with the fields we want, but if we decided we did:

I'd like this made more generalized (like map-to-objects instead of create-columns)

a second argument (either positional or named) for key, which would let you customize what you currently have hardcoded as label currently

My first suggestion would still be to consider if we really want to go down the road of invoking a helper every time we display a table, though.

Roger that. Normally we put this logic in our controllers check any controller that is using the SingleSelectDropdown feature and you'll see redundant code. IMHO the code isn't adding value when its in the controller or component because its not real state, just data transformation for working with another component API.

You'll see logic like this across 26 different views (about 50% of our views).

On pages where we have multiple dropdowns, it really muddies the controller and makes it challenging to find the code that matters. Most of the code in this file is just derived state that matches the component API for SingleSelectDropdown and all the code boils down to is iterating over a list to get a key and label.

philrenaud

Looks great, nice work

philrenaud · 2023-01-20T14:33:40Z

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

+    {{else}}
+    <main class="radio-group" data-test-template-list>
+        <Hds::Table @model={{this.model}} @columns={{ create-columns "Name" "Namespace" "Description"}}>
+            <:body as |B|>


This makes a ton of sense. Thanks for clearing it up for me!

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

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

            <:body as |B|>
                <B.Tr>
                    <B.Td>
                        <LinkTo @route="jobs.run.templates.template" @model={{concat B.data.path "@" B.data.namespace}} data-test-edit-template={{B.data.path}}>
-                            {{B.data.path}}
+                            {{invoke-fn this this.formatTemplateLabel B.data.path}}


Nice and reminds me of svelte attributes!

* 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 124e95d12140be38fc921f7e15243034092c4063. * 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 <[email protected]> * bug: edit `text` to `template` Co-authored-by: Phil Renaud <[email protected]> Co-authored-by: Phil Renaud <[email protected]> * 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 <[email protected]>

ui: manager list view

d4385f5

ChaiWithJai requested a review from philrenaud January 18, 2023 01:17

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

philrenaud requested changes Jan 18, 2023

View reviewed changes

ChaiWithJai added 2 commits January 19, 2023 11:16

test: edit flow

97241e1

refact: deprecate column-helper

f795c3e

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

ChaiWithJai requested a review from philrenaud January 19, 2023 20:24

ChaiWithJai added this to the 1.5.0 milestone Jan 19, 2023

philrenaud approved these changes Jan 20, 2023

View reviewed changes

ChaiWithJai merged commit 21fdcb4 into epic/job-templates Jan 20, 2023

ChaiWithJai deleted the 15550/job-templates-manager branch January 20, 2023 14:52

tgross mentioned this pull request Jun 28, 2024

[UI, Example Job Templates] New route: jobs/run/templates/manage #15550

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ui: job templates manager view #15815

ui: job templates manager view #15815

ChaiWithJai commented Jan 18, 2023

github-actions bot commented Jan 18, 2023 •

edited

Loading

github-actions bot commented Jan 18, 2023 •

edited

Loading

philrenaud Jan 18, 2023

philrenaud Jan 18, 2023

ChaiWithJai Jan 18, 2023

philrenaud Jan 18, 2023

ChaiWithJai Jan 18, 2023

philrenaud Jan 18, 2023

philrenaud Jan 18, 2023

ChaiWithJai Jan 19, 2023

philrenaud Jan 20, 2023

philrenaud Jan 18, 2023

ChaiWithJai Jan 19, 2023

philrenaud left a comment

philrenaud Jan 20, 2023

philrenaud Jan 20, 2023

ui: job templates manager view #15815

ui: job templates manager view #15815

Conversation

ChaiWithJai commented Jan 18, 2023

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

Ember Asset Size action

github-actions bot commented Jan 18, 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 18, 2023 •

edited

Loading

github-actions bot commented Jan 18, 2023 •

edited

Loading