Skip to content
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

Prevent user without permissions from seeing service as a clickable in context selector #304

Merged
merged 5 commits into from Nov 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 22 additions & 7 deletions app/javascript/src/Navigation/components/ContextSelector.jsx
Expand Up @@ -44,18 +44,33 @@ class ContextSelector extends React.Component {
)
}

getClassNamesFor ({ menu, api }) {
const { activeMenu, currentApi } = this.props
getClassNamesForMenu (menu) {
const { activeMenu } = this.props

if (menu === 'dashboard' && activeMenu === 'dashboard' ||
menu === 'audience' && (['buyers', 'finance', 'cms', 'site'].indexOf(activeMenu) !== -1) ||
api && (['serviceadmin', 'monitoring'].indexOf(activeMenu) !== -1) && api === currentApi.service.id) {
menu === 'audience' && (['buyers', 'finance', 'cms', 'site'].indexOf(activeMenu) !== -1)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe extract to an array
const menuNames = ['buyers', 'finance', 'cms', 'site'];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! This actually gives me some ideas for a better refactoring, I will address it in a different PR.

return 'PopNavigation-link current-context'
}

return 'PopNavigation-link'
}

getClassNamesForService (service) {
const { activeMenu, currentApi } = this.props
let classNames = 'PopNavigation-link'

if (['serviceadmin', 'monitoring'].indexOf(activeMenu) !== -1 &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here:
const servicesNames = ['serviceadmin', 'monitoring'];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read comment above.

service.id === currentApi.service.id) {
classNames += ' current-context'
}

if (!service.link) {
classNames += ' unauthorized'
}

return classNames
}

renderOptions () {
const { apis } = this.props
const { filterQuery } = this.state
Expand All @@ -67,7 +82,7 @@ class ContextSelector extends React.Component {

const displayedApis = filteredApis.map(({ service }) => (
<li key={service.id} className="PopNavigation-listItem">
<a className={this.getClassNamesFor({ api: service.id })} href={service.link}>
<a className={this.getClassNamesForService(service)} href={service.link}>
<i className="fa fa-puzzle-piece" />{service.name}
</a>
</li>
Expand All @@ -92,13 +107,13 @@ class ContextSelector extends React.Component {
</a>
<ul id="context-menu" className="PopNavigation-list u-toggleable">
<li className="PopNavigation-listItem">
<a className={this.getClassNamesFor({ menu: 'dashboard' })} href={DASHBOARD_PATH}>
<a className={this.getClassNamesForMenu('dashboard')} href={DASHBOARD_PATH}>
<i className='fa fa-home' />Dashboard
</a>
</li>
{audienceLink ? (
<li className="PopNavigation-listItem">
<a className={this.getClassNamesFor({ menu: 'audience' })} href={audienceLink}>
<a className={this.getClassNamesForMenu('audience')} href={audienceLink}>
<i className='fa fa-bullseye' />Audience
</a>
</li>
Expand Down
Expand Up @@ -12,7 +12,7 @@ function getWrapper (apis = [], audienceLink) {
let contextSelector

const apis = [
{ service: { name: 'api 0', id: 0 } },
{ service: { name: 'api 0', id: 0, link: 'foo.bar' } },
{ service: { name: 'api 1', id: 1 } },
{ service: { name: 'api 2', id: 2 } }
]
Expand Down Expand Up @@ -96,6 +96,14 @@ describe('When there is only 1 service', () => {
expect(api.exists()).toEqual(true)
expect(api.text()).toEqual('api 0')
})

it('should mark the api as unauthorized when link is undefined', () => {
contextSelector = getWrapper(apis.slice(0, 1), audienceLink)
expect(contextSelector.find('.unauthorized')).toHaveLength(0)

contextSelector = getWrapper(apis.slice(1, 2), audienceLink)
expect(contextSelector.find('.unauthorized')).toHaveLength(1)
})
})

describe('When there are many services', () => {
Expand Down Expand Up @@ -139,4 +147,9 @@ describe('When there are many services', () => {
input.simulate('change', { target: { value: 'wubba lubba dub dub' } })
expect(contextSelector.find('.PopNavigation-results').children()).toHaveLength(0)
})

it('should mark apis as unauthorized when link is undefined', () => {
const apiList = contextSelector.find('.PopNavigation-results').children()
expect(apiList.find('.unauthorized')).toHaveLength(2)
})
})
9 changes: 9 additions & 0 deletions app/javascript/src/Navigation/styles/ContextSelector.scss
@@ -1,10 +1,19 @@
// Copied from colors.scss
$lochMaraBlue: #0088ce; //pf-blue-400
$font-color: #393f44; // pf-black-800

.current-context {
color: $lochMaraBlue;
}

.PopNavigation-link > i {
padding-right: 0.5rem;
}

.unauthorized {
cursor: not-allowed;

&:hover {
color: $font-color;
}
}
2 changes: 0 additions & 2 deletions public/packs/dashboard-86117603365e13a08dda.js

This file was deleted.

Binary file removed public/packs/dashboard-86117603365e13a08dda.js.gz
Binary file not shown.
1 change: 0 additions & 1 deletion public/packs/dashboard-86117603365e13a08dda.js.map

This file was deleted.

2 changes: 2 additions & 0 deletions public/packs/dashboard-a2a5eb3eb48bf6d8aeb9.js

Large diffs are not rendered by default.

Binary file added public/packs/dashboard-a2a5eb3eb48bf6d8aeb9.js.gz
Binary file not shown.
1 change: 1 addition & 0 deletions public/packs/dashboard-a2a5eb3eb48bf6d8aeb9.js.map

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions public/packs/manifest.json
Expand Up @@ -4,10 +4,10 @@
"assets/glyphicons-halflings-regular.ttf": "/packs/assets/glyphicons-halflings-regular.e18bbf611f2a2e43afc071aa2f4e1512.ttf",
"assets/glyphicons-halflings-regular.woff": "/packs/assets/glyphicons-halflings-regular.fa2772327f55d8198301fdb8bcfc8158.woff",
"assets/glyphicons-halflings-regular.woff2": "/packs/assets/glyphicons-halflings-regular.448c34a56d699c29117adc64c43affeb.woff2",
"dashboard.js": "/packs/dashboard-86117603365e13a08dda.js",
"dashboard.js.map": "/packs/dashboard-86117603365e13a08dda.js.map",
"navigation.js": "/packs/navigation-f677a665c697929ee4e7.js",
"navigation.js.map": "/packs/navigation-f677a665c697929ee4e7.js.map",
"policies.js": "/packs/policies-2ee63d40bbeb2d7617c7.js",
"policies.js.map": "/packs/policies-2ee63d40bbeb2d7617c7.js.map"
"dashboard.js": "/packs/dashboard-a2a5eb3eb48bf6d8aeb9.js",
"dashboard.js.map": "/packs/dashboard-a2a5eb3eb48bf6d8aeb9.js.map",
"navigation.js": "/packs/navigation-7cdccc13141616d66596.js",
"navigation.js.map": "/packs/navigation-7cdccc13141616d66596.js.map",
"policies.js": "/packs/policies-c40172680fcd63c10cf8.js",
"policies.js.map": "/packs/policies-c40172680fcd63c10cf8.js.map"
}
Binary file modified public/packs/manifest.json.gz
Binary file not shown.
2 changes: 2 additions & 0 deletions public/packs/navigation-7cdccc13141616d66596.js

Large diffs are not rendered by default.

Binary file not shown.
1 change: 1 addition & 0 deletions public/packs/navigation-7cdccc13141616d66596.js.map

Large diffs are not rendered by default.

2 changes: 0 additions & 2 deletions public/packs/navigation-f677a665c697929ee4e7.js

This file was deleted.

Binary file removed public/packs/navigation-f677a665c697929ee4e7.js.gz
Binary file not shown.
1 change: 0 additions & 1 deletion public/packs/navigation-f677a665c697929ee4e7.js.map

This file was deleted.

2 changes: 0 additions & 2 deletions public/packs/policies-2ee63d40bbeb2d7617c7.js

This file was deleted.

Binary file removed public/packs/policies-2ee63d40bbeb2d7617c7.js.gz
Binary file not shown.
1 change: 0 additions & 1 deletion public/packs/policies-2ee63d40bbeb2d7617c7.js.map

This file was deleted.

2 changes: 2 additions & 0 deletions public/packs/policies-c40172680fcd63c10cf8.js

Large diffs are not rendered by default.

Binary file added public/packs/policies-c40172680fcd63c10cf8.js.gz
Binary file not shown.
1 change: 1 addition & 0 deletions public/packs/policies-c40172680fcd63c10cf8.js.map

Large diffs are not rendered by default.