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

Fixes #26753 - react-router foundation #3 #6741

Merged
merged 1 commit into from May 16, 2019
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
4 changes: 4 additions & 0 deletions app/assets/stylesheets/base.scss
Expand Up @@ -12,6 +12,10 @@ body {
margin-left: 200px;
padding-left: 20px;
padding-right: 20px;

&.collapsed-nav {
margin-left: 75px;
}
}

.base li {
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/react_controller.rb
@@ -1,3 +1,7 @@
class ReactController < ApplicationController
layout 'layouts/react_application'
end

def index
render 'react/index'
end
4 changes: 4 additions & 0 deletions app/registries/foreman/access_permissions.rb
Expand Up @@ -756,4 +756,8 @@
map.permission :edit_settings, { :settings => [:update],
:'api/v2/settings' => [:update] }
end

permission_set.security_block :react do |map|
map.permission :view_react, {:react => [:index]}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused, why is this needed?
Do we need special permission to see react pages or it should just be allowed to everyone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these changes are needed for a rake test to pass (permissions_test, seed_test)

Copy link
Member

Choose a reason for hiding this comment

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

They shouldn't be. We don't add permissions just so tests pass when they don't make sense from a user standpoint. opened https://projects.theforeman.org/issues/27333 to remove this.

end
end
Empty file added app/views/react/index.html.erb
Empty file.
3 changes: 2 additions & 1 deletion db/seeds.d/020-permissions_list.rb
Expand Up @@ -184,7 +184,8 @@ def permissions
['User', 'view_users'],
['User', 'create_users'],
['User', 'edit_users'],
['User', 'destroy_users']
['User', 'destroy_users'],
['React', 'view_react']
]
end
end
Expand Down
2 changes: 1 addition & 1 deletion webpack/assets/javascripts/hosts/tableCheckboxes.js
Expand Up @@ -5,7 +5,7 @@ import {
ngettext as n__,
translate as __,
} from '../react_app/common/I18n';
import { getURIsearch } from '../react_app/components/Pagination/PaginationHelper';
import { getURIsearch } from '../react_app/common/urlHelpers';
import { foremanUrl } from '../foreman_tools';

// Array contains list of host ids
Expand Down
@@ -0,0 +1,19 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`URI query and stringify tests should return stringified params: stringify params 1`] = `"?page=1&per_page=21"`;

exports[`URI query and stringify tests should return stringified params: stringify params w/search 1`] = `"?page=1&per_page=21&search=search"`;

exports[`URI query and stringify tests should test params functions: getPage 1`] = `1`;

exports[`URI query and stringify tests should test params functions: getParams 1`] = `
Object {
"page": 1,
"perPage": 25,
"searchQuery": "some-search",
}
`;

exports[`URI query and stringify tests should test params functions: getPerPage 1`] = `25`;

exports[`URI query and stringify tests should test params functions: getSearchQuery 1`] = `"some-search"`;
8 changes: 8 additions & 0 deletions webpack/assets/javascripts/react_app/common/document.js
Expand Up @@ -6,3 +6,11 @@ import { runningInPhantomJS } from './helpers';
*/
export const doesDocumentHasFocus = () =>
runningInPhantomJS() || (document.hasFocus ? document.hasFocus() : true);

/**
* Update title of document
* @param {String} title - the title
*/
export const updateDocumentTitle = title => {
document.title = title;
};
56 changes: 56 additions & 0 deletions webpack/assets/javascripts/react_app/common/urlHelpers.js
@@ -1,3 +1,5 @@
import URI from 'urijs';

/**
* Build a url from given controller, action and id
* @param {String} controller - the controller
Expand All @@ -13,3 +15,57 @@ export const urlBuilder = (controller, action, id = undefined) =>
*/
export const urlWithSearch = (base, searchQuery) =>
`/${base}?search=${searchQuery}`;

/**
* Get updated URI
*/
export const getURI = () => new URI(window.location.href);

/**
* Get updated page param
*/
export const getURIpage = () => Number(getURI().query(true).page) || 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

test?

/**
* Get updated perPage param
*/
export const getURIperPage = () => Number(getURI().query(true).per_page);
Copy link
Contributor

Choose a reason for hiding this comment

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

test?

/**
* Get updated searchQuery param
*/
export const getURIsearch = () => getURI().query(true).search || '';
Copy link
Contributor

Choose a reason for hiding this comment

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

test?


/**
* Get updated URI params
*/
export const getParams = () => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

test?

page: getURIpage(),
perPage: getURIperPage(),
searchQuery: getURIsearch(),
});

/**
* Get updated Stringified params
*/
export const stringifyParams = ({
page = 1,
perPage = 25,
searchQuery = '',
}) => {
const uri = getURI();
if (searchQuery !== '')
uri.search({ page, per_page: perPage, search: searchQuery });
else uri.search({ page, per_page: perPage });
return uri.search();
};

/**
* change current query and trigger navigation
* @param {URI} uri - URI object
* @param {Object} newQuery - Query Object
* @param {Function} navigateTo - navigate func
*/
export const changeQuery = (newQuery, navigateTo, uri = getURI()) => {
uri.setQuery(newQuery);
if (navigateTo) navigateTo(uri.toString());
else window.Turbolinks.visit(uri.toString());
};
61 changes: 60 additions & 1 deletion webpack/assets/javascripts/react_app/common/urlHelpers.test.js
@@ -1,4 +1,24 @@
import { urlBuilder, urlWithSearch } from './urlHelpers';
import {
urlBuilder,
urlWithSearch,
changeQuery,
stringifyParams,
getParams,
getURIpage,
getURIperPage,
getURIsearch,
} from './urlHelpers';

const mockWindow = ({ href, visit }) => {
const windowLocation = JSON.stringify(window.location);
delete window.location;
Object.defineProperty(window, 'location', {
value: JSON.parse(windowLocation),
});

window.location.href = href;
window.Turbolinks = { visit };
};

describe('urlBuilder', () => {
const controller = 'testController';
Expand All @@ -21,3 +41,42 @@ describe('urlWithSearch', () => {
expect(urlWithSearch(base, query)).toBe('/testBase?search=query=test');
});
});

describe('URI query and stringify tests', () => {
const visit = jest.fn();
const baseHref = 'http://some-url.com/';
const oldQuery = 'search=some-search&page=1&per_page=25';
const href = `${baseHref}?${oldQuery}`;

it('should resolve change-query', () => {
mockWindow({ href, visit });
const navigateToMock = jest.fn();
const newQuery = { search: 'some-new-search', per_page: 10 };

changeQuery(newQuery);
expect(visit).toBeCalledWith(
`${baseHref}?search=some-new-search&page=1&per_page=10`
);

changeQuery(newQuery, navigateToMock);
expect(navigateToMock).toHaveBeenCalled();
});

it('should return stringified params', () => {
const params = { page: 1, perPage: 21 };
const stringified = stringifyParams(params);
const stringifiedWithSearch = stringifyParams({
...params,
searchQuery: 'search',
});
expect(stringified).toMatchSnapshot('stringify params');
expect(stringifiedWithSearch).toMatchSnapshot('stringify params w/search');
});

it('should test params functions', () => {
expect(getParams()).toMatchSnapshot('getParams');
expect(getURIpage()).toMatchSnapshot('getPage');
expect(getURIperPage()).toMatchSnapshot('getPerPage');
expect(getURIsearch()).toMatchSnapshot('getSearchQuery');
});
});
Expand Up @@ -4,11 +4,10 @@ import { isEmpty } from 'lodash';
import { Paginator } from 'patternfly-react';
import { translateObject } from '../../common/helpers';
import {
getURI,
getURIpage,
getURIperPage,
changeQuery,
} from './PaginationHelper';
} from '../../common/urlHelpers';
import './pagination.scss';

const Pagination = props => {
Expand All @@ -32,7 +31,7 @@ const Pagination = props => {
pagination={
isEmpty(pagination)
? {
page: urlPage || 1,
page: urlPage,
perPage: urlPerPage || data.perPage,
perPageOptions: data.perPageOptions,
}
Expand Down Expand Up @@ -68,9 +67,8 @@ Pagination.propTypes = {
};

Pagination.defaultProps = {
onPageSet: page => changeQuery(getURI(), { page }),
onPerPageSelect: perPage =>
changeQuery(getURI(), { page: 1, per_page: perPage }),
onPageSet: page => changeQuery({ page }),
onPerPageSelect: perPage => changeQuery({ page: 1, per_page: perPage }),
dropdownButtonId: 'pagination-row-dropdown',
pagination: null,
};
Expand Down

This file was deleted.

This file was deleted.

Expand Up @@ -3,17 +3,21 @@ import isEmpty from 'lodash/isEmpty';
import PropTypes from 'prop-types';
import AutoComplete from '../AutoComplete';
import Bookmarks from '../bookmarks';
import { resolveSearchQuery } from './SearchBarHelpers';
import { changeQuery } from '../../common/urlHelpers';
import './search-bar.scss';

const SearchBar = ({
data: { autocomplete, controller, bookmarks },
searchQuery,
onSearch,
initialQuery,
onBookmarkClick,
}) => {
const bookmarksComponent = !isEmpty(bookmarks) ? (
<Bookmarks data={{ ...bookmarks, controller, searchQuery }} />
<Bookmarks
data={{ ...bookmarks, controller, searchQuery }}
onBookmarkClick={onBookmarkClick}
/>
) : null;
return (
<div className="search-bar input-group">
Expand All @@ -37,6 +41,7 @@ SearchBar.propTypes = {
searchQuery: PropTypes.string,
initialQuery: PropTypes.string,
onSearch: PropTypes.func,
onBookmarkClick: PropTypes.func,
data: PropTypes.shape({
autocomplete: PropTypes.shape({
results: PropTypes.array,
Expand All @@ -53,7 +58,9 @@ SearchBar.propTypes = {
SearchBar.defaultProps = {
searchQuery: '',
initialQuery: '',
onSearch: resolveSearchQuery,
onSearch: searchQuery => changeQuery({ search: searchQuery.trim(), page: 1 }),
onBookmarkClick: searchQuery =>
changeQuery({ search: searchQuery.trim(), page: 1 }),
data: {
autocomplete: {
results: [],
Expand Down

This file was deleted.

This file was deleted.

Expand Up @@ -31,6 +31,7 @@ exports[`AutoComplete rendering renders AutoComplete 1`] = `
"url": "/api/bookmarks",
}
}
onBookmarkClick={[Function]}
/>
</div>
</div>
Expand Down