ui: Fixup KV folder creation then further creation within that folder #12081
Conversation
Passing '' should load a brand new `isNew` model which is what we need here
...to return a brand new model, therefore isNew will be set
johncowen
changed the title
| @@ -16,7 +16,7 @@ as |parentKey|}} | |||
| partition=route.params.partition | |||
| nspace=route.params.nspace | |||
| dc=route.params.dc | |||
| key=(if (string-ends-with routeName 'create') parentKey route.params.key) | |||
| key=(if (string-ends-with routeName 'create') '' route.params.key) | |||
There was a problem hiding this comment.
This was the actual bug
| }); | ||
| } else { | ||
| item = await super.findBySlug(...arguments); | ||
| } |
There was a problem hiding this comment.
This bit makes the "pass an empty id to get a new ember-data object" work
| And I click "[data-test-create]" | ||
| And I see the text "New Key / Value" in "h1" | ||
| And I see the text "key-value" in "[data-test-breadcrumbs] li:nth-child(2) a" | ||
| And I see the "[data-test-kv-key]" element |
There was a problem hiding this comment.
Test was mainly copy/pasted from the above (not visible unless you view the entire file). We were already testing: "creating a folder" and "clicking create from within a folder" but not "creating a folder and then immediately clicking create form within that folder".
I also noticed that we weren't asserting for existence of the input field in the "clicking create from within a folder" test that already existed. Whilst this test wasn't failing even with the extra assertion, I thought it would be a good idea to add here to helper prevent any possible regression in the future (line 52)
|
This is just a QA test; someone else needs to conduct the front-end engineering review As far as I can tell, this fix does address the bug. I ran this against a locally running Consul client agent (v1.11.x) via: With that setup, I can no longer reproduce the bug with the steps described above. |
|
Thanks for checking that for me @jkirschner-hashicorp, also super glad you found it easy to test the reproduction. FYI (and anyone else who may read this) you can just do also: As localhost:8500 is fairly common I made it so it will just use that by default if you don't set it. |
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/557877. |
|
🍒✅ Cherry pick of commit cdb8a35 onto |
…#12081) The fix here is two fold: - We shouldn't be providing the DataSource (which loads the data) with an id when we are creating from within a folder (in the buggy code we are providing the parentKey of the new KV you are creating) - Being able to provide an empty id to the DataSource/KV repository and that repository responding with a newly created object is more towards the "new way of doing forms", therefore the corresponding code to return a newly created ember-data object. As we changed the actual bug in point 1 here, we need to make sure the repository responds with an empty object when the request id is empty.
Fixes #12073
See ^ for details.
The KV edit/form flow is one of our oldest form flows and it is currently a little bit of a mash up of mostly one of our old ways of doing forms with a tiny bit of cross over into our newer way of doing forms (the idea is soon to most entirely to new style forms the same as our namespace and partitions forms).
The fix here is two fold: