ui: Ensure we show a special readonly page for intentions #11767
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ```release-note:bug | ||
| ui: Ensure we show a readonly designed page for readonly intentions | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,7 @@ | ||
| <div | ||
| class="consul-intention-view" | ||
| ...attributes | ||
| > | ||
|
|
||
| <div class="definition-table"> | ||
| <dl> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| @setupApplicationTest | ||
| Feature: dc / intentions / read-only | ||
| Scenario: Viewing a readonly intention | ||
| Given 1 datacenter model with the value "dc1" | ||
| And 1 intention model from yaml: | ||
| --- | ||
| Meta: | ||
| external-source: kubernetes | ||
| --- | ||
| When I visit the intention page for yaml | ||
| --- | ||
| dc: dc1 | ||
| intention: default:external-source:web:default:external-source:db | ||
| --- | ||
| Then I see the "[data-test-readonly]" element |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import steps from '../../steps'; | ||
|
|
||
| // step definitions that are shared between features should be moved to the | ||
| // tests/acceptance/steps/steps.js file | ||
|
|
||
| export default function(assert) { | ||
| return steps(assert).then('I should find a file', function() { | ||
| assert.ok(true, this.step); | ||
| }); | ||
| } | ||
|
Comment on lines
+6
to
+10
There was a problem hiding this comment. Did you end up using this step? Looks like it always passes? There was a problem hiding this comment. Ah this is where it's just lack of context into the addon we use for our acceptance tests. The addon requires these files in order to run (the idea is you write a entire set of steps for every test, which seems like a lot of repetitive work). Instead we have about 25-30 reusable steps that we use for all our tests. Unfortunately the addon requires these files to still exist and generates them exactly like this for each and every acceptance test. If you delete them it breaks. Therefore we have oodles of these files in a nested folder structure that are all exactly the same. It's annoying, but I just pretend they aren't there. We had a PR a while ago to stop it from doing this which made it much nicer, but unfortunately it didn't go in and has been the root of confusion both previous to that PR and ever since 😹 There was a problem hiding this comment. Yikes, that isn’t ideal. Still, explanation makes sense for the context of this PR. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line here was the bugfix, previously the
readonlyvariable just below on line 63 was undefined always 😬Most of the other change here is to test this so it doesn't happen again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe an opportunity to use
cannot?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the codebase we prefer using
not (can "thing"), that way folks always use the same method to flip the conditional no matter if it's usingcanor some other flippable helper. So I'll probably leave this one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough