New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #27783 - CVV status error in default CV #8331
Conversation
Issues: #27783 |
ace4ccd
to
70ad386
Compare
@@ -63,6 +63,10 @@ angular.module('Bastion.capsule-content').controller('CapsuleContentController', | |||
} | |||
} | |||
|
|||
$scope.fetchUrl = function (cvId) { | |||
return cvId === 1 ? '/products' : '/content_views/' + cvId + '/versions'; |
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.
I think i mentioned this on irc, but putting it here: Checking for id==1 will only work for the first org :) there should be a 'default' attribute you can use
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.
oh gotcha, didn't know it would only work for the first organization. I'll update.
70ad386
to
6187a7f
Compare
@jlsherrill should be all set now. |
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.
Works as intended, but left a couple notes
describe('fetchUrl', function() { | ||
|
||
it('returns the correct url if default CV', function() { | ||
cvId = 1 |
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.
Looks like the cvId is inconsequential so I think it's better to just pass some random value to the method call in the expectation in this case
@@ -28,7 +28,7 @@ | |||
</tr> | |||
<tr ng-repeat-end ng-repeat="cv in env.content_views" ng-show="isEnvronmentExpanded(env)"> | |||
<td> | |||
<a href="/content_views/{{cv.id}}/versions" target="_self"> | |||
<a href="{{ fetchUrl(cv.default, cv.id) }}" target="_self"> |
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.
I think the name fetchUrl is a bit misleading since it doesn't fetch anything. Something like contentViewVersionUrl or productsOrVersionUrl are a bit clearer, or something else :)
Closing in favor of #8337 being merged. thanks @ianballou ! |
The user should not be able to browse to the Default Content View since it isn't meant to be interacted with. It should only be manually accessed via URL when needed for debugging.