fix broken link to `task-group` in `Recent Allocation` table in `jobs.job.index` #12765

ChaiWithJai · 2022-04-22T20:35:44Z

Resolves #12724

This PR fixes broken link to task-group in Recent Allocation table in jobs.job.index.

Side Effect

We needed to run prettier on 2 handlebar files. The commits are isolated to see the real change.

github-actions · 2022-04-22T20:41:58Z

Ember Asset Size action

As of 11f2ac0

Files that got Bigger 🚨:

File	raw	gzip
nomad-ui.js	+3.56 kB	+148 B

Files that got Smaller 🎉:

File	raw	gzip
vendor.js	-18.5 kB	-4.89 kB

Files that stayed the same size 🤷‍:

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

github-actions · 2022-04-22T21:12:12Z

Ember Test Audit comparison

	main	`11f2ac0`	change
passes	1282	1279	-3
failures	0	2	+2
flaky	0	1	+1
duration	9m 03s 728ms	000ms	-9m 03s 728ms

github-actions · 2022-04-23T00:46:50Z

Ember Test Audit flaky tests

Ember Test Audit detected these flaky tests on main:

Acceptance | job dispatch: required meta fields are properly indicated

mikenomitch

Can confirm that this fixes the issue locally for me!

Was able to easily repro with the old code and couldnt repro with the new!

lgfa29

I still see the error in the browser console in some pages. The easiest way to trigger it is by running a job in a non-default namespace then navigating to the job Allocations tab. I think it may be related to the lines I pointed where it's using the job as the model for LinkTo.

They happen in backgroud though, so it doesn't results in an error page, which is a big improvement 😄

There's also an added problem with the rendering of service IP:Port text that needs to be fixed.

lgfa29 · 2022-04-26T16:46:06Z

ui/app/templates/allocations/allocation/index.hbs

+                  {{row.model.hostIp}}
+                  :
+                  {{row.model.value}}


This change introduces a space between the host IP and port

Here's how it currently looks:

ui/app/templates/allocations/allocation/index.hbs

lgfa29 · 2022-04-26T17:52:22Z

ui/app/templates/allocations/allocation/index.hbs

+              <span class="pair job-link">
+                <span class="term">
+                  Job
+                </span>
                <LinkTo
                  @route="jobs.job"
                  @model={{this.preempter.job}}


Does this need to be @model={{format-job-id this.model.job.id}} as well?

lgfa29 · 2022-04-26T17:54:12Z

ui/app/templates/components/allocation-row.hbs

+        @model={{this.allocation.job}}
+        @query={{hash jobNamespace=this.allocation.job.namespace.id}}


I think this needs to be updated as well?

Yup its not causing any breaks and its ticketed here: #12764

lgfa29 · 2022-04-26T17:57:13Z

ui/app/templates/components/job-page/parts/recent-allocations.hbs

-        View all {{this.job.allocations.length}} {{pluralize "allocation" this.job.allocations.length}}
-      </LinkTo></p>
+      <p class="pull-right" data-test-view-all-allocations>
+        <LinkTo @route="jobs.job.allocations" @model={{this.job}}>


Not sure if this needs to use the new helper as well.

Yup its not causing any breaks and its ticketed here: #12764

Is it alright if I address that change in the above ticket?

lgfa29 · 2022-04-26T17:59:18Z

ui/tests/acceptance/allocation-detail-test.js

@@ -279,7 +279,7 @@ module('Acceptance | allocation detail', function (hooks) {
      assert.equal(renderedPort.name, serverPort.Label);
      assert.equal(renderedPort.to, serverPort.To);
      assert.equal(
-        renderedPort.address,
+        renderedPort.address.replace(/\s/g, ''),


I think this is result of added line breaks in ui/app/templates/allocations/allocation/index.hbs. We shouldn't modify the test as it correctly reports a breaking change.

There's nothing I can do about the extra spaces being added to handlebars as a result of prettier, however. So I'm not sure what I can do without spending a few hours to understand why.

Do you have any ideas?

You can ignore parts of the code: https://prettier.io/docs/en/ignore.html#handlebars

Ignoring prettier seems yucky. I decided to use a template helper to avoid the problem. However, I think @philrenaud and I should prioritize taking a look into our prettier and linting set-up when I'm back from vacation.

…bort

lgfa29 · 2022-04-28T00:11:51Z

ui/app/templates/allocations/allocation/index.hbs

              <td data-test-allocation-port-address>
                <a
                  href="http://{{row.model.hostIp}}:{{row.model.value}}"
                  target="_blank"
                  rel="noopener noreferrer"
-                >{{row.model.hostIp}}:{{row.model.value}}</a>
+                >
+                  {{row.model.hostIp}}:{{row.model.value}}


I reverted the change to use a helper here. It didn't seemed like a good idea to add more code to workaround a tooling issue.

It also seems like Prettier is fine with the code like this? It didn't try to change it for me at least, so I think it may be another plugin in your machine that is causing this formatting.

Moving forwards though, I think we should reconsider using Prettier for hbs files and minimize changes to the UI code to only what's necessary. I detected some other minor visual regressions caused by all these formatting and I worry about having others we may have missed. I will leave this decision to you and @philrenaud.

github-actions · 2022-04-28T00:43:08Z

Ember Test Audit flaky tests

Ember Test Audit detected these flaky tests on 11f2ac0:

Acceptance | client detail: each allocation should have high-level details for the allocation

github-actions · 2022-10-14T02:47:34Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

ChaiWithJai added 2 commits April 22, 2022 16:27

chore: run prettier on hbs files

d737d16

ui: ensure to pass a real job object to task-group link

f9bcc60

vercel bot deployed to Preview – nomad-storybook-and-ui April 22, 2022 20:35 View deployment

chore: add changelog entry

315ec5f

ChaiWithJai requested review from philrenaud and lgfa29 April 22, 2022 20:38

ChaiWithJai self-assigned this Apr 22, 2022

ChaiWithJai requested a review from mikenomitch April 22, 2022 20:40

vercel bot deployed to Preview – nomad-storybook-and-ui April 22, 2022 20:41 View deployment

ChaiWithJai added 3 commits April 22, 2022 20:08

chore: prettify template

82b6e5e

ui: template helper for formatting jobId in LinkTo component

4e961d4

ui: handle async relationship

73f4622

vercel bot deployed to Preview – nomad-storybook-and-ui April 23, 2022 00:13 View deployment

ChaiWithJai added 2 commits April 25, 2022 14:56

ui: pass in job id to model arg instead of job model

d93fee3

update test for serialized namespace

a481ca6

vercel bot deployed to Preview – nomad-storybook-and-ui April 25, 2022 19:00 View deployment

ChaiWithJai added 2 commits April 25, 2022 15:40

ui: defend against null in tests

5d14fe8

ui: prettified template added whitespace

bbd709e

vercel bot deployed to Preview – nomad-storybook-and-ui April 25, 2022 19:43 View deployment

mikenomitch reviewed Apr 25, 2022

View reviewed changes

philrenaud approved these changes Apr 26, 2022

View reviewed changes

lgfa29 suggested changes Apr 26, 2022

View reviewed changes

ChaiWithJai added 2 commits April 27, 2022 14:21

ui: rollback ember-data to 3.24 because watcher return undefined on a…

81c800c

…bort

ui: use format-job-helper instead of job model via alloc

ca5c791

vercel bot deployed to Preview – nomad-storybook-and-ui April 27, 2022 18:27 View deployment

ChaiWithJai added 2 commits April 27, 2022 15:23

ui: fix whitespace in template caused by prettier using template helper

05d1f2e

ui: update test for new namespace

0b9b380

vercel bot deployed to Preview – nomad-storybook-and-ui April 27, 2022 19:47 View deployment

ChaiWithJai requested a review from lgfa29 April 27, 2022 20:01

lgfa29 added this to the 1.3.0 milestone Apr 27, 2022

ui: revert prettier change

11f2ac0

vercel bot deployed to Preview – nomad-storybook-and-ui April 28, 2022 00:10 View deployment

lgfa29 reviewed Apr 28, 2022

View reviewed changes

lgfa29 approved these changes Apr 28, 2022

View reviewed changes

ChaiWithJai merged commit c180c8d into main Apr 28, 2022

ChaiWithJai deleted the b-ui/task-group-namespace-bug branch April 28, 2022 18:02

github-actions bot locked as resolved and limited conversation to collaborators Oct 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix broken link to `task-group` in `Recent Allocation` table in `jobs.job.index` #12765

fix broken link to `task-group` in `Recent Allocation` table in `jobs.job.index` #12765

ChaiWithJai commented Apr 22, 2022

github-actions bot commented Apr 22, 2022 •

edited

Loading

github-actions bot commented Apr 22, 2022 •

edited

Loading

github-actions bot commented Apr 23, 2022

mikenomitch left a comment

lgfa29 left a comment

lgfa29 Apr 26, 2022

lgfa29 Apr 26, 2022

lgfa29 Apr 26, 2022

ChaiWithJai Apr 26, 2022

lgfa29 Apr 26, 2022

ChaiWithJai Apr 26, 2022

ChaiWithJai Apr 27, 2022

lgfa29 Apr 26, 2022

ChaiWithJai Apr 26, 2022

lgfa29 Apr 26, 2022

ChaiWithJai Apr 27, 2022

lgfa29 Apr 28, 2022 •

edited

Loading

github-actions bot commented Apr 28, 2022

github-actions bot commented Oct 14, 2022

		@model={{this.allocation.job}}
		@query={{hash jobNamespace=this.allocation.job.namespace.id}}

fix broken link to task-group in Recent Allocation table in jobs.job.index #12765

fix broken link to task-group in Recent Allocation table in jobs.job.index #12765

Conversation

ChaiWithJai commented Apr 22, 2022

github-actions bot commented Apr 22, 2022 • edited Loading

Ember Asset Size action

github-actions bot commented Apr 22, 2022 • edited Loading

Ember Test Audit comparison

github-actions bot commented Apr 23, 2022

Ember Test Audit flaky tests

mikenomitch left a comment

Choose a reason for hiding this comment

lgfa29 left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgfa29 Apr 28, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Apr 28, 2022

Ember Test Audit flaky tests

github-actions bot commented Oct 14, 2022

fix broken link to `task-group` in `Recent Allocation` table in `jobs.job.index` #12765

fix broken link to `task-group` in `Recent Allocation` table in `jobs.job.index` #12765

github-actions bot commented Apr 22, 2022 •

edited

Loading

github-actions bot commented Apr 22, 2022 •

edited

Loading

lgfa29 Apr 28, 2022 •

edited

Loading