From a58468ffeef015d898c1530264e0122ffcc1b5d8 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 23 Jul 2019 15:39:30 -0400 Subject: [PATCH 1/8] initial implementation of search in UI_next --- awx/ui_next/SEARCH.md | 255 +++ awx/ui_next/src/api/Base.js | 23 +- awx/ui_next/src/api/Base.test.jsx | 49 +- .../src/api/mixins/InstanceGroups.mixin.js | 30 +- .../src/api/mixins/Notifications.mixin.js | 151 +- awx/ui_next/src/api/models/Organizations.js | 6 +- .../src/api/models/Organizations.test.jsx | 57 +- .../components/AddRole/AddResourceRole.jsx | 79 +- .../components/AddRole/SelectResourceStep.jsx | 4 +- .../AddRole/SelectResourceStep.test.jsx | 41 +- awx/ui_next/src/components/Chip/ChipGroup.jsx | 21 +- .../DataListToolbar/DataListToolbar.test.jsx | 56 +- .../src/components/FilterTags/FilterTags.jsx | 80 + .../components/FilterTags/FilterTags.test.jsx | 42 + .../src/components/FilterTags/index.js | 1 + .../src/components/ListHeader/ListHeader.jsx | 140 ++ .../components/ListHeader/ListHeader.test.jsx | 56 + .../src/components/ListHeader/index.js | 1 + awx/ui_next/src/components/Lookup/Lookup.jsx | 4 +- .../PaginatedDataList/PaginatedDataList.jsx | 202 +- .../PaginatedDataList.test.jsx | 90 +- awx/ui_next/src/components/Search/Search.jsx | 149 +- .../src/components/Search/Search.test.jsx | 34 +- awx/ui_next/src/components/Sort/Sort.test.jsx | 18 +- .../src/screens/Job/JobList/JobList.jsx | 74 +- .../OrganizationAccess/OrganizationAccess.jsx | 106 +- .../OrganizationAccess.test.jsx | 2 +- .../OrganizationAccess.test.jsx.snap | 240 ++- .../OrganizationAccessItem.test.jsx.snap | 9 +- .../OrganizationList/OrganizationList.jsx | 104 +- .../OrganizationNotifications.jsx | 60 +- .../OrganizationNotifications.test.jsx.snap | 1851 +++++++++-------- .../OrganizationTeams/OrganizationTeams.jsx | 4 +- .../shared/InstanceGroupsLookup.jsx | 42 +- .../Template/TemplateList/TemplateList.jsx | 112 +- awx/ui_next/src/util/qs.js | 403 +++- awx/ui_next/src/util/qs.test.js | 578 +++-- 37 files changed, 3166 insertions(+), 2008 deletions(-) create mode 100644 awx/ui_next/SEARCH.md create mode 100644 awx/ui_next/src/components/FilterTags/FilterTags.jsx create mode 100644 awx/ui_next/src/components/FilterTags/FilterTags.test.jsx create mode 100644 awx/ui_next/src/components/FilterTags/index.js create mode 100644 awx/ui_next/src/components/ListHeader/ListHeader.jsx create mode 100644 awx/ui_next/src/components/ListHeader/ListHeader.test.jsx create mode 100644 awx/ui_next/src/components/ListHeader/index.js diff --git a/awx/ui_next/SEARCH.md b/awx/ui_next/SEARCH.md new file mode 100644 index 0000000000..0035213446 --- /dev/null +++ b/awx/ui_next/SEARCH.md @@ -0,0 +1,255 @@ +# Search Iteration 1 Requirements: + +## DONE + +- DONE update handleSearch to follow handleSort param +- DONE update qsConfig columns to utilize isSearchable bool (just like isSortable bool) +- DONE enter keydown in text search bar to search +- DONE get decoded params and write test +- DONE make list header component +- DONE make filter component +- DONE make filters show up for empty list +- DONE make clear all button +- DONE styling of FilterTags component +- DONE clear out text input after tag has been made +- DONE deal with duplicate key tags being added/removed in qs util file +- DONE deal with widgetry changing between one dropdown option to the left of search and many +- DONE bug: figure out why ?name=org returning just org not “org 2” +- DONE update contrib file to have the first section with updated text as is in this pr description. +- DONE rebase with latest awx-pf changes +- DONE styling of search bar +- DONE make filter and list header tests +- DONE change api paramsSerializer to handle duplicate key stuff +- DONE update qs update function to be smaller, simple param functions, as opposed to one big one with a lot of params + +## TODO for this PR + +- add search filter removal test for qs. +- bug: remove button for search tags of duplicate keys are broken, fix that + +## TODO later on in 3.6: stuff to be finished for search iteration 1 (I'll card up an issue to tackle this. I plan on doing this after I finished the awx project branch work) + +- currently handleSearch in Search.jsx always appends the __icontains post-fix to make the filtering ux expected work right. Once we start adding number-based params we will won't to change this behavior. +- utilize new defaultSearchKey prop instead of relying on sort key +- make access have username as the default key? +- make default params only accept page, page_size and order_by +- support custom order_by being typed in the url bar +- fix up which keys are displayed in the various lists (note this will also require non-string widgetry to the right of the search key dropdown, for integers, dates, etc.) + +## Lists affected in 3.6 timeframe + +We should update all places to use consistent handleSearch/handleSort with paginated data list pattern. This shouldn't be too difficult to get hooked up, as the lists all inherit from PaginatedDataList, where search is hooked up. We will need to make sure the queryset config for each list includes the searchable boolean on keys that will need to be searched for. + +orgs stuff + - org list + - org add/edit instance groups lookup list + - org access list + - org user/teams list in wizard + - org teams list + - org notifications list +jt stuff + - jt list + - jt add/edit inventory, project, credentials, instance groups lookups lists + - jt access list + - jt user/teams list in wizard + - jt notifications list + - jt schedules list + - jt completed jobs list +jobs stuff + - jobs list + +# Search code details + +## Search component + +Search is configured using the qsConfig in a similar way to sort. Columns are passed as an array, as defined in the screen where the list is located. You pass a bool isSearchable (an analog to isSortable) to mark that a certain key should show up in the left-hand dropdown of the search bar. + +If you don't pass any columns, a default of isSearchable true will be added to a name column, which is nearly universally shared throughout the models of awx. + +The component looks like this: + +``` + +``` + +## ListHeader component + +DataListToolbar, EmptyListControls, and FilterTags components were created/moved to a new sub-component of PaginatedDataList, ListHeader. This allowed us to consolidate the logic between both lists with data (which need to show search, sort, any search tags currently active, and actions) as well as empty lists (which need to show search tags currently active so they can be removed, potentially getting you back to a "list-has-data" state, as well as a subset of options still valid (such as "add"). + +search and sort are passed callbacks from functions defined in ListHeader. These will be the following. + +``` +handleSort (sortedColumnKey, sortOrder) { + this.pushHistoryState({ + order_by: sortOrder === 'ascending' ? sortedColumnKey : `-${sortedColumnKey}`, + page: null, + }); +} + +handleSearch (key, value, remove) { + this.pushHistoryState({ + // ... use key and value to push a new value to the param + // if remove false you add a new tag w key value if remove true, + // you are removing one + }); +} +``` + +Similarly, there are handleRemove and handleRemoveAll functions. All of these functions act on the react-router history using the pushHistoryState function. This causes the query params in the url to update, which in turn triggers change handlers that will re-fetch data. + +## FilterTags component + +Similar to the way the list grabs data based on changes to the react-router params, the FilterTags component updates when new params are added. This component is a fairly straight-forward map (only slightly complex, because it needed to do a nested map over any values with duplicate keys that were represented by an inner-array). + +Currently the filter tags do not display the key, though that data is available and they could very easily do so. + +Some small changes were made to our custom ChipGroup component to make this possible--a new displayAll bool prop which bypasses all chip hiding based on number of chips. + +## QS Updates (and supporting duplicate keys) + +The logic that was updated to handle search tags can be found in the qs.js util file. + +From a UX perspective, we wanted to be able to support searching on the same key multiple times (i.e. searching for things like ?foo=bar&foo=baz). We do this by creating an array of all values. i.e.: + +``` +{ + foo: ['bar', 'baz'] +} +``` + +Changes to encodeQueryString and parseQueryString were made to convert between a single value string representation and multiple value array representations. Test cases were also added to qs.test.js. + +In addition, we needed to make sure any changes to the params that are not handled by search (page, page_size, and order_by) were updated by replacing the single value, rather than adding multiple values with the array representation. This additional piece of the specification was made in the newly created addParams and removeParams qs functions and a few test-cases were written to verify this. + +The api is coupled with the qs util through the paramsSerializer, due to the fact we need axios to support the array for duplicate key values object representation of the params to pass to the get request. This is done where axios is configured in the Base.js file, so all requests and request types should support our array syntax for duplicate keys. + +# UX considerations + +**UX should be more tags always equates to more filtering. (so "and" logic not "or")** + +Also, for simple search results should be returned that partially match value (i.e. use icontains prefix) + +**ALL query params namespaced and in url bar** + + - this includes lists that aren't necessarily hyperlinked, like lookup lists. + - the reason behind this is so we can treat the url bar as the source of truth for queries always + - currently /#/organizations/add?lookup.name=bar -> will eventually be something like /#/organizations/add?ig_lookup.name=bar + - any params that have both a key AND value that is in the defaultParams section of the qs config should be stripped out of the search string + +**django fuzzy search (?search=) is not accessible outside of "advanced search"** + + - How "search" query param works + - in current smart search typing a term with no key utilizes search= i.e. for "foo" tag, ?search=foo is given + - search= looks on a static list of field name "guesses" (such as name, description, etc.), as well as specific fields as defined for each endpoint (for example, the events endpoint looks for a "stdout" field as well) + - note that search= tags are OR'd together + - search=foo&name=bar returns items that have a name field of bar (not case insensitive) AND some text field with foo on it + - search=foo&search=bar&name=baz returns (foo in name OR foo in description OR ...) AND (bar in name OR bar in description OR ...) AND (baz in name) + - similarly ?related__search= looks on the static list of "guesses" for models related to the endpoint + - the specific fields are not "searched" for related__search + - related__search not currently used in awx ui + +**a note on typing in a smart search query** + +In order to not support a special "language" or "syntax" for crafting the query like we have now (and is the cause of a large amount of bugs), we will not support the old way of typing in a filter like in the current implementation of search. + +Since all search bars are represented in the url, for users who want to input a string to filter results in a single step, typing directly in the url to achieve the filter is acceptable. + +**a note on clicking a tag to putting it back into the search bar** + +This was brought up as a nice to have when we were discussing features. There isn't a way we would be able to know if the user created the tag from the smart search or simple search interface? that info is not traceable using the query params as the exclusive source of truth + +We have decided to not try to tackle this up front with our advanced search implementation, and may go back to this based on user feedback at a later time. + +# Advanced search notes + +Current thinking is Advanced Search will be post-3.6, or at least late 3.6 after awx features and "simple search" with the left dropdown and right input for the above phase 1 lists. + +That being said, we want to plan it out so we make sure the infrastructure of how we set up adding/removing tags, what shows up in the url bar, etc. all doesn't have to be redone. + +Users will get to advanced search with a button to the right of search bar. When selected type-ahead key thing opens, left dropdown of search bar goes away, and x is given to get back to regular search (this is in the mockups) + +It is okay to only make this typing representation available initially (i.e. they start doing stuff with the type-ahead and the phases, no more typing in to make a query that way). + +when you click through or type in the search bar for the various phases of crafting the query ("not", "related resource project", "related resource key name", "value foo") which might be represented in the top bar as a series of tags that can be added and removed before submitting the tag. + +We will try to form options data from a static file. Because options data is static, we may be able to generate and store as a static file of some sort (that we can use for managing smart search). Alan had ideas around this. If we do this it will mean we don't have to make a ton of requests as we craft smart search filters. It sounds like tower cli may start using something similar. + +## Smart search flow + +Smart search will be able to craft the tag through various states. Note that the phases don't necessarily need to be completed in sequential order. + + PHASE 1: prefix operators + +**TODO: Double check there's no reason we need to include or__ and chain__ and can just do not__** + + - not__ + - or__ + - chain__ + + how these work: + + To exclude results matching certain criteria, prefix the field parameter with not__: + + ?not__field=value + By default, all query string filters are AND'ed together, so only the results matching all filters will be returned. To combine results matching any one of multiple criteria, prefix each query string parameter with or__: + + ?or__field=value&or__field=othervalue + ?or__not__field=value&or__field=othervalue + (Added in Ansible Tower 1.4.5) The default AND filtering applies all filters simultaneously to each related object being filtered across database relationships. The chain filter instead applies filters separately for each related object. To use, prefix the query string parameter with chain__: + + ?chain__related__field=value&chain__related__field2=othervalue + ?chain__not__related__field=value&chain__related__field2=othervalue + If the first query above were written as ?related__field=value&related__field2=othervalue, it would return only the primary objects where the same related object satisfied both conditions. As written using the chain filter, it would return the intersection of primary objects matching each condition. + + PHASE 2: related fields, given by array, where __search is appended to them, i.e. + + ``` + "related_search_fields": [ + "credentials__search", + "labels__search", + "created_by__search", + "modified_by__search", + "notification_templates__search", + "custom_inventory_scripts__search", + "notification_templates_error__search", + "notification_templates_success__search", + "notification_templates_any__search", + "teams__search", + "projects__search", + "inventories__search", + "applications__search", + "workflows__search", + "instance_groups__search" + ], + ``` + + PHASE 3: keys, give by object key names for data.actions.GET + - type is given for each key which we could use to help craft the value + + PHASE 4: after key postfix operators can be + +**TODO: will need to figure out which ones we support** + + - exact: Exact match (default lookup if not specified). + - iexact: Case-insensitive version of exact. + - contains: Field contains value. + - icontains: Case-insensitive version of contains. + - startswith: Field starts with value. + - istartswith: Case-insensitive version of startswith. + - endswith: Field ends with value. + - iendswith: Case-insensitive version of endswith. + - regex: Field matches the given regular expression. + - iregex: Case-insensitive version of regex. + - gt: Greater than comparison. + - gte: Greater than or equal to comparison. + - lt: Less than comparison. + - lte: Less than or equal to comparison. + - isnull: Check whether the given field or related object is null; expects a boolean value. + - in: Check whether the given field's value is present in the list provided; expects a list of items. + + PHASE 5: The value. Based on options, we can give hints or validation based on type of value (like number fields don't accept "foo" or whatever) \ No newline at end of file diff --git a/awx/ui_next/src/api/Base.js b/awx/ui_next/src/api/Base.js index d884f8061c..a5f62d0cd9 100644 --- a/awx/ui_next/src/api/Base.js +++ b/awx/ui_next/src/api/Base.js @@ -1,41 +1,48 @@ import axios from 'axios'; +import { + encodeQueryString +} from '@util/qs'; + const defaultHttp = axios.create({ xsrfCookieName: 'csrftoken', xsrfHeaderName: 'X-CSRFToken', + paramsSerializer(params) { + return encodeQueryString(params); + } }); class Base { - constructor(http = defaultHttp, baseURL) { + constructor (http = defaultHttp, baseURL) { this.http = http; this.baseUrl = baseURL; } - create(data) { + create (data) { return this.http.post(this.baseUrl, data); } - destroy(id) { + destroy (id) { return this.http.delete(`${this.baseUrl}${id}/`); } - read(params = {}) { + read (params) { return this.http.get(this.baseUrl, { params }); } - readDetail(id) { + readDetail (id) { return this.http.get(`${this.baseUrl}${id}/`); } - readOptions() { + readOptions () { return this.http.options(this.baseUrl); } - replace(id, data) { + replace (id, data) { return this.http.put(`${this.baseUrl}${id}/`, data); } - update(id, data) { + update (id, data) { return this.http.patch(`${this.baseUrl}${id}/`, data); } } diff --git a/awx/ui_next/src/api/Base.test.jsx b/awx/ui_next/src/api/Base.test.jsx index df6d874bd0..53246dd80a 100644 --- a/awx/ui_next/src/api/Base.test.jsx +++ b/awx/ui_next/src/api/Base.test.jsx @@ -3,14 +3,14 @@ import Base from './Base'; describe('Base', () => { const createPromise = () => Promise.resolve(); const mockBaseURL = '/api/v2/organizations/'; - const mockHttp = { + const mockHttp = ({ delete: jest.fn(createPromise), get: jest.fn(createPromise), options: jest.fn(createPromise), patch: jest.fn(createPromise), post: jest.fn(createPromise), - put: jest.fn(createPromise), - }; + put: jest.fn(createPromise) + }); const BaseAPI = new Base(mockHttp, mockBaseURL); @@ -18,7 +18,7 @@ describe('Base', () => { jest.clearAllMocks(); }); - test('create calls http method with expected data', async done => { + test('create calls http method with expected data', async (done) => { const data = { name: 'test ' }; await BaseAPI.create(data); @@ -28,44 +28,45 @@ describe('Base', () => { done(); }); - test('destroy calls http method with expected data', async done => { + test('destroy calls http method with expected data', async (done) => { const resourceId = 1; await BaseAPI.destroy(resourceId); expect(mockHttp.delete).toHaveBeenCalledTimes(1); - expect(mockHttp.delete.mock.calls[0][0]).toEqual( - `${mockBaseURL}${resourceId}/` - ); + expect(mockHttp.delete.mock.calls[0][0]).toEqual(`${mockBaseURL}${resourceId}/`); done(); }); - test('read calls http method with expected data', async done => { - const defaultParams = {}; + test('read calls http method with expected data', async (done) => { const testParams = { foo: 'bar' }; + const testParamsDuplicates = { foo: ['bar', 'baz']}; await BaseAPI.read(testParams); await BaseAPI.read(); + await BaseAPI.read(testParamsDuplicates); - expect(mockHttp.get).toHaveBeenCalledTimes(2); - expect(mockHttp.get.mock.calls[0][1]).toEqual({ params: testParams }); - expect(mockHttp.get.mock.calls[1][1]).toEqual({ params: defaultParams }); + expect(mockHttp.get).toHaveBeenCalledTimes(3); + expect(mockHttp.get.mock.calls[0][0]).toEqual(`${mockBaseURL}`); + expect(mockHttp.get.mock.calls[0][1]).toEqual({"params": {"foo": "bar"}}); + expect(mockHttp.get.mock.calls[1][0]).toEqual(`${mockBaseURL}`); + expect(mockHttp.get.mock.calls[1][1]).toEqual({"params": undefined}); + expect(mockHttp.get.mock.calls[2][0]).toEqual(`${mockBaseURL}`); + expect(mockHttp.get.mock.calls[2][1]).toEqual({"params": {"foo": ["bar", "baz"]}}); done(); }); - test('readDetail calls http method with expected data', async done => { + test('readDetail calls http method with expected data', async (done) => { const resourceId = 1; await BaseAPI.readDetail(resourceId); expect(mockHttp.get).toHaveBeenCalledTimes(1); - expect(mockHttp.get.mock.calls[0][0]).toEqual( - `${mockBaseURL}${resourceId}/` - ); + expect(mockHttp.get.mock.calls[0][0]).toEqual(`${mockBaseURL}${resourceId}/`); done(); }); - test('readOptions calls http method with expected data', async done => { + test('readOptions calls http method with expected data', async (done) => { await BaseAPI.readOptions(); expect(mockHttp.options).toHaveBeenCalledTimes(1); @@ -73,31 +74,27 @@ describe('Base', () => { done(); }); - test('replace calls http method with expected data', async done => { + test('replace calls http method with expected data', async (done) => { const resourceId = 1; const data = { name: 'test ' }; await BaseAPI.replace(resourceId, data); expect(mockHttp.put).toHaveBeenCalledTimes(1); - expect(mockHttp.put.mock.calls[0][0]).toEqual( - `${mockBaseURL}${resourceId}/` - ); + expect(mockHttp.put.mock.calls[0][0]).toEqual(`${mockBaseURL}${resourceId}/`); expect(mockHttp.put.mock.calls[0][1]).toEqual(data); done(); }); - test('update calls http method with expected data', async done => { + test('update calls http method with expected data', async (done) => { const resourceId = 1; const data = { name: 'test ' }; await BaseAPI.update(resourceId, data); expect(mockHttp.patch).toHaveBeenCalledTimes(1); - expect(mockHttp.patch.mock.calls[0][0]).toEqual( - `${mockBaseURL}${resourceId}/` - ); + expect(mockHttp.patch.mock.calls[0][0]).toEqual(`${mockBaseURL}${resourceId}/`); expect(mockHttp.patch.mock.calls[0][1]).toEqual(data); done(); diff --git a/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js b/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js index 3deff44bf7..a4fba3cc0e 100644 --- a/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js +++ b/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js @@ -1,23 +1,15 @@ -const InstanceGroupsMixin = parent => - class extends parent { - readInstanceGroups(resourceId, params = {}) { - return this.http.get(`${this.baseUrl}${resourceId}/instance_groups/`, { - params, - }); - } +const InstanceGroupsMixin = (parent) => class extends parent { + readInstanceGroups (resourceId, params) { + return this.http.get(`${this.baseUrl}${resourceId}/instance_groups/`, params); + } - associateInstanceGroup(resourceId, instanceGroupId) { - return this.http.post(`${this.baseUrl}${resourceId}/instance_groups/`, { - id: instanceGroupId, - }); - } + associateInstanceGroup (resourceId, instanceGroupId) { + return this.http.post(`${this.baseUrl}${resourceId}/instance_groups/`, { id: instanceGroupId }); + } - disassociateInstanceGroup(resourceId, instanceGroupId) { - return this.http.post(`${this.baseUrl}${resourceId}/instance_groups/`, { - id: instanceGroupId, - disassociate: true, - }); - } - }; + disassociateInstanceGroup (resourceId, instanceGroupId) { + return this.http.post(`${this.baseUrl}${resourceId}/instance_groups/`, { id: instanceGroupId, disassociate: true }); + } +}; export default InstanceGroupsMixin; diff --git a/awx/ui_next/src/api/mixins/Notifications.mixin.js b/awx/ui_next/src/api/mixins/Notifications.mixin.js index eccadc5dc2..f7c934a0c8 100644 --- a/awx/ui_next/src/api/mixins/Notifications.mixin.js +++ b/awx/ui_next/src/api/mixins/Notifications.mixin.js @@ -1,106 +1,65 @@ -const NotificationsMixin = parent => - class extends parent { - readOptionsNotificationTemplates(id) { - return this.http.options(`${this.baseUrl}${id}/notification_templates/`); +const NotificationsMixin = (parent) => class extends parent { + readOptionsNotificationTemplates(id) { + return this.http.options(`${this.baseUrl}${id}/notification_templates/`); + } + + readNotificationTemplates (id, params) { + return this.http.get(`${this.baseUrl}${id}/notification_templates/`, params); + } + + readNotificationTemplatesSuccess (id, params) { + return this.http.get(`${this.baseUrl}${id}/notification_templates_success/`, params); + } + + readNotificationTemplatesError (id, params) { + return this.http.get(`${this.baseUrl}${id}/notification_templates_error/`, params); + } + + associateNotificationTemplatesSuccess (resourceId, notificationId) { + return this.http.post(`${this.baseUrl}${resourceId}/notification_templates_success/`, { id: notificationId }); + } + + disassociateNotificationTemplatesSuccess (resourceId, notificationId) { + return this.http.post(`${this.baseUrl}${resourceId}/notification_templates_success/`, { id: notificationId, disassociate: true }); + } + + associateNotificationTemplatesError (resourceId, notificationId) { + return this.http.post(`${this.baseUrl}${resourceId}/notification_templates_error/`, { id: notificationId }); + } + + disassociateNotificationTemplatesError (resourceId, notificationId) { + return this.http.post(`${this.baseUrl}${resourceId}/notification_templates_error/`, { id: notificationId, disassociate: true }); + } + + /** + * This is a helper method meant to simplify setting the "on" or "off" status of + * a related notification. + * + * @param[resourceId] - id of the base resource + * @param[notificationId] - id of the notification + * @param[notificationType] - the type of notification, options are "success" and "error" + * @param[associationState] - Boolean for associating or disassociating, options are true or false + */ + // eslint-disable-next-line max-len + updateNotificationTemplateAssociation (resourceId, notificationId, notificationType, associationState) { + if (notificationType === 'success' && associationState === true) { + return this.associateNotificationTemplatesSuccess(resourceId, notificationId); } - readNotificationTemplates(id, params = {}) { - return this.http.get(`${this.baseUrl}${id}/notification_templates/`, { - params, - }); + if (notificationType === 'success' && associationState === false) { + return this.disassociateNotificationTemplatesSuccess(resourceId, notificationId); } - readNotificationTemplatesSuccess(id, params = {}) { - return this.http.get( - `${this.baseUrl}${id}/notification_templates_success/`, - { params } - ); + if (notificationType === 'error' && associationState === true) { + return this.associateNotificationTemplatesError(resourceId, notificationId); } - readNotificationTemplatesError(id, params = {}) { - return this.http.get( - `${this.baseUrl}${id}/notification_templates_error/`, - { params } - ); + if (notificationType === 'error' && associationState === false) { + return this.disassociateNotificationTemplatesError(resourceId, notificationId); } - associateNotificationTemplatesSuccess(resourceId, notificationId) { - return this.http.post( - `${this.baseUrl}${resourceId}/notification_templates_success/`, - { id: notificationId } - ); - } - - disassociateNotificationTemplatesSuccess(resourceId, notificationId) { - return this.http.post( - `${this.baseUrl}${resourceId}/notification_templates_success/`, - { id: notificationId, disassociate: true } - ); - } - - associateNotificationTemplatesError(resourceId, notificationId) { - return this.http.post( - `${this.baseUrl}${resourceId}/notification_templates_error/`, - { id: notificationId } - ); - } - - disassociateNotificationTemplatesError(resourceId, notificationId) { - return this.http.post( - `${this.baseUrl}${resourceId}/notification_templates_error/`, - { id: notificationId, disassociate: true } - ); - } - - /** - * This is a helper method meant to simplify setting the "on" or "off" status of - * a related notification. - * - * @param[resourceId] - id of the base resource - * @param[notificationId] - id of the notification - * @param[notificationType] - the type of notification, options are "success" and "error" - * @param[associationState] - Boolean for associating or disassociating, - * options are true or false - */ - // eslint-disable-next-line max-len - updateNotificationTemplateAssociation( - resourceId, - notificationId, - notificationType, - associationState - ) { - if (notificationType === 'success' && associationState === true) { - return this.associateNotificationTemplatesSuccess( - resourceId, - notificationId - ); - } - - if (notificationType === 'success' && associationState === false) { - return this.disassociateNotificationTemplatesSuccess( - resourceId, - notificationId - ); - } - - if (notificationType === 'error' && associationState === true) { - return this.associateNotificationTemplatesError( - resourceId, - notificationId - ); - } - - if (notificationType === 'error' && associationState === false) { - return this.disassociateNotificationTemplatesError( - resourceId, - notificationId - ); - } - - throw new Error( - `Unsupported notificationType, associationState combination: ${notificationType}, ${associationState}` - ); - } - }; + throw new Error(`Unsupported notificationType, associationState combination: ${notificationType}, ${associationState}`); + } +}; export default NotificationsMixin; diff --git a/awx/ui_next/src/api/models/Organizations.js b/awx/ui_next/src/api/models/Organizations.js index c8531b3ecd..343c51f5b2 100644 --- a/awx/ui_next/src/api/models/Organizations.js +++ b/awx/ui_next/src/api/models/Organizations.js @@ -3,16 +3,16 @@ import NotificationsMixin from '../mixins/Notifications.mixin'; import InstanceGroupsMixin from '../mixins/InstanceGroups.mixin'; class Organizations extends InstanceGroupsMixin(NotificationsMixin(Base)) { - constructor(http) { + constructor (http) { super(http); this.baseUrl = '/api/v2/organizations/'; } - readAccessList(id, params = {}) { + readAccessList (id, params) { return this.http.get(`${this.baseUrl}${id}/access_list/`, { params }); } - readTeams(id, params = {}) { + readTeams (id, params) { return this.http.get(`${this.baseUrl}${id}/teams/`, { params }); } } diff --git a/awx/ui_next/src/api/models/Organizations.test.jsx b/awx/ui_next/src/api/models/Organizations.test.jsx index 19a1958dae..cb4dfbde70 100644 --- a/awx/ui_next/src/api/models/Organizations.test.jsx +++ b/awx/ui_next/src/api/models/Organizations.test.jsx @@ -3,9 +3,8 @@ import { describeNotificationMixin } from '../../../testUtils/apiReusable'; describe('OrganizationsAPI', () => { const orgId = 1; - const searchParams = { foo: 'bar' }; const createPromise = () => Promise.resolve(); - const mockHttp = { get: jest.fn(createPromise) }; + const mockHttp = ({ get: jest.fn(createPromise) }); const OrganizationsAPI = new Organizations(mockHttp); @@ -13,37 +12,43 @@ describe('OrganizationsAPI', () => { jest.clearAllMocks(); }); - test('read access list calls get with expected params', async done => { + test('read access list calls get with expected params', async (done) => { + const testParams = { foo: 'bar' }; + const testParamsDuplicates = { foo: ['bar', 'baz']}; + + const mockBaseURL = `/api/v2/organizations/${orgId}/access_list/`; + await OrganizationsAPI.readAccessList(orgId); - await OrganizationsAPI.readAccessList(orgId, searchParams); - - expect(mockHttp.get).toHaveBeenCalledTimes(2); - expect(mockHttp.get.mock.calls[0]).toContainEqual( - `/api/v2/organizations/${orgId}/access_list/`, - { params: {} } - ); - expect(mockHttp.get.mock.calls[1]).toContainEqual( - `/api/v2/organizations/${orgId}/access_list/`, - { params: searchParams } - ); + await OrganizationsAPI.readAccessList(orgId, testParams); + await OrganizationsAPI.readAccessList(orgId, testParamsDuplicates); + expect(mockHttp.get).toHaveBeenCalledTimes(3); + expect(mockHttp.get.mock.calls[0][0]).toEqual(`${mockBaseURL}`); + expect(mockHttp.get.mock.calls[0][1]).toEqual({"params": undefined}); + expect(mockHttp.get.mock.calls[1][0]).toEqual(`${mockBaseURL}`); + expect(mockHttp.get.mock.calls[1][1]).toEqual({"params": {"foo": "bar"}}); + expect(mockHttp.get.mock.calls[2][0]).toEqual(`${mockBaseURL}`); + expect(mockHttp.get.mock.calls[2][1]).toEqual({"params": {"foo": ["bar", "baz"]}}); done(); }); - test('read teams calls get with expected params', async done => { + test('read teams calls get with expected params', async (done) => { + const testParams = { foo: 'bar' }; + const testParamsDuplicates = { foo: ['bar', 'baz']}; + + const mockBaseURL = `/api/v2/organizations/${orgId}/teams/`; + await OrganizationsAPI.readTeams(orgId); - await OrganizationsAPI.readTeams(orgId, searchParams); - - expect(mockHttp.get).toHaveBeenCalledTimes(2); - expect(mockHttp.get.mock.calls[0]).toContainEqual( - `/api/v2/organizations/${orgId}/teams/`, - { params: {} } - ); - expect(mockHttp.get.mock.calls[1]).toContainEqual( - `/api/v2/organizations/${orgId}/teams/`, - { params: searchParams } - ); + await OrganizationsAPI.readTeams(orgId, testParams); + await OrganizationsAPI.readTeams(orgId, testParamsDuplicates); + expect(mockHttp.get).toHaveBeenCalledTimes(3); + expect(mockHttp.get.mock.calls[0][0]).toEqual(`${mockBaseURL}`); + expect(mockHttp.get.mock.calls[0][1]).toEqual({"params": undefined}); + expect(mockHttp.get.mock.calls[1][0]).toEqual(`${mockBaseURL}`); + expect(mockHttp.get.mock.calls[1][1]).toEqual({"params": {"foo": "bar"}}); + expect(mockHttp.get.mock.calls[2][0]).toEqual(`${mockBaseURL}`); + expect(mockHttp.get.mock.calls[2][1]).toEqual({"params": {"foo": ["bar", "baz"]}}); done(); }); }); diff --git a/awx/ui_next/src/components/AddRole/AddResourceRole.jsx b/awx/ui_next/src/components/AddRole/AddResourceRole.jsx index 7bf58c3cb7..1cbf55a3bd 100644 --- a/awx/ui_next/src/components/AddRole/AddResourceRole.jsx +++ b/awx/ui_next/src/components/AddRole/AddResourceRole.jsx @@ -8,13 +8,14 @@ import SelectRoleStep from './SelectRoleStep'; import SelectableCard from './SelectableCard'; import { TeamsAPI, UsersAPI } from '../../api'; -const readUsers = async queryParams => - UsersAPI.read(Object.assign(queryParams, { is_superuser: false })); +const readUsers = async (queryParams) => UsersAPI.read( + Object.assign(queryParams, { is_superuser: false }) +); -const readTeams = async queryParams => TeamsAPI.read(queryParams); +const readTeams = async (queryParams) => TeamsAPI.read(queryParams); class AddResourceRole extends React.Component { - constructor(props) { + constructor (props) { super(props); this.state = { @@ -24,69 +25,67 @@ class AddResourceRole extends React.Component { currentStepId: 1, }; - this.handleResourceCheckboxClick = this.handleResourceCheckboxClick.bind( - this - ); + this.handleResourceCheckboxClick = this.handleResourceCheckboxClick.bind(this); this.handleResourceSelect = this.handleResourceSelect.bind(this); this.handleRoleCheckboxClick = this.handleRoleCheckboxClick.bind(this); this.handleWizardNext = this.handleWizardNext.bind(this); this.handleWizardSave = this.handleWizardSave.bind(this); } - handleResourceCheckboxClick(user) { + handleResourceCheckboxClick (user) { const { selectedResourceRows } = this.state; - const selectedIndex = selectedResourceRows.findIndex( - selectedRow => selectedRow.id === user.id - ); + const selectedIndex = selectedResourceRows + .findIndex(selectedRow => selectedRow.id === user.id); if (selectedIndex > -1) { selectedResourceRows.splice(selectedIndex, 1); this.setState({ selectedResourceRows }); } else { this.setState(prevState => ({ - selectedResourceRows: [...prevState.selectedResourceRows, user], + selectedResourceRows: [...prevState.selectedResourceRows, user] })); } } - handleRoleCheckboxClick(role) { + handleRoleCheckboxClick (role) { const { selectedRoleRows } = this.state; - const selectedIndex = selectedRoleRows.findIndex( - selectedRow => selectedRow.id === role.id - ); + const selectedIndex = selectedRoleRows + .findIndex(selectedRow => selectedRow.id === role.id); if (selectedIndex > -1) { selectedRoleRows.splice(selectedIndex, 1); this.setState({ selectedRoleRows }); } else { this.setState(prevState => ({ - selectedRoleRows: [...prevState.selectedRoleRows, role], + selectedRoleRows: [...prevState.selectedRoleRows, role] })); } } - handleResourceSelect(resourceType) { + handleResourceSelect (resourceType) { this.setState({ selectedResource: resourceType, selectedResourceRows: [], - selectedRoleRows: [], + selectedRoleRows: [] }); } - handleWizardNext(step) { + handleWizardNext (step) { this.setState({ currentStepId: step.id, }); } - async handleWizardSave() { - const { onSave } = this.props; + async handleWizardSave () { + const { + onSave + } = this.props; const { selectedResourceRows, selectedRoleRows, - selectedResource, + selectedResource } = this.state; try { @@ -96,17 +95,11 @@ class AddResourceRole extends React.Component { for (let j = 0; j < selectedRoleRows.length; j++) { if (selectedResource === 'users') { roleRequests.push( - UsersAPI.associateRole( - selectedResourceRows[i].id, - selectedRoleRows[j].id - ) + UsersAPI.associateRole(selectedResourceRows[i].id, selectedRoleRows[j].id) ); } else if (selectedResource === 'teams') { roleRequests.push( - TeamsAPI.associateRole( - selectedResourceRows[i].id, - selectedRoleRows[j].id - ) + TeamsAPI.associateRole(selectedResourceRows[i].id, selectedRoleRows[j].id) ); } } @@ -119,21 +112,25 @@ class AddResourceRole extends React.Component { } } - render() { + render () { const { selectedResource, selectedResourceRows, selectedRoleRows, currentStepId, } = this.state; - const { onClose, roles, i18n } = this.props; + const { + onClose, + roles, + i18n + } = this.props; const userColumns = [ - { name: i18n._(t`Username`), key: 'username', isSortable: true }, + { name: i18n._(t`Username`), key: 'username', isSortable: true, isSearchable: true } ]; const teamColumns = [ - { name: i18n._(t`Name`), key: 'name', isSortable: true }, + { name: i18n._(t`Name`), key: 'name', isSortable: true, isSearchable: true } ]; let wizardTitle = ''; @@ -167,7 +164,7 @@ class AddResourceRole extends React.Component { /> ), - enableNext: selectedResource !== null, + enableNext: selectedResource !== null }, { id: 2, @@ -198,7 +195,7 @@ class AddResourceRole extends React.Component { )} ), - enableNext: selectedResourceRows.length > 0, + enableNext: selectedResourceRows.length > 0 }, { id: 3, @@ -214,8 +211,8 @@ class AddResourceRole extends React.Component { /> ), nextButtonText: i18n._(t`Save`), - enableNext: selectedRoleRows.length > 0, - }, + enableNext: selectedRoleRows.length > 0 + } ]; const currentStep = steps.find(step => step.id === currentStepId); @@ -239,11 +236,11 @@ class AddResourceRole extends React.Component { AddResourceRole.propTypes = { onClose: PropTypes.func.isRequired, onSave: PropTypes.func.isRequired, - roles: PropTypes.shape(), + roles: PropTypes.shape() }; AddResourceRole.defaultProps = { - roles: {}, + roles: {} }; export { AddResourceRole as _AddResourceRole }; diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx index 8610bfa779..fb7b00e4f1 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx @@ -7,7 +7,7 @@ import PaginatedDataList from '../PaginatedDataList'; import DataListToolbar from '../DataListToolbar'; import CheckboxListItem from '../CheckboxListItem'; import SelectedList from '../SelectedList'; -import { getQSConfig, parseNamespacedQueryString } from '../../util/qs'; +import { getQSConfig, parseQueryString } from '../../util/qs'; class SelectResourceStep extends React.Component { constructor(props) { @@ -40,7 +40,7 @@ class SelectResourceStep extends React.Component { async readResourceList() { const { onSearch, location } = this.props; - const queryParams = parseNamespacedQueryString( + const queryParams = parseQueryString( this.qsConfig, location.search ); diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx index aab32c7df6..6dced8c9f9 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx @@ -6,7 +6,9 @@ import { sleep } from '../../../testUtils/testUtils'; import SelectResourceStep from './SelectResourceStep'; describe('', () => { - const columns = [{ name: 'Username', key: 'username', isSortable: true }]; + const columns = [ + { name: 'Username', key: 'username', isSortable: true, isSearchable: true } + ]; afterEach(() => { jest.restoreAllMocks(); }); @@ -28,9 +30,9 @@ describe('', () => { count: 2, results: [ { id: 1, username: 'foo', url: 'item/1' }, - { id: 2, username: 'bar', url: 'item/2' }, - ], - }, + { id: 2, username: 'bar', url: 'item/2' } + ] + } }); mountWithContexts( ', () => { expect(handleSearch).toHaveBeenCalledWith({ order_by: 'username', page: 1, - page_size: 5, + page_size: 5 }); }); test('readResourceList properly adds rows to state', async () => { - const selectedResourceRows = [{ id: 1, username: 'foo', url: 'item/1' }]; + const selectedResourceRows = [ + { id: 1, username: 'foo', url: 'item/1' } + ]; const handleSearch = jest.fn().mockResolvedValue({ data: { count: 2, results: [ { id: 1, username: 'foo', url: 'item/1' }, - { id: 2, username: 'bar', url: 'item/2' }, - ], - }, + { id: 2, username: 'bar', url: 'item/2' } + ] + } }); const history = createMemoryHistory({ - initialEntries: [ - '/organizations/1/access?resource.page=1&resource.order_by=-username', - ], + initialEntries: ['/organizations/1/access?resource.page=1&resource.order_by=-username'], }); const wrapper = await mountWithContexts( ', () => { onSearch={handleSearch} selectedResourceRows={selectedResourceRows} sortedColumnKey="username" - />, - { - context: { router: { history, route: { location: history.location } } }, - } + />, { context: { router: { history, route: { location: history.location } } } } ).find('SelectResourceStep'); await wrapper.instance().readResourceList(); expect(handleSearch).toHaveBeenCalledWith({ @@ -85,7 +84,7 @@ describe('', () => { }); expect(wrapper.state('resources')).toEqual([ { id: 1, username: 'foo', url: 'item/1' }, - { id: 2, username: 'bar', url: 'item/2' }, + { id: 2, username: 'bar', url: 'item/2' } ]); }); @@ -95,8 +94,8 @@ describe('', () => { count: 2, results: [ { id: 1, username: 'foo', url: 'item/1' }, - { id: 2, username: 'bar', url: 'item/2' }, - ], + { id: 2, username: 'bar', url: 'item/2' } + ] }; const wrapper = mountWithContexts( ', () => { wrapper.update(); const checkboxListItemWrapper = wrapper.find('CheckboxListItem'); expect(checkboxListItemWrapper.length).toBe(2); - checkboxListItemWrapper - .first() - .find('input[type="checkbox"]') + checkboxListItemWrapper.first().find('input[type="checkbox"]') .simulate('change', { target: { checked: true } }); expect(handleRowClick).toHaveBeenCalledWith(data.results[0]); }); diff --git a/awx/ui_next/src/components/Chip/ChipGroup.jsx b/awx/ui_next/src/components/Chip/ChipGroup.jsx index df00f86eaa..24ffdbc220 100644 --- a/awx/ui_next/src/components/Chip/ChipGroup.jsx +++ b/awx/ui_next/src/components/Chip/ChipGroup.jsx @@ -1,17 +1,16 @@ import React, { useState } from 'react'; -import { number } from 'prop-types'; +import { number, bool } from 'prop-types'; import styled from 'styled-components'; import Chip from './Chip'; -const ChipGroup = ({ children, className, showOverflowAfter, ...props }) => { +const ChipGroup = ({ children, className, showOverflowAfter, displayAll, ...props }) => { const [isExpanded, setIsExpanded] = useState(!showOverflowAfter); const toggleIsOpen = () => setIsExpanded(!isExpanded); - const mappedChildren = React.Children.map(children, c => + const mappedChildren = React.Children.map(children, c => ( React.cloneElement(c, { component: 'li' }) - ); - const showOverflowToggle = - showOverflowAfter && children.length > showOverflowAfter; + )); + const showOverflowToggle = showOverflowAfter && children.length > showOverflowAfter; const numToShow = isExpanded ? children.length : Math.min(showOverflowAfter, children.length); @@ -20,8 +19,8 @@ const ChipGroup = ({ children, className, showOverflowAfter, ...props }) => { return (
    - {mappedChildren.slice(0, numToShow)} - {showOverflowToggle && ( + {displayAll ? mappedChildren : mappedChildren.slice(0, numToShow)} + {!displayAll && showOverflowToggle && ( {isExpanded ? expandedText : collapsedText} @@ -31,12 +30,18 @@ const ChipGroup = ({ children, className, showOverflowAfter, ...props }) => { }; ChipGroup.propTypes = { showOverflowAfter: number, + displayAll: bool }; ChipGroup.defaultProps = { showOverflowAfter: null, + displayAll: false }; export default styled(ChipGroup)` --pf-c-chip-group--c-chip--MarginRight: 10px; --pf-c-chip-group--c-chip--MarginBottom: 10px; + + > .pf-c-chip.pf-m-overflow button { + padding: 3px 8px; + } `; diff --git a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.test.jsx b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.test.jsx index 9bf646ba96..e8262b3333 100644 --- a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.test.jsx +++ b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.test.jsx @@ -13,9 +13,9 @@ describe('', () => { }); test('it triggers the expected callbacks', () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true }]; + const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; - const search = 'button[aria-label="Search"]'; + const search = 'button[aria-label="Search submit button"]'; const searchTextInput = 'input[aria-label="Search text input"]'; const selectAll = 'input[aria-label="Select all"]'; const sort = 'button[aria-label="Sort"]'; @@ -53,19 +53,20 @@ describe('', () => { toolbar.find(search).simulate('click'); expect(onSearch).toHaveBeenCalledTimes(1); - expect(onSearch).toBeCalledWith('test-321'); + expect(onSearch).toBeCalledWith('name__icontains', 'test-321'); }); - test('dropdown items sortable columns work', () => { + test('dropdown items sortable/searchable columns work', () => { const sortDropdownToggleSelector = 'button[id="awx-sort"]'; const searchDropdownToggleSelector = 'button[id="awx-search"]'; - const dropdownMenuItems = 'DropdownMenu > ul'; + const sortDropdownMenuItems = 'DropdownMenu > ul[aria-labelledby="awx-sort"]'; + const searchDropdownMenuItems = 'DropdownMenu > ul[aria-labelledby="awx-search"]'; const multipleColumns = [ - { name: 'Foo', key: 'foo', isSortable: true }, - { name: 'Bar', key: 'bar', isSortable: true }, + { name: 'Foo', key: 'foo', isSortable: true, isSearchable: true }, + { name: 'Bar', key: 'bar', isSortable: true, isSearchable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' }, + { name: 'Baz', key: 'baz' } ]; const onSort = jest.fn(); @@ -82,9 +83,14 @@ describe('', () => { expect(sortDropdownToggle.length).toBe(1); sortDropdownToggle.simulate('click'); toolbar.update(); - const sortDropdownItems = toolbar.find(dropdownMenuItems).children(); + const sortDropdownItems = toolbar.find(sortDropdownMenuItems).children(); expect(sortDropdownItems.length).toBe(2); - + let searchDropdownToggle = toolbar.find(searchDropdownToggleSelector); + expect(searchDropdownToggle.length).toBe(1); + searchDropdownToggle.simulate('click'); + toolbar.update(); + let searchDropdownItems = toolbar.find(searchDropdownMenuItems).children(); + expect(searchDropdownItems.length).toBe(1); const mockedSortEvent = { target: { innerText: 'Bar' } }; sortDropdownItems.at(0).simulate('click', mockedSortEvent); toolbar = mountWithContexts( @@ -97,16 +103,12 @@ describe('', () => { ); toolbar.update(); - const sortDropdownToggleDescending = toolbar.find( - sortDropdownToggleSelector - ); + const sortDropdownToggleDescending = toolbar.find(sortDropdownToggleSelector); expect(sortDropdownToggleDescending.length).toBe(1); sortDropdownToggleDescending.simulate('click'); toolbar.update(); - const sortDropdownItemsDescending = toolbar - .find(dropdownMenuItems) - .children(); + const sortDropdownItemsDescending = toolbar.find(sortDropdownMenuItems).children(); expect(sortDropdownItemsDescending.length).toBe(2); sortDropdownToggleDescending.simulate('click'); // toggle close the sort dropdown @@ -114,13 +116,13 @@ describe('', () => { sortDropdownItems.at(0).simulate('click', mockedSortEventDescending); toolbar.update(); - const searchDropdownToggle = toolbar.find(searchDropdownToggleSelector); + searchDropdownToggle = toolbar.find(searchDropdownToggleSelector); expect(searchDropdownToggle.length).toBe(1); searchDropdownToggle.simulate('click'); toolbar.update(); - const searchDropdownItems = toolbar.find(dropdownMenuItems).children(); - expect(searchDropdownItems.length).toBe(3); + searchDropdownItems = toolbar.find(searchDropdownMenuItems).children(); + expect(searchDropdownItems.length).toBe(1); const mockedSearchEvent = { target: { innerText: 'Bar' } }; searchDropdownItems.at(0).simulate('click', mockedSearchEvent); @@ -132,12 +134,8 @@ describe('', () => { const downAlphaIconSelector = 'SortAlphaDownIcon'; const upAlphaIconSelector = 'SortAlphaUpIcon'; - const numericColumns = [ - { name: 'ID', key: 'id', isSortable: true, isNumeric: true }, - ]; - const alphaColumns = [ - { name: 'Name', key: 'name', isSortable: true, isNumeric: false }, - ]; + const numericColumns = [{ name: 'ID', key: 'id', isSortable: true, isNumeric: true }]; + const alphaColumns = [{ name: 'Name', key: 'name', isSortable: true, isNumeric: false }]; toolbar = mountWithContexts( ', () => { }); test('should render additionalControls', () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true }]; + const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; const onSearch = jest.fn(); const onSort = jest.fn(); const onSelectAll = jest.fn(); @@ -196,11 +194,7 @@ describe('', () => { onSearch={onSearch} onSort={onSort} onSelectAll={onSelectAll} - additionalControls={[ - , - ]} + additionalControls={[]} /> ); diff --git a/awx/ui_next/src/components/FilterTags/FilterTags.jsx b/awx/ui_next/src/components/FilterTags/FilterTags.jsx new file mode 100644 index 0000000000..cf0ca41575 --- /dev/null +++ b/awx/ui_next/src/components/FilterTags/FilterTags.jsx @@ -0,0 +1,80 @@ +import React from 'react'; +import { withRouter } from 'react-router-dom'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import styled from 'styled-components'; +import { Button } from '@patternfly/react-core'; +import { parseQueryString } from '@util/qs'; +import { ChipGroup, Chip } from '@components/Chip'; +import VerticalSeparator from '@components/VerticalSeparator'; + +const FilterTagsRow = styled.div` + display: flex; + padding: 15px 20px; + border-top: 1px solid #d2d2d2; + font-size: 14px; + align-items: center; +`; + +const ResultCount = styled.span` + font-weight: bold; +`; + +const FilterLabel = styled.span` + padding-right: 20px; +`; + +// remove non-default query params so they don't show up as filter tags +const filterOutDefaultParams = (paramsArr, config) => { + const defaultParamsKeys = Object.keys(config.defaultParams); + return paramsArr.filter(key => defaultParamsKeys.indexOf(key) === -1); +}; + +const FilterTags = ({ i18n, itemCount, qsConfig, location, onRemove, onRemoveAll }) => { + const queryParams = parseQueryString(qsConfig, location.search); + const queryParamsArr = []; + const displayAll = true; + const nonDefaultParams = filterOutDefaultParams(Object.keys(queryParams), qsConfig); + nonDefaultParams + .forEach(key => { + if (Array.isArray(queryParams[key])) { + queryParams[key].forEach(val => queryParamsArr.push({ key, value: val })); + } else { + queryParamsArr.push({ key, value: queryParams[key] }); + } + }); + + return (queryParamsArr.length > 0) && ( + + + {`${itemCount} results`} + + + Active Filters: + + {queryParamsArr.map(param => ( + onRemove(param)} + > + {param.value} + + ))} +
    + +
    +
    +
    + ); +}; + +export default withI18n()(withRouter(FilterTags)); diff --git a/awx/ui_next/src/components/FilterTags/FilterTags.test.jsx b/awx/ui_next/src/components/FilterTags/FilterTags.test.jsx new file mode 100644 index 0000000000..7ad82361f4 --- /dev/null +++ b/awx/ui_next/src/components/FilterTags/FilterTags.test.jsx @@ -0,0 +1,42 @@ +import React from 'react'; +import { createMemoryHistory } from 'history'; +import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; +import FilterTags from './FilterTags'; + +describe('', () => { + const qsConfig = { + namespace: 'item', + defaultParams: { page: 1, page_size: 5, order_by: 'name' }, + integerFields: [], + }; + const onRemoveFn = jest.fn(); + const onRemoveAllFn = jest.fn(); + + test('initially renders without crashing', () => { + const wrapper = mountWithContexts( + + ); + expect(wrapper.length).toBe(1); + wrapper.unmount(); + }); + + test('renders non-default param tags based on location history', () => { + const history = createMemoryHistory({ + initialEntries: ['/foo?item.page=1&item.page_size=2&item.foo=bar&item.baz=bust'], + }); + const wrapper = mountWithContexts( + , { context: { router: { history, route: { location: history.location } } } } + ); + const chips = wrapper.find('.pf-c-chip.searchTagChip'); + expect(chips.length).toBe(2); + wrapper.unmount(); + }); +}); diff --git a/awx/ui_next/src/components/FilterTags/index.js b/awx/ui_next/src/components/FilterTags/index.js new file mode 100644 index 0000000000..19a1fa21b9 --- /dev/null +++ b/awx/ui_next/src/components/FilterTags/index.js @@ -0,0 +1 @@ +export { default } from './FilterTags'; diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.jsx new file mode 100644 index 0000000000..1009de50f2 --- /dev/null +++ b/awx/ui_next/src/components/ListHeader/ListHeader.jsx @@ -0,0 +1,140 @@ +import React, { Fragment } from 'react'; +import PropTypes, { arrayOf, shape, string, bool } from 'prop-types'; +import { withRouter } from 'react-router-dom'; +import styled from 'styled-components'; + +import DataListToolbar from '@components/DataListToolbar'; +import FilterTags from '@components/FilterTags'; + +import { + encodeNonDefaultQueryString, + parseQueryString, + addParams, + removeParams +} from '@util/qs'; +import { QSConfig } from '@types'; + +const EmptyStateControlsWrapper = styled.div` + display: flex; + margin-top: 20px; + margin-right: 20px; + margin-bottom: 20px; + justify-content: flex-end; + + & > :not(:first-child) { + margin-left: 20px; + } +`; +class ListHeader extends React.Component { + constructor (props) { + super(props); + + this.handleSearch = this.handleSearch.bind(this); + this.handleSort = this.handleSort.bind(this); + this.handleRemove = this.handleRemove.bind(this); + this.handleRemoveAll = this.handleRemoveAll.bind(this); + } + + getSortOrder () { + const { qsConfig, location } = this.props; + const queryParams = parseQueryString(qsConfig, location.search); + if (queryParams.order_by && queryParams.order_by.startsWith('-')) { + return [queryParams.order_by.substr(1), 'descending']; + } + return [queryParams.order_by, 'ascending']; + } + + handleSearch (key, value) { + const { history, qsConfig } = this.props; + const { search } = history.location; + this.pushHistoryState(addParams(qsConfig, search, { [key]: value })); + } + + handleRemove ({ key, value }) { + const { history, qsConfig } = this.props; + const { search } = history.location; + this.pushHistoryState(removeParams(qsConfig, search, { [key]: value })); + } + + handleRemoveAll () { + this.pushHistoryState(null); + } + + handleSort (key, order) { + const { history, qsConfig } = this.props; + const { search } = history.location; + this.pushHistoryState(addParams(qsConfig, search, { + order_by: order === 'ascending' ? key : `-${key}`, + page: null, + })); + } + + pushHistoryState (params) { + const { history, qsConfig } = this.props; + const { pathname } = history.location; + const encodedParams = encodeNonDefaultQueryString(qsConfig, params); + history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); + } + + render () { + const { + emptyStateControls, + itemCount, + columns, + renderToolbar, + qsConfig + } = this.props; + const [orderBy, sortOrder] = this.getSortOrder(); + return ( + + {itemCount === 0 ? ( + + + {emptyStateControls} + + + + ) : ( + + {renderToolbar({ + sortedColumnKey: orderBy, + sortOrder, + columns, + onSearch: this.handleSearch, + onSort: this.handleSort, + })} + + + )} + + ); + } +} + +ListHeader.propTypes = { + itemCount: PropTypes.number.isRequired, + qsConfig: QSConfig.isRequired, + columns: arrayOf(shape({ + name: string.isRequired, + key: string.isRequired, + isSortable: bool, + isSearchable: bool + })).isRequired, + renderToolbar: PropTypes.func, +}; + +ListHeader.defaultProps = { + renderToolbar: (props) => (), +}; + +export default withRouter(ListHeader); diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx new file mode 100644 index 0000000000..264c4e66b6 --- /dev/null +++ b/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx @@ -0,0 +1,56 @@ +import React from 'react'; +import { createMemoryHistory } from 'history'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import { sleep } from '@testUtils/testUtils'; +import ListHeader from './ListHeader'; + +describe('ListHeader', () => { + const qsConfig = { + namespace: 'item', + defaultParams: { page: 1, page_size: 5, order_by: 'name' }, + integerFields: [], + }; + const renderToolbarFn = jest.fn(); + + test('initially renders without crashing', () => { + const wrapper = mountWithContexts( + + ); + expect(wrapper.length).toBe(1); + wrapper.unmount(); + }); + + test('should navigate when DataListToolbar calls onSort prop', async () => { + const history = createMemoryHistory({ + initialEntries: ['/organizations/1/teams'], + }); + const wrapper = mountWithContexts( + , { context: { router: { history } } } + ); + + const toolbar = wrapper.find('DataListToolbar'); + expect(toolbar.prop('sortedColumnKey')).toEqual('name'); + expect(toolbar.prop('sortOrder')).toEqual('ascending'); + toolbar.prop('onSort')('name', 'descending'); + expect(history.location.search).toEqual('?item.order_by=-name'); + await sleep(0); + wrapper.update(); + + expect(toolbar.prop('sortedColumnKey')).toEqual('name'); + // TODO: this assertion required updating queryParams prop. Consider + // fixing after #147 is done: + // expect(toolbar.prop('sortOrder')).toEqual('descending'); + toolbar.prop('onSort')('name', 'ascending'); + // since order_by = name is the default, that should be strip out of the search + expect(history.location.search).toEqual(''); + }); +}); diff --git a/awx/ui_next/src/components/ListHeader/index.js b/awx/ui_next/src/components/ListHeader/index.js new file mode 100644 index 0000000000..64af4f118a --- /dev/null +++ b/awx/ui_next/src/components/ListHeader/index.js @@ -0,0 +1 @@ +export { default } from './ListHeader'; diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index 9eeecfe6af..49586324ca 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -25,7 +25,7 @@ import DataListToolbar from '../DataListToolbar'; import CheckboxListItem from '../CheckboxListItem'; import SelectedList from '../SelectedList'; import { ChipGroup, Chip } from '../Chip'; -import { getQSConfig, parseNamespacedQueryString } from '../../util/qs'; +import { getQSConfig, parseQueryString } from '../../util/qs'; const InputGroup = styled(PFInputGroup)` ${props => @@ -95,7 +95,7 @@ class Lookup extends React.Component { getItems, location: { search }, } = this.props; - const queryParams = parseNamespacedQueryString(this.qsConfig, search); + const queryParams = parseQueryString(this.qsConfig, search); this.setState({ error: false }); try { diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index 48bc97bf73..6508e0d6aa 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -4,99 +4,54 @@ import { DataList } from '@patternfly/react-core'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { withRouter } from 'react-router-dom'; -import styled from 'styled-components'; -import ContentEmpty from '../ContentEmpty'; -import ContentError from '../ContentError'; -import ContentLoading from '../ContentLoading'; -import Pagination from '../Pagination'; -import DataListToolbar from '../DataListToolbar'; -import PaginatedDataListItem from './PaginatedDataListItem'; +import ListHeader from '@components/ListHeader'; +import ContentEmpty from '@components/ContentEmpty'; +import ContentError from '@components/ContentError'; +import ContentLoading from '@components/ContentLoading'; +import Pagination from '@components/Pagination'; +import DataListToolbar from '@components/DataListToolbar'; + import { - parseNamespacedQueryString, - updateNamespacedQueryString, -} from '../../util/qs'; -import { pluralize, ucFirst } from '../../util/strings'; -import { QSConfig } from '../../types'; + encodeNonDefaultQueryString, + parseQueryString, + addParams +} from '@util/qs'; +import { pluralize, ucFirst } from '@util/strings'; -const EmptyStateControlsWrapper = styled.div` - display: flex; - margin-top: 20px; - margin-right: 20px; - margin-bottom: 20px; - justify-content: flex-end; +import { QSConfig } from '@types'; + +import PaginatedDataListItem from './PaginatedDataListItem'; - & > :not(:first-child) { - margin-left: 20px; - } -`; class PaginatedDataList extends React.Component { - constructor(props) { + constructor (props) { super(props); this.handleSetPage = this.handleSetPage.bind(this); this.handleSetPageSize = this.handleSetPageSize.bind(this); - this.handleSort = this.handleSort.bind(this); } - componentDidUpdate(prevProps) { - const { itemCount: prevItemCount } = prevProps; - const { itemCount } = this.props; - if (prevItemCount !== itemCount) { - this.getCurrPage(itemCount); - } - } - - getSortOrder() { - const { qsConfig, location } = this.props; - const queryParams = parseNamespacedQueryString(qsConfig, location.search); - if (queryParams.order_by && queryParams.order_by.startsWith('-')) { - return [queryParams.order_by.substr(1), 'descending']; - } - return [queryParams.order_by, 'ascending']; - } - - getCurrPage(itemCount) { - if (itemCount < 0) { - return; - } - const { qsConfig, location } = this.props; - const { page_size, page: currPage } = parseNamespacedQueryString( - qsConfig, - location.search - ); - const lastPage = Math.ceil(itemCount / page_size); - if (currPage > lastPage) { - this.pushHistoryState({ page: lastPage || 1 }); - } - } - - handleSetPage(event, pageNumber) { - this.pushHistoryState({ page: pageNumber }); - } - - handleSetPageSize(event, pageSize) { - this.pushHistoryState({ page_size: pageSize }); - } - - handleSort(sortedColumnKey, sortOrder) { - this.pushHistoryState({ - order_by: - sortOrder === 'ascending' ? sortedColumnKey : `-${sortedColumnKey}`, - page: null, - }); - } - - pushHistoryState(newParams) { + handleSetPage (event, pageNumber) { const { history, qsConfig } = this.props; - const { pathname, search } = history.location; - const qs = updateNamespacedQueryString(qsConfig, search, newParams); - history.push(`${pathname}?${qs}`); + const { search } = history.location; + this.pushHistoryState(addParams(qsConfig, search, { page: pageNumber })); } - render() { - const [orderBy, sortOrder] = this.getSortOrder(); + handleSetPageSize (event, pageSize) { + const { history, qsConfig } = this.props; + const { search } = history.location; + this.pushHistoryState(addParams(qsConfig, search, { page_size: pageSize })); + } + + pushHistoryState (params) { + const { history, qsConfig } = this.props; + const { pathname } = history.location; + const encodedParams = encodeNonDefaultQueryString(qsConfig, params); + history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); + } + + render () { const { - contentError, + hasContentError, hasContentLoading, emptyStateControls, items, @@ -111,46 +66,36 @@ class PaginatedDataList extends React.Component { i18n, renderToolbar, } = this.props; - const columns = toolbarColumns.length - ? toolbarColumns - : [{ name: i18n._(t`Name`), key: 'name', isSortable: true }]; - const queryParams = parseNamespacedQueryString(qsConfig, location.search); + const columns = toolbarColumns.length ? toolbarColumns : [{ name: i18n._(t`Name`), key: 'name', isSortable: true, isSearchable: true }]; + const queryParams = parseQueryString(qsConfig, location.search); const itemDisplayName = ucFirst(pluralize(itemName)); - const itemDisplayNamePlural = ucFirst( - itemNamePlural || pluralize(itemName) - ); + const itemDisplayNamePlural = ucFirst(itemNamePlural || pluralize(itemName)); const dataListLabel = i18n._(t`${itemDisplayName} List`); - const emptyContentMessage = i18n._( - t`Please add ${itemDisplayNamePlural} to populate this list ` - ); + const emptyContentMessage = i18n._(t`Please add ${itemDisplayNamePlural} to populate this list `); const emptyContentTitle = i18n._(t`No ${itemDisplayNamePlural} Found `); let Content; if (hasContentLoading && items.length <= 0) { - Content = ; - } else if (contentError) { - Content = ; + Content = (); + } else if (hasContentError) { + Content = (); } else if (items.length <= 0) { - Content = ( - - ); + Content = (); } else { - Content = ( - {items.map(renderItem)} - ); + Content = ({items.map(renderItem)}); } if (items.length <= 0) { return ( - {emptyStateControls && ( - - {emptyStateControls} - - )} - {emptyStateControls &&
    } + {Content} ); @@ -158,29 +103,24 @@ class PaginatedDataList extends React.Component { return ( - {renderToolbar({ - sortedColumnKey: orderBy, - sortOrder, - columns, - onSearch: () => {}, - onSort: this.handleSort, - })} + {Content} @@ -202,28 +142,26 @@ PaginatedDataList.propTypes = { itemNamePlural: PropTypes.string, qsConfig: QSConfig.isRequired, renderItem: PropTypes.func, - toolbarColumns: arrayOf( - shape({ - name: string.isRequired, - key: string.isRequired, - isSortable: bool, - }) - ), + toolbarColumns: arrayOf(shape({ + name: string.isRequired, + key: string.isRequired, + isSortable: bool, + })), showPageSizeOptions: PropTypes.bool, renderToolbar: PropTypes.func, hasContentLoading: PropTypes.bool, - contentError: PropTypes.shape(), + hasContentError: PropTypes.bool, }; PaginatedDataList.defaultProps = { hasContentLoading: false, - contentError: null, + hasContentError: false, toolbarColumns: [], itemName: 'item', itemNamePlural: '', showPageSizeOptions: true, - renderItem: item => , - renderToolbar: props => , + renderItem: (item) => (), + renderToolbar: (props) => (), }; export { PaginatedDataList as _PaginatedDataList }; diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx index 1c9a22c374..c03c26d1ff 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx @@ -1,7 +1,6 @@ import React from 'react'; import { createMemoryHistory } from 'history'; -import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; -import { sleep } from '../../../testUtils/testUtils'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; import PaginatedDataList from './PaginatedDataList'; const mockData = [ @@ -10,13 +9,11 @@ const mockData = [ { id: 3, name: 'three', url: '/org/team/3' }, { id: 4, name: 'four', url: '/org/team/4' }, { id: 5, name: 'five', url: '/org/team/5' }, - { id: 6, name: 'six', url: '/org/team/6' }, - { id: 7, name: 'seven', url: '/org/team/7' }, ]; const qsConfig = { namespace: 'item', - defaultParams: { page: 1, page_size: 5 }, + defaultParams: { page: 1, page_size: 5, order_by: 'name' }, integerFields: [], }; @@ -40,40 +37,6 @@ describe('', () => { ); }); - test('should navigate when DataListToolbar calls onSort prop', async () => { - const history = createMemoryHistory({ - initialEntries: ['/organizations/1/teams'], - }); - const wrapper = mountWithContexts( - , - { context: { router: { history } } } - ); - - const toolbar = wrapper.find('DataListToolbar'); - expect(toolbar.prop('sortedColumnKey')).toEqual('name'); - expect(toolbar.prop('sortOrder')).toEqual('ascending'); - toolbar.prop('onSort')('name', 'descending'); - expect(history.location.search).toEqual('?item.order_by=-name'); - await sleep(0); - wrapper.update(); - - expect(toolbar.prop('sortedColumnKey')).toEqual('name'); - // TODO: this assertion required updating queryParams prop. Consider - // fixing after #147 is done: - // expect(toolbar.prop('sortOrder')).toEqual('descending'); - toolbar.prop('onSort')('name', 'ascending'); - expect(history.location.search).toEqual('?item.order_by=name'); - }); - test('should navigate to page when Pagination calls onSetPage prop', () => { const history = createMemoryHistory({ initialEntries: ['/organizations/1/teams'], @@ -88,8 +51,7 @@ describe('', () => { order_by: 'name', }} qsConfig={qsConfig} - />, - { context: { router: { history } } } + />, { context: { router: { history } } } ); const pagination = wrapper.find('Pagination'); @@ -97,7 +59,8 @@ describe('', () => { expect(history.location.search).toEqual('?item.page=2'); wrapper.update(); pagination.prop('onSetPage')(null, 1); - expect(history.location.search).toEqual('?item.page=1'); + // since page = 1 is the default, that should be strip out of the search + expect(history.location.search).toEqual(''); }); test('should navigate to page when Pagination calls onPerPageSelect prop', () => { @@ -114,48 +77,15 @@ describe('', () => { order_by: 'name', }} qsConfig={qsConfig} - />, - { context: { router: { history } } } + />, { context: { router: { history } } } ); const pagination = wrapper.find('Pagination'); - pagination.prop('onPerPageSelect')(null, 5); - expect(history.location.search).toEqual('?item.page_size=5'); - wrapper.update(); pagination.prop('onPerPageSelect')(null, 25); expect(history.location.search).toEqual('?item.page_size=25'); - }); - test('should navigate to correct current page when list items change', () => { - const customQSConfig = { - namespace: 'foo', - defaultParams: { page: 7, page_size: 1 }, // show only 1 item per page - integerFields: [], - }; - const testParams = [5, 25, 0, -1]; // number of items - const expected = [5, 5, 1, 1]; // expected current page - const history = createMemoryHistory({ - initialEntries: ['/organizations/1/teams'], - }); - const wrapper = mountWithContexts( - , - { context: { router: { history } } } - ); - testParams.forEach((param, i) => { - wrapper.setProps({ itemCount: param }); - expect(history.location.search).toEqual( - `?${customQSConfig.namespace}.page=${expected[i]}` - ); - wrapper.update(); - }); - wrapper.unmount(); + wrapper.update(); + // since page_size = 5 is the default, that should be strip out of the search + pagination.prop('onPerPageSelect')(null, 5); + expect(history.location.search).toEqual(''); }); }); diff --git a/awx/ui_next/src/components/Search/Search.jsx b/awx/ui_next/src/components/Search/Search.jsx index 59cbe74658..007e749c7c 100644 --- a/awx/ui_next/src/components/Search/Search.jsx +++ b/awx/ui_next/src/components/Search/Search.jsx @@ -8,9 +8,13 @@ import { DropdownPosition, DropdownToggle, DropdownItem, - TextInput as PFTextInput, + Form, + FormGroup, + TextInput as PFTextInput } from '@patternfly/react-core'; -import { SearchIcon } from '@patternfly/react-icons'; +import { + SearchIcon +} from '@patternfly/react-icons'; import styled from 'styled-components'; @@ -25,8 +29,7 @@ const Button = styled(PFButton)` `; const Dropdown = styled(PFDropdown)` - &&& { - /* Higher specificity required because we are selecting unclassed elements */ + &&& { /* Higher specificity required because we are selecting unclassed elements */ > button { min-height: 30px; min-width: 70px; @@ -34,22 +37,31 @@ const Dropdown = styled(PFDropdown)` padding: 0 10px; margin: 0px; - > span { - /* text element */ + > span { /* text element */ width: auto; } - > svg { - /* caret icon */ + > svg { /* caret icon */ margin: 0px; padding-top: 3px; padding-left: 3px; } } - } + } `; + +const NoOptionDropdown = styled.div` + align-self: stretch; + border: 1px solid grey; + padding: 3px 7px; +`; + +const InputFormGroup = styled(FormGroup)` + flex: 1; +`; + class Search extends React.Component { - constructor(props) { + constructor (props) { super(props); const { sortedColumnKey } = this.props; @@ -65,11 +77,11 @@ class Search extends React.Component { this.handleSearch = this.handleSearch.bind(this); } - handleDropdownToggle(isSearchDropdownOpen) { + handleDropdownToggle (isSearchDropdownOpen) { this.setState({ isSearchDropdownOpen }); } - handleDropdownSelect({ target }) { + handleDropdownSelect ({ target }) { const { columns } = this.props; const { innerText } = target; @@ -77,28 +89,38 @@ class Search extends React.Component { this.setState({ isSearchDropdownOpen: false, searchKey }); } - handleSearch() { - const { searchValue } = this.state; + handleSearch (e) { + // keeps page from fully reloading + e.preventDefault(); + + const { searchKey, searchValue } = this.state; const { onSearch } = this.props; - onSearch(searchValue); + // TODO: probably not _always_ add icontains. I don't think icontains works for numbers. + onSearch(`${searchKey}__icontains`, searchValue); + + this.setState({ searchValue: '' }); } - handleSearchInputChange(searchValue) { + handleSearchInputChange (searchValue) { this.setState({ searchValue }); } - render() { + render () { const { up } = DropdownPosition; - const { columns, i18n } = this.props; - const { isSearchDropdownOpen, searchKey, searchValue } = this.state; - - const { name: searchColumnName } = columns.find( - ({ key }) => key === searchKey - ); + const { + columns, + i18n + } = this.props; + const { + isSearchDropdownOpen, + searchKey, + searchValue, + } = this.state; + const { name: searchColumnName } = columns.find(({ key }) => key === searchKey); const searchDropdownItems = columns - .filter(({ key }) => key !== searchKey) + .filter(({ key, isSearchable }) => isSearchable && key !== searchKey) .map(({ key, name }) => ( {name} @@ -106,37 +128,56 @@ class Search extends React.Component { )); return ( -
    - +
    + {searchDropdownItems.length > 0 ? ( + {i18n._(t`Search key dropdown`)})} > + + {searchColumnName} + + )} + dropdownItems={searchDropdownItems} + /> + + ) : ( + {searchColumnName} - - } - dropdownItems={searchDropdownItems} - /> - - -
    + + )} + {i18n._(t`Search value text input`)})} + > + + + +
    + ); } } @@ -149,7 +190,7 @@ Search.propTypes = { Search.defaultProps = { onSearch: null, - sortedColumnKey: 'name', + sortedColumnKey: 'name' }; export default withI18n()(Search); diff --git a/awx/ui_next/src/components/Search/Search.test.jsx b/awx/ui_next/src/components/Search/Search.test.jsx index 3b7f1816c2..e59716b9fe 100644 --- a/awx/ui_next/src/components/Search/Search.test.jsx +++ b/awx/ui_next/src/components/Search/Search.test.jsx @@ -12,15 +12,19 @@ describe('', () => { }); test('it triggers the expected callbacks', () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true }]; + const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; - const searchBtn = 'button[aria-label="Search"]'; + const searchBtn = 'button[aria-label="Search submit button"]'; const searchTextInput = 'input[aria-label="Search text input"]'; const onSearch = jest.fn(); search = mountWithContexts( - + ); search.find(searchTextInput).instance().value = 'test-321'; @@ -28,14 +32,18 @@ describe('', () => { search.find(searchBtn).simulate('click'); expect(onSearch).toHaveBeenCalledTimes(1); - expect(onSearch).toBeCalledWith('test-321'); + expect(onSearch).toBeCalledWith('name__icontains', 'test-321'); }); test('handleDropdownToggle properly updates state', async () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true }]; + const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; const onSearch = jest.fn(); const wrapper = mountWithContexts( - + ).find('Search'); expect(wrapper.state('isSearchDropdownOpen')).toEqual(false); wrapper.instance().handleDropdownToggle(true); @@ -44,17 +52,19 @@ describe('', () => { test('handleDropdownSelect properly updates state', async () => { const columns = [ - { name: 'Name', key: 'name', isSortable: true }, - { name: 'Description', key: 'description', isSortable: true }, + { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, + { name: 'Description', key: 'description', isSortable: true, isSearchable: true } ]; const onSearch = jest.fn(); const wrapper = mountWithContexts( - + ).find('Search'); expect(wrapper.state('searchKey')).toEqual('name'); - wrapper - .instance() - .handleDropdownSelect({ target: { innerText: 'Description' } }); + wrapper.instance().handleDropdownSelect({ target: { innerText: 'Description' } }); expect(wrapper.state('searchKey')).toEqual('description'); }); }); diff --git a/awx/ui_next/src/components/Sort/Sort.test.jsx b/awx/ui_next/src/components/Sort/Sort.test.jsx index d23c75fa2f..b979ed4ee0 100644 --- a/awx/ui_next/src/components/Sort/Sort.test.jsx +++ b/awx/ui_next/src/components/Sort/Sort.test.jsx @@ -12,7 +12,7 @@ describe('', () => { }); test('it triggers the expected callbacks', () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true }]; + const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; const sortBtn = 'button[aria-label="Sort"]'; @@ -38,7 +38,7 @@ describe('', () => { { name: 'Foo', key: 'foo', isSortable: true }, { name: 'Bar', key: 'bar', isSortable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' }, + { name: 'Baz', key: 'baz' } ]; const onSort = jest.fn(); @@ -62,7 +62,7 @@ describe('', () => { { name: 'Foo', key: 'foo', isSortable: true }, { name: 'Bar', key: 'bar', isSortable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' }, + { name: 'Baz', key: 'baz' } ]; const onSort = jest.fn(); @@ -86,7 +86,7 @@ describe('', () => { { name: 'Foo', key: 'foo', isSortable: true }, { name: 'Bar', key: 'bar', isSortable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' }, + { name: 'Baz', key: 'baz' } ]; const onSort = jest.fn(); @@ -109,7 +109,7 @@ describe('', () => { { name: 'Foo', key: 'foo', isSortable: true }, { name: 'Bar', key: 'bar', isSortable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' }, + { name: 'Baz', key: 'baz' } ]; const onSort = jest.fn(); @@ -133,12 +133,8 @@ describe('', () => { const downAlphaIconSelector = 'SortAlphaDownIcon'; const upAlphaIconSelector = 'SortAlphaUpIcon'; - const numericColumns = [ - { name: 'ID', key: 'id', isSortable: true, isNumeric: true }, - ]; - const alphaColumns = [ - { name: 'Name', key: 'name', isSortable: true, isNumeric: false }, - ]; + const numericColumns = [{ name: 'ID', key: 'id', isSortable: true, isNumeric: true }]; + const alphaColumns = [{ name: 'Name', key: 'name', isSortable: true, isNumeric: false }]; const onSort = jest.fn(); sort = mountWithContexts( diff --git a/awx/ui_next/src/screens/Job/JobList/JobList.jsx b/awx/ui_next/src/screens/Job/JobList/JobList.jsx index f519639c94..62ca1eb5b5 100644 --- a/awx/ui_next/src/screens/Job/JobList/JobList.jsx +++ b/awx/ui_next/src/screens/Job/JobList/JobList.jsx @@ -2,16 +2,19 @@ import React, { Component } from 'react'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { Card, PageSection, PageSectionVariants } from '@patternfly/react-core'; +import { + Card, + PageSection, + PageSectionVariants, +} from '@patternfly/react-core'; import { UnifiedJobsAPI } from '@api'; import AlertModal from '@components/AlertModal'; import DatalistToolbar from '@components/DataListToolbar'; -import ErrorDetail from '@components/ErrorDetail'; import PaginatedDataList, { - ToolbarDeleteButton, + ToolbarDeleteButton } from '@components/PaginatedDataList'; -import { getQSConfig, parseNamespacedQueryString } from '@util/qs'; +import { getQSConfig, parseQueryString } from '@util/qs'; import JobListItem from './JobListItem'; @@ -23,13 +26,13 @@ const QS_CONFIG = getQSConfig('job', { }); class JobList extends Component { - constructor(props) { + constructor (props) { super(props); this.state = { hasContentLoading: true, - contentError: null, - deletionError: null, + hasContentError: false, + deletionError: false, selected: [], jobs: [], itemCount: 0, @@ -41,28 +44,28 @@ class JobList extends Component { this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this); } - componentDidMount() { + componentDidMount () { this.loadJobs(); } - componentDidUpdate(prevProps) { + componentDidUpdate (prevProps) { const { location } = this.props; if (location !== prevProps.location) { this.loadJobs(); } } - handleDeleteErrorClose() { - this.setState({ deletionError: null }); + handleDeleteErrorClose () { + this.setState({ deletionError: false }); } - handleSelectAll(isSelected) { + handleSelectAll (isSelected) { const { jobs } = this.state; const selected = isSelected ? [...jobs] : []; this.setState({ selected }); } - handleSelect(item) { + handleSelect (item) { const { selected } = this.state; if (selected.some(s => s.id === item.id)) { this.setState({ selected: selected.filter(s => s.id !== item.id) }); @@ -71,49 +74,50 @@ class JobList extends Component { } } - async handleDelete() { + async handleDelete () { const { selected } = this.state; - this.setState({ hasContentLoading: true }); + this.setState({ hasContentLoading: true, deletionError: false }); try { await Promise.all(selected.map(({ id }) => UnifiedJobsAPI.destroy(id))); } catch (err) { - this.setState({ deletionError: err }); + this.setState({ deletionError: true }); } finally { await this.loadJobs(); } } - async loadJobs() { + async loadJobs () { const { location } = this.props; - const params = parseNamespacedQueryString(QS_CONFIG, location.search); + const params = parseQueryString(QS_CONFIG, location.search); - this.setState({ contentError: null, hasContentLoading: true }); + this.setState({ hasContentError: false, hasContentLoading: true }); try { - const { - data: { count, results }, - } = await UnifiedJobsAPI.read(params); + const { data: { count, results } } = await UnifiedJobsAPI.read(params); this.setState({ itemCount: count, jobs: results, selected: [], }); } catch (err) { - this.setState({ contentError: err }); + this.setState({ hasContentError: true }); } finally { this.setState({ hasContentLoading: false }); } } - render() { + render () { const { - contentError, + hasContentError, hasContentLoading, deletionError, jobs, itemCount, selected, } = this.state; - const { match, i18n } = this.props; + const { + match, + i18n + } = this.props; const { medium } = PageSectionVariants; const isAllSelected = selected.length === jobs.length; const itemName = i18n._(t`Job`); @@ -121,22 +125,17 @@ class JobList extends Component { ( + renderToolbar={(props) => ( , + /> ]} /> )} - renderItem={job => ( + renderItem={(job) => ( {i18n._(t`Failed to delete one or more jobs.`)} - ); diff --git a/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.jsx b/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.jsx index 6961cf9fd5..4ce28f4393 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.jsx @@ -7,14 +7,11 @@ import { OrganizationsAPI, TeamsAPI, UsersAPI } from '@api'; import AddResourceRole from '@components/AddRole/AddResourceRole'; import AlertModal from '@components/AlertModal'; import DataListToolbar from '@components/DataListToolbar'; -import ErrorDetail from '@components/ErrorDetail'; -import PaginatedDataList, { - ToolbarAddButton, -} from '@components/PaginatedDataList'; +import PaginatedDataList, { ToolbarAddButton } from '@components/PaginatedDataList'; import { getQSConfig, encodeQueryString, - parseNamespacedQueryString, + parseQueryString } from '@util/qs'; import { Organization } from '@types'; @@ -32,13 +29,13 @@ class OrganizationAccess extends React.Component { organization: Organization.isRequired, }; - constructor(props) { + constructor (props) { super(props); this.state = { accessRecords: [], - contentError: null, + hasContentError: false, hasContentLoading: true, - deletionError: null, + hasDeletionError: false, deletionRecord: null, deletionRole: null, isAddModalOpen: false, @@ -54,61 +51,58 @@ class OrganizationAccess extends React.Component { this.handleDeleteOpen = this.handleDeleteOpen.bind(this); } - componentDidMount() { + componentDidMount () { this.loadAccessList(); } - componentDidUpdate(prevProps) { + componentDidUpdate (prevProps) { const { location } = this.props; - const prevParams = parseNamespacedQueryString( - QS_CONFIG, - prevProps.location.search - ); - const currentParams = parseNamespacedQueryString( - QS_CONFIG, - location.search - ); + const prevParams = parseQueryString(QS_CONFIG, prevProps.location.search); + const currentParams = parseQueryString(QS_CONFIG, location.search); if (encodeQueryString(currentParams) !== encodeQueryString(prevParams)) { this.loadAccessList(); } } - async loadAccessList() { + async loadAccessList () { const { organization, location } = this.props; - const params = parseNamespacedQueryString(QS_CONFIG, location.search); + const params = parseQueryString(QS_CONFIG, location.search); - this.setState({ contentError: null, hasContentLoading: true }); + this.setState({ hasContentError: false, hasContentLoading: true }); try { const { - data: { results: accessRecords = [], count: itemCount = 0 }, + data: { + results: accessRecords = [], + count: itemCount = 0 + } } = await OrganizationsAPI.readAccessList(organization.id, params); this.setState({ itemCount, accessRecords }); - } catch (err) { - this.setState({ contentError: err }); + } catch (error) { + this.setState({ hasContentError: true }); } finally { this.setState({ hasContentLoading: false }); } } - handleDeleteOpen(deletionRole, deletionRecord) { + handleDeleteOpen (deletionRole, deletionRecord) { this.setState({ deletionRole, deletionRecord }); } - handleDeleteCancel() { + handleDeleteCancel () { this.setState({ deletionRole: null, deletionRecord: null }); } - handleDeleteErrorClose() { + handleDeleteErrorClose () { this.setState({ - deletionError: null, + hasDeletionError: false, deletionRecord: null, - deletionRole: null, + deletionRole: null }); } - async handleDeleteConfirm() { + async handleDeleteConfirm () { const { deletionRole, deletionRecord } = this.state; if (!deletionRole || !deletionRecord) { @@ -117,10 +111,7 @@ class OrganizationAccess extends React.Component { let promise; if (typeof deletionRole.team_id !== 'undefined') { - promise = TeamsAPI.disassociateRole( - deletionRole.team_id, - deletionRole.id - ); + promise = TeamsAPI.disassociateRole(deletionRole.team_id, deletionRole.id); } else { promise = UsersAPI.disassociateRole(deletionRecord.id, deletionRole.id); } @@ -130,72 +121,64 @@ class OrganizationAccess extends React.Component { await promise.then(this.loadAccessList); this.setState({ deletionRole: null, - deletionRecord: null, + deletionRecord: null }); - } catch (err) { + } catch (error) { this.setState({ hasContentLoading: false, - deletionError: err, + hasDeletionError: true }); } } - handleAddClose() { + handleAddClose () { this.setState({ isAddModalOpen: false }); } - handleAddOpen() { + handleAddOpen () { this.setState({ isAddModalOpen: true }); } - handleAddSuccess() { + handleAddSuccess () { this.setState({ isAddModalOpen: false }); this.loadAccessList(); } - render() { + render () { const { organization, i18n } = this.props; const { accessRecords, - contentError, + hasContentError, hasContentLoading, deletionRole, deletionRecord, - deletionError, + hasDeletionError, itemCount, isAddModalOpen, } = this.state; const canEdit = organization.summary_fields.user_capabilities.edit; - const isDeleteModalOpen = - !hasContentLoading && !deletionError && deletionRole; + const isDeleteModalOpen = !hasContentLoading && !hasDeletionError && deletionRole; return ( ( + renderToolbar={(props) => ( , - ] - : null - } + additionalControls={canEdit ? [ + + ] : null} /> )} renderItem={accessRecord => ( @@ -222,13 +205,12 @@ class OrganizationAccess extends React.Component { /> )} {i18n._(t`Failed to delete role`)} - ); diff --git a/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx index b0d0b1bb33..12dd0dbc68 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx @@ -103,7 +103,7 @@ describe('', () => { expect(wrapper.find('OrganizationAccess').state('hasContentLoading')).toBe( false ); - expect(wrapper.find('OrganizationAccess').state('contentError')).toBe(null); + expect(wrapper.find('OrganizationAccess').state('hasContentError')).toBe(false); done(); }); diff --git a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap index 321c03c4eb..1eb2c6cc4f 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap +++ b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap @@ -34,7 +34,7 @@ exports[` initially renders succesfully 1`] = ` } > initially renders succesfully 1`] = ` toolbarColumns={ Array [ Object { + "isSearchable": true, "isSortable": true, "key": "first_name", - "name": "Name", + "name": "First Name", }, Object { + "isSearchable": true, "isSortable": true, "key": "username", "name": "Username", }, Object { + "isSearchable": true, "isSortable": true, "key": "last_name", "name": "Last Name", @@ -80,7 +83,7 @@ exports[` initially renders succesfully 1`] = ` withHash={true} > initially renders succesfully 1`] = ` toolbarColumns={ Array [ Object { + "isSearchable": true, "isSortable": true, "key": "first_name", - "name": "Name", + "name": "First Name", }, Object { + "isSearchable": true, "isSortable": true, "key": "username", "name": "Username", }, Object { + "isSearchable": true, "isSortable": true, "key": "last_name", "name": "Last Name", @@ -124,8 +130,7 @@ exports[` initially renders succesfully 1`] = ` > initially renders succesfully 1`] = ` toolbarColumns={ Array [ Object { + "isSearchable": true, "isSortable": true, "key": "first_name", - "name": "Name", + "name": "First Name", }, Object { + "isSearchable": true, "isSortable": true, "key": "username", "name": "Username", }, Object { + "isSearchable": true, "isSortable": true, "key": "last_name", "name": "Last Name", @@ -186,6 +194,222 @@ exports[` initially renders succesfully 1`] = ` ] } > + + + + + :not(:first-child){margin-left:20px;}", + ], + }, + "displayName": "ListHeader__EmptyStateControlsWrapper", + "foldedComponentIds": Array [], + "render": [Function], + "styledComponentId": "ListHeader__EmptyStateControlsWrapper-sc-1gm7wbv-0", + "target": "div", + "toString": [Function], + "warnTooManyClasses": [Function], + "withComponent": [Function], + } + } + forwardedRef={null} + > +
    + + + + + + + + + + + + + + initially renders succesfully 1`] = ` <_default - isOpen={null} + isOpen={false} onClose={[Function]} title="Error!" variant="danger" diff --git a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap index c1b9903e68..3a85a5f874 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap +++ b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap @@ -695,9 +695,9 @@ exports[` initially renders succesfully 1`] = ` "componentStyle": ComponentStyle { "componentId": "ChipGroup-sc-10zu8t0-0", "isStatic": true, - "lastClassName": "vIGxT", + "lastClassName": "ldAQsx", "rules": Array [ - "--pf-c-chip-group--c-chip--MarginRight:10px;--pf-c-chip-group--c-chip--MarginBottom:10px;", + "--pf-c-chip-group--c-chip--MarginRight:10px;--pf-c-chip-group--c-chip--MarginBottom:10px;> .pf-c-chip.pf-m-overflow button{padding:3px 8px;}", ], }, "displayName": "ChipGroup", @@ -713,11 +713,12 @@ exports[` initially renders succesfully 1`] = ` forwardedRef={null} >
      s.id === row.id)) { @@ -71,28 +74,27 @@ class OrganizationsList extends Component { } } - handleDeleteErrorClose() { - this.setState({ deletionError: null }); + handleDeleteErrorClose () { + this.setState({ hasDeletionError: false }); } - async handleOrgDelete() { - const { selected, itemCount } = this.state; + async handleOrgDelete () { + const { selected } = this.state; - this.setState({ hasContentLoading: true }); + this.setState({ hasContentLoading: true, hasDeletionError: false }); try { - await Promise.all(selected.map(org => OrganizationsAPI.destroy(org.id))); - this.setState({ itemCount: itemCount - selected.length }); + await Promise.all(selected.map((org) => OrganizationsAPI.destroy(org.id))); } catch (err) { - this.setState({ deletionError: err }); + this.setState({ hasDeletionError: true }); } finally { await this.loadOrganizations(); } } - async loadOrganizations() { + async loadOrganizations () { const { location } = this.props; const { actions: cachedActions } = this.state; - const params = parseNamespacedQueryString(QS_CONFIG, location.search); + const params = parseQueryString(QS_CONFIG, location.search); let optionsPromise; if (cachedActions) { @@ -106,16 +108,9 @@ class OrganizationsList extends Component { optionsPromise, ]); - this.setState({ contentError: null, hasContentLoading: true }); + this.setState({ hasContentError: false, hasContentLoading: true }); try { - const [ - { - data: { count, results }, - }, - { - data: { actions }, - }, - ] = await promises; + const [{ data: { count, results } }, { data: { actions } }] = await promises; this.setState({ actions, itemCount: count, @@ -123,27 +118,28 @@ class OrganizationsList extends Component { selected: [], }); } catch (err) { - this.setState({ contentError: err }); + this.setState(({ hasContentError: true })); } finally { this.setState({ hasContentLoading: false }); } } - render() { - const { medium } = PageSectionVariants; + render () { + const { + medium, + } = PageSectionVariants; const { actions, itemCount, - contentError, + hasContentError, hasContentLoading, - deletionError, + hasDeletionError, selected, organizations, } = this.state; const { match, i18n } = this.props; - const canAdd = - actions && Object.prototype.hasOwnProperty.call(actions, 'POST'); + const canAdd = actions && Object.prototype.hasOwnProperty.call(actions, 'POST'); const isAllSelected = selected.length === organizations.length; return ( @@ -151,28 +147,18 @@ class OrganizationsList extends Component { ( + renderToolbar={(props) => ( , - canAdd ? ( - - ) : null, + canAdd + ? + : null, ]} /> )} - renderItem={o => ( + renderItem={(o) => ( )} emptyStateControls={ - canAdd ? ( - - ) : null + canAdd ? + : null } /> {i18n._(t`Failed to delete one or more organizations.`)} - ); diff --git a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx index ec0334d3a9..2abe9941c1 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx @@ -9,7 +9,7 @@ import AlertModal from '@components/AlertModal'; import ErrorDetail from '@components/ErrorDetail'; import NotificationListItem from '@components/NotificationsList/NotificationListItem'; import PaginatedDataList from '@components/PaginatedDataList'; -import { getQSConfig, parseNamespacedQueryString } from '@util/qs'; +import { getQSConfig, parseQueryString } from '@util/qs'; const QS_CONFIG = getQSConfig('notification', { page: 1, @@ -18,18 +18,18 @@ const QS_CONFIG = getQSConfig('notification', { }); const COLUMNS = [ - { key: 'name', name: 'Name', isSortable: true }, + { key: 'name', name: 'Name', isSortable: true, isSearchable: true }, { key: 'modified', name: 'Modified', isSortable: true, isNumeric: true }, { key: 'created', name: 'Created', isSortable: true, isNumeric: true }, ]; class OrganizationNotifications extends Component { - constructor(props) { + constructor (props) { super(props); this.state = { contentError: null, hasContentLoading: true, - toggleError: null, + toggleError: false, toggleLoading: false, itemCount: 0, notifications: [], @@ -38,27 +38,25 @@ class OrganizationNotifications extends Component { typeLabels: null, }; this.handleNotificationToggle = this.handleNotificationToggle.bind(this); - this.handleNotificationErrorClose = this.handleNotificationErrorClose.bind( - this - ); + this.handleNotificationErrorClose = this.handleNotificationErrorClose.bind(this); this.loadNotifications = this.loadNotifications.bind(this); } - componentDidMount() { + componentDidMount () { this.loadNotifications(); } - componentDidUpdate(prevProps) { + componentDidUpdate (prevProps) { const { location } = this.props; if (location !== prevProps.location) { this.loadNotifications(); } } - async loadNotifications() { + async loadNotifications () { const { id, location } = this.props; const { typeLabels } = this.state; - const params = parseNamespacedQueryString(QS_CONFIG, location.search); + const params = parseQueryString(QS_CONFIG, location.search); const promises = [OrganizationsAPI.readNotificationTemplates(id, params)]; @@ -68,12 +66,14 @@ class OrganizationNotifications extends Component { this.setState({ contentError: null, hasContentLoading: true }); try { - const [ - { - data: { count: itemCount = 0, results: notifications = [] }, - }, - optionsResponse, - ] = await Promise.all(promises); + const { + data: { + count: itemCount = 0, + results: notifications = [], + } + } = await OrganizationsAPI.readNotificationTemplates(id, params); + + const optionsResponse = await OrganizationsAPI.readOptionsNotificationTemplates(id, params); let idMatchParams; if (notifications.length > 0) { @@ -122,7 +122,7 @@ class OrganizationNotifications extends Component { } } - async handleNotificationToggle(notificationId, isCurrentlyOn, status) { + async handleNotificationToggle (notificationId, isCurrentlyOn, status) { const { id } = this.props; let stateArrayName; @@ -135,15 +135,13 @@ class OrganizationNotifications extends Component { let stateUpdateFunction; if (isCurrentlyOn) { // when switching off, remove the toggled notification id from the array - stateUpdateFunction = prevState => ({ - [stateArrayName]: prevState[stateArrayName].filter( - i => i !== notificationId - ), + stateUpdateFunction = (prevState) => ({ + [stateArrayName]: prevState[stateArrayName].filter(i => i !== notificationId) }); } else { // when switching on, add the toggled notification id to the array - stateUpdateFunction = prevState => ({ - [stateArrayName]: prevState[stateArrayName].concat(notificationId), + stateUpdateFunction = (prevState) => ({ + [stateArrayName]: prevState[stateArrayName].concat(notificationId) }); } @@ -157,17 +155,17 @@ class OrganizationNotifications extends Component { ); this.setState(stateUpdateFunction); } catch (err) { - this.setState({ toggleError: err }); + this.setState({ toggleError: true }); } finally { this.setState({ toggleLoading: false }); } } - handleNotificationErrorClose() { - this.setState({ toggleError: null }); + handleNotificationErrorClose () { + this.setState({ toggleError: false }); } - render() { + render () { const { canToggleNotifications, i18n } = this.props; const { contentError, @@ -178,7 +176,7 @@ class OrganizationNotifications extends Component { notifications, successTemplateIds, errorTemplateIds, - typeLabels, + typeLabels } = this.state; return ( @@ -191,7 +189,7 @@ class OrganizationNotifications extends Component { itemName="notification" qsConfig={QS_CONFIG} toolbarColumns={COLUMNS} - renderItem={notification => ( + renderItem={(notification) => ( {i18n._(t`Failed to toggle notification.`)} diff --git a/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap b/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap index dd84655a06..aaef95f74a 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap +++ b/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap @@ -77,6 +77,7 @@ exports[` initially renders succesfully 1`] = ` toolbarColumns={ Array [ Object { + "isSearchable": true, "isSortable": true, "key": "name", "name": "Name", @@ -140,6 +141,7 @@ exports[` initially renders succesfully 1`] = ` toolbarColumns={ Array [ Object { + "isSearchable": true, "isSortable": true, "key": "name", "name": "Name", @@ -162,6 +164,7 @@ exports[` initially renders succesfully 1`] = ` initially renders succesfully 1`] = ` toolbarColumns={ Array [ Object { + "isSearchable": true, "isSortable": true, "key": "name", "name": "Name", @@ -239,10 +243,11 @@ exports[` initially renders succesfully 1`] = ` ] } > - initially renders succesfully 1`] = ` }, ] } - onSearch={[Function]} - onSort={[Function]} - sortOrder="ascending" - sortedColumnKey="name" + itemCount={2} + qsConfig={ + Object { + "defaultParams": Object { + "order_by": "name", + "page": 1, + "page_size": 5, + }, + "integerFields": Array [ + "page", + "page_size", + ], + "namespace": "notification", + } + } + renderToolbar={[Function]} > - - + initially renders succesfully 1`] = ` }, ] } - fillWidth={false} - i18n={"/i18n/"} - isAllSelected={false} - isCompact={false} - onCompact={null} - onExpand={null} - onSearch={[Function]} - onSelectAll={null} - onSort={[Function]} - showSelectAll={false} - sortOrder="ascending" - sortedColumnKey="name" + history={"/history/"} + itemCount={2} + location={ + Object { + "hash": "", + "pathname": "", + "search": "", + "state": "", + } + } + match={ + Object { + "isExact": false, + "params": Object {}, + "path": "", + "url": "", + } + } + qsConfig={ + Object { + "defaultParams": Object { + "order_by": "name", + "page": 1, + "page_size": 5, + }, + "integerFields": Array [ + "page", + "page_size", + ], + "namespace": "notification", + } + } + renderToolbar={[Function]} > - - + -
      - + initially renders succesfully 1`] = ` } forwardedRef={null} > - -
      - - +
      - + initially renders succesfully 1`] = ` } forwardedRef={null} > - -
      - + - - -
      - + +
      + +
      + + +
      + Name +
      +
      +
      + + Search value text input + + } + > + + Search value text input + + } + > + + Search value text input + + } + > +
      + + + + + + + + +
      +
      +
      +
      + + + + + + +
      +
      + +
      + + +
      + +
      + + +
      + + + + + +
      +
      +
      +
      +
      + + +
      + +
      + + + + initially renders succesfully 1`] = ` isOpen={false} onSelect={[Function]} onToggle={[Function]} + style={ + Object { + "marginRight": "20px", + } + } toggle={ initially renders succesfully 1`] = ` "$$typeof": Symbol(react.forward_ref), "attrs": Array [], "componentStyle": ComponentStyle { - "componentId": "Search__Dropdown-sc-1dwuww3-2", + "componentId": "Sort__Dropdown-sc-21g5aw-0", "isStatic": true, - "lastClassName": "kcnywV", + "lastClassName": "kdSQuN", "rules": Array [ "&&&{> button{min-height:30px;min-width:70px;height:30px;padding:0 10px;margin:0px;> span{width:auto;}> svg{margin:0px;padding-top:3px;padding-left:3px;}}}", ], }, - "displayName": "Search__Dropdown", + "displayName": "Sort__Dropdown", "foldedComponentIds": Array [], "render": [Function], - "styledComponentId": "Search__Dropdown-sc-1dwuww3-2", + "styledComponentId": "Sort__Dropdown-sc-21g5aw-0", "target": [Function], "toString": [Function], "warnTooManyClasses": [Function], @@ -586,11 +1123,16 @@ exports[` initially renders succesfully 1`] = ` isOpen={false} onSelect={[Function]} onToggle={[Function]} + style={ + Object { + "marginRight": "20px", + } + } toggle={ initially renders succesfully 1`] = ` } > initially renders succesfully 1`] = ` onSelect={[Function]} onToggle={[Function]} position="left" + style={ + Object { + "marginRight": "20px", + } + } toggle={ initially renders succesfully 1`] = ` } >
      initially renders succesfully 1`] = ` onToggle={[Function]} parentRef={
      - - - - - - - - - + initially renders succesfully 1`] = ` } forwardedRef={null} onClick={[Function]} - variant="tertiary" + variant="plain" > - -
      - -
      -
      -
      -
      - - - -
      - - + + + +
      + + + :not(:first-child){margin-left:20px;}", + ], + }, + "displayName": "DataListToolbar__AdditionalControlsWrapper", + "foldedComponentIds": Array [], + "render": [Function], + "styledComponentId": "DataListToolbar__AdditionalControlsWrapper-ajzso8-5", + "target": "div", + "toString": [Function], + "warnTooManyClasses": [Function], + "withComponent": [Function], + } } - } - forwardedRef={null} - > - - - -
      - -
      -
      -
      - - -
      - -
      - - - - - Modified - , - - Created - , - ] - } - isOpen={false} - onSelect={[Function]} - onToggle={[Function]} - style={ - Object { - "marginRight": "20px", - } - } - toggle={ - - Name - - } - > - - Modified - , - - Created - , - ] - } - forwardedComponent={ - Object { - "$$typeof": Symbol(react.forward_ref), - "attrs": Array [], - "componentStyle": ComponentStyle { - "componentId": "Sort__Dropdown-sc-21g5aw-0", - "isStatic": true, - "lastClassName": "kdSQuN", - "rules": Array [ - "&&&{> button{min-height:30px;min-width:70px;height:30px;padding:0 10px;margin:0px;> span{width:auto;}> svg{margin:0px;padding-top:3px;padding-left:3px;}}}", - ], - }, - "displayName": "Sort__Dropdown", - "foldedComponentIds": Array [], - "render": [Function], - "styledComponentId": "Sort__Dropdown-sc-21g5aw-0", - "target": [Function], - "toString": [Function], - "warnTooManyClasses": [Function], - "withComponent": [Function], - } - } - forwardedRef={null} - isOpen={false} - onSelect={[Function]} - onToggle={[Function]} - style={ - Object { - "marginRight": "20px", - } - } - toggle={ - - Name - - } - > - - Modified - , - - Created - , - ] - } - isOpen={false} - isPlain={false} - onSelect={[Function]} - onToggle={[Function]} - position="left" - style={ - Object { - "marginRight": "20px", - } - } - toggle={ - - Name - - } - > -
      - - -
      - } - > - - -
      - } - > - - - -
      - -
      - - - - - - - - - - -
      - - - :not(:first-child){margin-left:20px;}", - ], - }, - "displayName": "DataListToolbar__AdditionalControlsWrapper", - "foldedComponentIds": Array [], - "render": [Function], - "styledComponentId": "DataListToolbar__AdditionalControlsWrapper-ajzso8-5", - "target": "div", - "toString": [Function], - "warnTooManyClasses": [Function], - "withComponent": [Function], - } - } - forwardedRef={null} - > -
      +
      + + +
      - +
      -
      - -
      - + +
      + +
    - -
    - - -
    - - + +
    + + + + + + + + + + + + + + initially renders succesfully 1`] = ` <_default - isOpen={null} + isOpen={false} onClose={[Function]} title="Error!" variant="danger" diff --git a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx index 89dcadc219..faa9d4b359 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx @@ -4,7 +4,7 @@ import { withRouter } from 'react-router-dom'; import { OrganizationsAPI } from '@api'; import PaginatedDataList from '@components/PaginatedDataList'; -import { getQSConfig, parseNamespacedQueryString } from '@util/qs'; +import { getQSConfig, parseQueryString } from '@util/qs'; const QS_CONFIG = getQSConfig('team', { page: 1, @@ -39,7 +39,7 @@ class OrganizationTeams extends React.Component { async loadOrganizationTeamsList() { const { id, location } = this.props; - const params = parseNamespacedQueryString(QS_CONFIG, location.search); + const params = parseQueryString(QS_CONFIG, location.search); this.setState({ hasContentLoading: true, contentError: null }); try { diff --git a/awx/ui_next/src/screens/Organization/shared/InstanceGroupsLookup.jsx b/awx/ui_next/src/screens/Organization/shared/InstanceGroupsLookup.jsx index 66ed5f706b..5e7dfa38d4 100644 --- a/awx/ui_next/src/screens/Organization/shared/InstanceGroupsLookup.jsx +++ b/awx/ui_next/src/screens/Organization/shared/InstanceGroupsLookup.jsx @@ -8,24 +8,30 @@ import { QuestionCircleIcon } from '@patternfly/react-icons'; import { InstanceGroupsAPI } from '@api'; import Lookup from '@components/Lookup'; -const getInstanceGroups = async params => InstanceGroupsAPI.read(params); +const getInstanceGroups = async (params) => InstanceGroupsAPI.read(params); class InstanceGroupsLookup extends React.Component { - render() { + render () { const { value, tooltip, onChange, i18n } = this.props; return ( - {i18n._(t`Instance Groups`)}{' '} - {tooltip && ( - - - - )} + {i18n._(t`Instance Groups`)} + {' '} + { + tooltip && ( + + + + ) + } - } + )} fieldId="org-instance-groups" > diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index 93c3352790..ebc9b3b81c 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -2,20 +2,23 @@ import React, { Component } from 'react'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { Card, PageSection, PageSectionVariants } from '@patternfly/react-core'; +import { + Card, + PageSection, + PageSectionVariants, +} from '@patternfly/react-core'; import { JobTemplatesAPI, UnifiedJobTemplatesAPI, - WorkflowJobTemplatesAPI, + WorkflowJobTemplatesAPI } from '@api'; import AlertModal from '@components/AlertModal'; import DatalistToolbar from '@components/DataListToolbar'; -import ErrorDetail from '@components/ErrorDetail'; import PaginatedDataList, { - ToolbarDeleteButton, + ToolbarDeleteButton } from '@components/PaginatedDataList'; -import { getQSConfig, parseNamespacedQueryString } from '@util/qs'; +import { getQSConfig, parseQueryString } from '@util/qs'; import TemplateListItem from './TemplateListItem'; @@ -25,17 +28,17 @@ const QS_CONFIG = getQSConfig('template', { page: 1, page_size: 5, order_by: 'name', - type: 'job_template,workflow_job_template', + type: 'job_template,workflow_job_template' }); class TemplatesList extends Component { - constructor(props) { + constructor (props) { super(props); this.state = { hasContentLoading: true, - contentError: null, - deletionError: null, + hasContentError: false, + hasDeletionError: false, selected: [], templates: [], itemCount: 0, @@ -47,28 +50,28 @@ class TemplatesList extends Component { this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this); } - componentDidMount() { + componentDidMount () { this.loadTemplates(); } - componentDidUpdate(prevProps) { + componentDidUpdate (prevProps) { const { location } = this.props; if (location !== prevProps.location) { this.loadTemplates(); } } - handleDeleteErrorClose() { - this.setState({ deletionError: null }); + handleDeleteErrorClose () { + this.setState({ hasDeletionError: false }); } - handleSelectAll(isSelected) { + handleSelectAll (isSelected) { const { templates } = this.state; const selected = isSelected ? [...templates] : []; this.setState({ selected }); } - handleSelect(template) { + handleSelect (template) { const { selected } = this.state; if (selected.some(s => s.id === template.id)) { this.setState({ selected: selected.filter(s => s.id !== template.id) }); @@ -77,89 +80,77 @@ class TemplatesList extends Component { } } - async handleTemplateDelete() { - const { selected, itemCount } = this.state; + async handleTemplateDelete () { + const { selected } = this.state; - this.setState({ hasContentLoading: true }); + this.setState({ hasContentLoading: true, hasDeletionError: false }); try { - await Promise.all( - selected.map(({ type, id }) => { - let deletePromise; - if (type === 'job_template') { - deletePromise = JobTemplatesAPI.destroy(id); - } else if (type === 'workflow_job_template') { - deletePromise = WorkflowJobTemplatesAPI.destroy(id); - } - return deletePromise; - }) - ); - this.setState({ itemCount: itemCount - selected.length }); + await Promise.all(selected.map(({ type, id }) => { + let deletePromise; + if (type === 'job_template') { + deletePromise = JobTemplatesAPI.destroy(id); + } else if (type === 'workflow_job_template') { + deletePromise = WorkflowJobTemplatesAPI.destroy(id); + } + return deletePromise; + })); } catch (err) { - this.setState({ deletionError: err }); + this.setState({ hasDeletionError: true }); } finally { await this.loadTemplates(); } } - async loadTemplates() { + async loadTemplates () { const { location } = this.props; - const params = parseNamespacedQueryString(QS_CONFIG, location.search); + const params = parseQueryString(QS_CONFIG, location.search); - this.setState({ contentError: null, hasContentLoading: true }); + this.setState({ hasContentError: false, hasContentLoading: true }); try { - const { - data: { count, results }, - } = await UnifiedJobTemplatesAPI.read(params); + const { data: { count, results } } = await UnifiedJobTemplatesAPI.read(params); this.setState({ itemCount: count, templates: results, selected: [], }); } catch (err) { - this.setState({ contentError: err }); + this.setState({ hasContentError: true }); } finally { this.setState({ hasContentLoading: false }); } } - render() { + render () { const { - contentError, + hasContentError, hasContentLoading, - deletionError, + hasDeletionError, templates, itemCount, selected, } = this.state; - const { match, i18n } = this.props; + const { + match, + i18n + } = this.props; const isAllSelected = selected.length === templates.length; const { medium } = PageSectionVariants; return ( ( + renderToolbar={(props) => ( , + /> ]} /> )} - renderItem={template => ( + renderItem={(template) => ( {i18n._(t`Failed to delete one or more template.`)} - ); diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index b5903cf78d..f02e58f0a4 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -1,55 +1,150 @@ +/** + * helper function used to convert from + * Object.entries format ([ [ key, value ], ... ]) to object + * @param {array} array in the format [ [ key, value ], ...] + * @return {object} object in the forms { key: value, ... } + */ +const toObject = (entriesArr) => entriesArr.reduce((acc, [key, value]) => { + if (acc[key] && Array.isArray(acc[key])) { + acc[key].push(value); + } else if (acc[key]) { + acc[key] = [acc[key], value]; + } else { + acc[key] = value; + } + return acc; +}, {}); + +/** + * helper function to namespace params object + * @param {string} namespace to append to params + * @param {object} params object to append namespace to + * @return {object} params object with namespaced keys + */ +const namespaceParams = (namespace, params = {}) => { + if (!namespace) return params; + + const namespaced = {}; + Object.keys(params).forEach(key => { + namespaced[`${namespace}.${key}`] = params[key]; + }); + + return namespaced || {}; +} + +/** + * helper function to remove namespace from params object + * @param {string} namespace to remove from params + * @param {object} params object to append namespace to + * @return {object} params object with non-namespaced keys + */ +const denamespaceParams = (namespace, params = {}) => { + if (!namespace) return params; + + const denamespaced = {}; + Object.keys(params).forEach(key => { + denamespaced[key.substr(namespace.length + 1)] = params[key]; + }); + + return denamespaced || {}; +} + +/** + * helper function to check the namespace of a param is what you expec + * @param {string} namespace to append to params + * @param {object} params object to append namespace to + * @return {object} params object with namespaced keys + */ +const namespaceMatches = (namespace, fieldname) => { + if (!namespace) return !fieldname.includes('.'); + + return fieldname.startsWith(`${namespace}.`); +} + +/** + * helper function to check the value of a param is equal to another + * @param {string or number or array} param value one + * @param {string or number or array} params value two + * @return {boolean} true if values are equal + */ +const paramValueIsEqual = (one, two) => { + let isEqual = false; + + if (Array.isArray(one) && Array.isArray(two)) { + isEqual = one.filter(val => two.indexOf(val) > -1).length === 0; + } else if ((typeof(one) === "string" && typeof(two) === "string") || + (typeof(one) === "number" && typeof(two) === "number")){ + isEqual = one === two; + } + + return isEqual; +} + /** * Convert query param object to url query string - * + * Used to encode params for interacting with the api + * @param {object} qs config object for namespacing params, filtering defaults * @param {object} query param object * @return {string} url query string */ -export const encodeQueryString = params => { - if (!params) { - return ''; - } +export const encodeQueryString = (params) => { + if (!params) return ''; return Object.keys(params) .sort() .filter(key => params[key] !== null) - .map(key => [key, params[key]]) - .map( - ([key, value]) => - `${encodeURIComponent(key)}=${encodeURIComponent(value)}` - ) + .map(key => ([key, params[key]])) + .map(([key, value]) => { + // if value is array, should return more than one key value pair + if (Array.isArray(value)) { + return value.map(val => `${encodeURIComponent(key)}=${encodeURIComponent(val)}`).join('&'); + } + return `${encodeURIComponent(key)}=${encodeURIComponent(value)}`; + }) .join('&'); }; /** - * Convert url query string to query param object - * - * @param {string} url query string - * @param {object} default params - * @param {array} array of keys to parse as integers - * @return {object} query param object + * Convert query param object to url query string, adding namespace and removing defaults + * Used to put into url bar after ui route + * @param {object} qs config object for namespacing params, filtering defaults + * @param {object} query param object + * @return {string} url query string */ -export const parseQueryString = ( - queryString, - integerFields = ['page', 'page_size'] -) => { - if (!queryString) return {}; +export const encodeNonDefaultQueryString = (config, params) => { + if (!params) return ''; - const keyValuePairs = queryString - .replace(/^\?/, '') - .split('&') - .map(s => s.split('=')) + const namespacedParams = namespaceParams(config.namespace, params); + const namespacedDefaults = namespaceParams(config.namespace, config.defaultParams); + const namespacedDefaultKeys = Object.keys(namespacedDefaults); + const namespacedParamsWithoutDefaultsKeys = Object.keys(namespacedParams) + .filter(key => namespacedDefaultKeys.indexOf(key) === -1 || + !paramValueIsEqual(namespacedParams[key], namespacedDefaults[key])); + + return namespacedParamsWithoutDefaultsKeys + .sort() + .filter(key => namespacedParams[key] !== null) + .map(key => { + return ([key, namespacedParams[key]]) + }) .map(([key, value]) => { - if (integerFields.includes(key)) { - return [key, parseInt(value, 10)]; + // if value is array, should return more than one key value pair + if (Array.isArray(value)) { + return value.map(val => `${encodeURIComponent(key)}=${encodeURIComponent(val)}`).join('&'); } - - return [key, value]; - }); - - return Object.assign(...keyValuePairs.map(([k, v]) => ({ [k]: v }))); + return `${encodeURIComponent(key)}=${encodeURIComponent(value)}`; + }) + .join('&'); }; -export function getQSConfig( +/** + * Returns queryset config with defaults, if needed + * @param {string} namespace for appending to url querystring + * @param {object} default params that are not handled with search (page, page_size and order_by) + * @param {array} params that are number fields + * @return {object} query param object + */ +export function getQSConfig ( namespace, defaultParams = { page: 1, page_size: 5, order_by: 'name' }, integerFields = ['page', 'page_size'] @@ -58,71 +153,207 @@ export function getQSConfig( throw new Error('a QS namespace is required'); } return { - defaultParams, namespace, + defaultParams, integerFields, }; } -export function encodeNamespacedQueryString(config, params) { - return encodeQueryString(namespaceParams(config.namespace, params)); -} +/** + * Convert url query string to query param object + * @param {object} qs config object (used for getting defaults, current query params etc.) + * @param {string} url query string + * @return {object} query param object + */ +export function parseQueryString (config, queryString) { + if (!queryString) return config.defaultParams; -export function parseNamespacedQueryString( - config, - queryString, - includeDefaults = true -) { - const integerFields = prependNamespaceToArray( - config.namespace, - config.integerFields - ); - const parsed = parseQueryString(queryString, integerFields); + const namespacedIntegerFields = config.integerFields.map(f => config.namespace ? `${config.namespace}.${f}` : f); - const namespace = {}; - Object.keys(parsed).forEach(field => { - if (namespaceMatches(config.namespace, field)) { - let fieldname = field; - if (config.namespace) { - fieldname = field.substr(config.namespace.length + 1); + const keyValuePairs = queryString.replace(/^\?/, '').split('&') + .map(s => s.split('=')) + .map(([key, value]) => { + if (namespacedIntegerFields.includes(key)) { + return [decodeURIComponent(key), parseInt(value, 10)]; } - namespace[fieldname] = parsed[field]; - } - }); - return { - ...(includeDefaults ? config.defaultParams : {}), - ...namespace, - }; + + return [decodeURIComponent(key), decodeURIComponent(value)]; + }); + + const keyValueObject = toObject(keyValuePairs); + + // needs to return array for duplicate keys + // ie [[k1, v1], [k1, v2], [k2, v3]] + // -> [[k1, [v1, v2]], [k2, v3]] + const dedupedKeyValuePairs = Object.keys(keyValueObject) + .map(key => { + const values = keyValuePairs + .filter(([k]) => k === key) + .map(([, v]) => v); + + if (values.length === 1) { + return [key, values[0]]; + } + + return [key, values]; + }); + + const parsed = Object.assign(...dedupedKeyValuePairs.map(([k, v]) => ({ + [k]: v + }))); + + const namespacedParams = {}; + + Object.keys(parsed) + .forEach(field => { + if (namespaceMatches(config.namespace, field)) { + let fieldname = field; + if (config.namespace) { + fieldname = field.substr(config.namespace.length + 1); + } + namespacedParams[fieldname] = parsed[field]; + } + }); + + const namespacedDefaults = namespaceParams(config.namespace, config.defaultParams); + + Object.keys(namespacedDefaults) + .filter(key => Object.keys(parsed).indexOf(key) === -1) + .forEach(field => { + if (namespaceMatches(config.namespace, field)) { + let fieldname = field; + if (config.namespace) { + fieldname = field.substr(config.namespace.length + 1); + } + namespacedParams[fieldname] = namespacedDefaults[field]; + } + }); + + return namespacedParams; } -export function updateNamespacedQueryString(config, queryString, newParams) { - const params = parseQueryString(queryString); - return encodeQueryString({ - ...params, - ...namespaceParams(config.namespace, newParams), +/** + * helper function to get params that are defaults + * @param {object} namespaced params object + * @param {object} namespaced params object of default params + * @return {object} namespaced params object of only defaults + */ +const getDefaultParams = (params, defaults) => toObject( + Object.keys(params) + .filter(key => Object.keys(defaults).indexOf(key) > -1) + .map(key => [key, params[key]]) +); + +/** + * helper function to get params that are not defaults + * @param {object} namespaced params object + * @param {object} namespaced params object of default params + * @return {object} namespaced params object of non-defaults + */ +const getNonDefaultParams = (params, defaults) => toObject( + Object.keys(params) + .filter(key => Object.keys(defaults).indexOf(key) === -1) + .map(key => [key, params[key]]) +); + +/** + * helper function to merge old and new params together + * @param {object} namespaced params object old params with defaults filtered out + * @param {object} namespaced params object of new params + * @return {object} merged namespaced params object + */ +const getMergedParams = (oldParams, newParams) => toObject( + Object.keys(oldParams) + .map(key => { + let oldVal = oldParams[key]; + const newVal = newParams[key]; + if (newVal) { + if (Array.isArray(oldVal)) { + oldVal.push(newVal); + } else { + oldVal = [oldVal, newVal]; + } + } + return [key, oldVal]; + }) +); + +/** + * helper function to get new params that are not in merged params + * @param {object} namespaced params object of merged params + * @param {object} namespaced params object of new params + * @return {object} remaining new namespaced params object + */ +const getRemainingNewParams = (mergedParams, newParams) => toObject( + Object.keys(newParams) + .filter(key => Object.keys(mergedParams).indexOf(key) === -1) + .map(key => [key, newParams[key]]) +); + +/** + * Merges existing params of search string with new ones and returns the updated list of params + * @param {object} qs config object (used for getting defaults, current query params etc.) + * @param {string} url query string + * @param {object} object with new params to add + * @return {object} query param object + */ +export function addParams (config, searchString, newParams) { + const namespacedOldParams = namespaceParams( + config.namespace, + parseQueryString(config, searchString) + ); + const namespacedNewParams = namespaceParams(config.namespace, newParams); + const namespacedDefaultParams = namespaceParams(config.namespace, config.defaultParams); + + const namespacedOldParamsNotDefaults = getNonDefaultParams( + namespacedOldParams, + namespacedDefaultParams + ); + const namespacedMergedParams = getMergedParams( + namespacedOldParamsNotDefaults, + namespacedNewParams + ); + + // return updated params. + // If newParams includes updates to the defaults, they will be replaced, + // not concatenated. + return denamespaceParams(config.namespace, { + ...getDefaultParams(namespacedOldParams, namespacedDefaultParams), + ...namespacedMergedParams, + ...getRemainingNewParams(namespacedMergedParams, namespacedNewParams) }); } -function namespaceParams(ns, params) { - if (!ns) return params; - - const namespaced = {}; - Object.keys(params).forEach(key => { - namespaced[`${ns}.${key}`] = params[key]; +/** + * Removes params from the search string and returns the updated list of params + * @param {object} qs config object (used for getting defaults, current query params etc.) + * @param {string} url query string + * @param {object} object with new params to remove + * @return {object} query param object + */ +export function removeParams (config, searchString, paramsToRemove) { + const oldParams = parseQueryString(config, searchString); + const paramsEntries = []; + Object.entries(oldParams) + .forEach(([key, value]) => { + if (Array.isArray(value)) { + value.forEach(val => { + paramsEntries.push([key, val]); + }) + } else { + paramsEntries.push([key, value]); + } + }) + const paramsToRemoveEntries = Object.entries(paramsToRemove); + const remainingEntries = paramsEntries + .filter(([key, value]) => paramsToRemoveEntries + .filter(([newKey, newValue]) => key === newKey && value === newValue).length === 0); + const remainingObject = toObject(remainingEntries); + const defaultEntriesLeftover = Object.entries(config.defaultParams) + .filter(([key]) => !remainingObject[key]); + const finalParamsEntries = remainingEntries; + defaultEntriesLeftover.forEach(value => { + finalParamsEntries.push(value); }); - return namespaced; -} - -function namespaceMatches(namespace, fieldname) { - if (!namespace) { - return !fieldname.includes('.'); - } - return fieldname.startsWith(`${namespace}.`); -} - -function prependNamespaceToArray(namespace, arr) { - if (!namespace) { - return arr; - } - return arr.map(f => `${namespace}.${f}`); + return toObject(finalParamsEntries); } diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index 2d13074318..c6440b24dd 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -1,116 +1,123 @@ import { encodeQueryString, + encodeNonDefaultQueryString, parseQueryString, getQSConfig, - parseNamespacedQueryString, - encodeNamespacedQueryString, - updateNamespacedQueryString, + addParams, + removeParams } from './qs'; describe('qs (qs.js)', () => { - test('encodeQueryString returns the expected queryString', () => { - [ - [null, ''], - [{}, ''], + describe('encodeQueryString', () => { + test('encodeQueryString returns the expected queryString', () => { [ - { order_by: 'name', page: 1, page_size: 5 }, - 'order_by=name&page=1&page_size=5', - ], - [ - { '-order_by': 'name', page: '1', page_size: 5 }, - '-order_by=name&page=1&page_size=5', - ], - ].forEach(([params, expectedQueryString]) => { - const actualQueryString = encodeQueryString(params); + [null, ''], + [{}, ''], + [{ order_by: 'name', page: 1, page_size: 5 }, 'order_by=name&page=1&page_size=5'], + [{ '-order_by': 'name', page: '1', page_size: 5 }, '-order_by=name&page=1&page_size=5'], + ] + .forEach(([params, expectedQueryString]) => { + const actualQueryString = encodeQueryString(params); - expect(actualQueryString).toEqual(expectedQueryString); + expect(actualQueryString).toEqual(expectedQueryString); + }); + }); + + test('encodeQueryString omits null values', () => { + const vals = { + order_by: 'name', + page: null, + }; + expect(encodeQueryString(vals)).toEqual('order_by=name'); }); }); - test('encodeQueryString omits null values', () => { - const vals = { - order_by: 'name', - page: null, + describe('encodeNonDefaultQueryString', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 5, order_by: 'name'}, + integerFields: ['page'], }; - expect(encodeQueryString(vals)).toEqual('order_by=name'); + + test('encodeNonDefaultQueryString returns the expected queryString', () => { + [ + [null, ''], + [{}, ''], + [{ order_by: 'name', page: 1, page_size: 5 }, ''], + [{ order_by: '-name', page: 1, page_size: 5 }, 'order_by=-name'], + [{ order_by: '-name', page: 3, page_size: 10 }, 'order_by=-name&page=3&page_size=10'], + [{ order_by: '-name', page: 3, page_size: 10, foo: 'bar' }, 'foo=bar&order_by=-name&page=3&page_size=10'], + ] + .forEach(([params, expectedQueryString]) => { + const actualQueryString = encodeNonDefaultQueryString(config, params); + + expect(actualQueryString).toEqual(expectedQueryString); + }); + }); + + test('encodeNonDefaultQueryString omits null values', () => { + const vals = { + order_by: 'foo', + page: null, + }; + expect(encodeNonDefaultQueryString(config, vals)).toEqual('order_by=foo'); + }); + }); + + describe('getQSConfig', () => { + test('should get default QS config object', () => { + expect(getQSConfig('organization')).toEqual({ + namespace: 'organization', + defaultParams: { page: 1, page_size: 5, order_by: 'name' }, + integerFields: ['page', 'page_size'], + }); + }); + + test('should throw if no namespace given', () => { + expect(() => getQSConfig()).toThrow(); + }); + + test('should build configured QS config object', () => { + const defaults = { + page: 1, + page_size: 15, + }; + expect(getQSConfig('inventory', defaults)).toEqual({ + namespace: 'inventory', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }); + }); }); describe('parseQueryString', () => { - test('parseQueryString returns the expected queryParams', () => { - [ - [ - 'order_by=name&page=1&page_size=5', - ['page', 'page_size'], - { order_by: 'name', page: 1, page_size: 5 }, - ], - [ - 'order_by=name&page=1&page_size=5', - ['page_size'], - { order_by: 'name', page: '1', page_size: 5 }, - ], - ].forEach(([queryString, integerFields, expectedQueryParams]) => { - const actualQueryParams = parseQueryString(queryString, integerFields); - - expect(actualQueryParams).toEqual(expectedQueryParams); - }); - }); - - test('parseQueryString should strip leading "?"', () => { - expect(parseQueryString('?foo=bar&order_by=win')).toEqual({ - foo: 'bar', - order_by: 'win', - }); - - expect(parseQueryString('foo=bar&order_by=?win')).toEqual({ - foo: 'bar', - order_by: '?win', - }); - }); - - test('should return empty object if no values', () => { - expect(parseQueryString('')).toEqual({}); - }); - }); - - test('should get default QS config object', () => { - expect(getQSConfig('organization')).toEqual({ - namespace: 'organization', - defaultParams: { page: 1, page_size: 5, order_by: 'name' }, - integerFields: ['page', 'page_size'], - }); - }); - - test('should throw if no namespace given', () => { - expect(() => getQSConfig()).toThrow(); - }); - - test('should build configured QS config object', () => { - const defaults = { - page: 1, - page_size: 15, - }; - expect(getQSConfig('inventory', defaults)).toEqual({ - namespace: 'inventory', - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }); - }); - - describe('parseNamespacedQueryString', () => { test('should get query params', () => { const config = { namespace: null, defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?foo=bar&page=3'; - expect(parseNamespacedQueryString(config, query)).toEqual({ - foo: 'bar', + const query = '?baz=bar&page=3'; + expect(parseQueryString(config, query)).toEqual({ + baz: 'bar', page: 3, page_size: 15, }); }); + test('should return namespaced defaults if empty query string passed', () => { + const config = { + namespace: 'foo', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = ''; + expect(parseQueryString(config, query)).toEqual({ + page: 1, + page_size: 15 + }); + }); + test('should get query params with correct integer fields', () => { const config = { namespace: null, @@ -118,12 +125,24 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'foo'], }; const query = '?foo=4&bar=5'; - expect(parseNamespacedQueryString(config, query)).toEqual({ + expect(parseQueryString(config, query)).toEqual({ foo: 4, bar: '5', }); }); + test('should decode parsed params', () => { + const config = { + namespace: null, + defaultParams: {}, + integerFields: ['page'], + }; + const query = '?foo=bar%20baz'; + expect(parseQueryString(config, query)).toEqual({ + foo: 'bar baz', + }); + }); + test('should get namespaced query params', () => { const config = { namespace: 'inventory', @@ -131,7 +150,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?inventory.page=2&inventory.order_by=name&other=15'; - expect(parseNamespacedQueryString(config, query)).toEqual({ + expect(parseQueryString(config, query)).toEqual({ page: 2, order_by: 'name', page_size: 5, @@ -145,81 +164,360 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?inventory.page=2&inventory.order_by=name&foo.other=15'; - expect(parseNamespacedQueryString(config, query)).toEqual({ + expect(parseQueryString(config, query)).toEqual({ page: 2, order_by: 'name', page_size: 5, }); }); - test('should exclude defaults if includeDefaults is false', () => { - const config = { - namespace: null, - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const query = '?foo=bar&page=3'; - expect(parseNamespacedQueryString(config, query, false)).toEqual({ - foo: 'bar', - page: 3, - }); - }); - }); - - describe('encodeNamespacedQueryString', () => { - test('should encode params without namespace', () => { - const config = { - namespace: null, - defaultParams: { page: 1, page_size: 5 }, - integerFields: ['page', 'page_size'], - }; - const params = { - page: 1, - order_by: 'name', - }; - const qs = 'order_by=name&page=1'; - expect(encodeNamespacedQueryString(config, params)).toEqual(qs); - }); - - test('should encode params with namespace', () => { + test('should exclude other namespaced default query params', () => { const config = { namespace: 'inventory', defaultParams: { page: 1, page_size: 5 }, integerFields: ['page', 'page_size'], }; - const params = { + const query = '?foo.page=2&inventory.order_by=name&foo.other=15'; + expect(parseQueryString(config, query)).toEqual({ page: 1, order_by: 'name', + page_size: 5, + }); + }); + + test('should add duplicate non-default params as array', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], }; - const qs = 'inventory.order_by=name&inventory.page=1'; - expect(encodeNamespacedQueryString(config, params)).toEqual(qs); + const query = '?baz=bar&baz=boo&page=3'; + expect(parseQueryString(config, query)).toEqual({ + baz: ['bar', 'boo'], + page: 3, + page_size: 15, + }); + }); + + test('should add duplicate namespaced non-default params as array', () => { + const config = { + namespace: 'bee', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?bee.baz=bar&bee.baz=boo&bee.page=3'; + expect(parseQueryString(config, query)).toEqual({ + baz: ['bar', 'boo'], + page: 3, + page_size: 15, + }); }); }); - describe('updateNamespacedQueryString', () => { - test('should return current values', () => { - const qs = '?foo=bar&inventory.page=1'; - const updated = updateNamespacedQueryString({}, qs, {}); - expect(updated).toEqual('foo=bar&inventory.page=1'); + describe('addParams', () => { + test('should add query params', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?baz=bar&page=3'; + const newParams = { bag: 'boom' } + expect(addParams(config, query, newParams)).toEqual({ + baz: 'bar', + bag: 'boom', + page: 3, + page_size: 15, + }); }); - test('should update new values', () => { - const qs = '?foo=bar&inventory.page=1'; - const updated = updateNamespacedQueryString({}, qs, { foo: 'baz' }); - expect(updated).toEqual('foo=baz&inventory.page=1'); + test('should add query params with duplicates', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?baz=bar&baz=bang&page=3'; + const newParams = { baz: 'boom' } + expect(addParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang', 'boom'], + page: 3, + page_size: 15, + }); }); - test('should add new values', () => { - const qs = '?foo=bar&inventory.page=1'; - const updated = updateNamespacedQueryString({}, qs, { page: 5 }); - expect(updated).toEqual('foo=bar&inventory.page=1&page=5'); + test('should replace query params that are defaults', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?baz=bar&baz=bang&page=3'; + const newParams = { page: 5 } + expect(addParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang'], + page: 5, + page_size: 15, + }); }); - test('should update namespaced values', () => { - const qs = '?foo=bar&inventory.page=1'; - const config = { namespace: 'inventory' }; - const updated = updateNamespacedQueryString(config, qs, { page: 2 }); - expect(updated).toEqual('foo=bar&inventory.page=2'); + test('should add multiple params', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?baz=bar&baz=bang&page=3'; + const newParams = { baz: 'bust', pat: 'pal' } + expect(addParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang', 'bust'], + pat: 'pal', + page: 3, + page_size: 15, + }); + }); + + test('should add namespaced query params', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&item.page=3'; + const newParams = { bag: 'boom' } + expect(addParams(config, query, newParams)).toEqual({ + baz: 'bar', + bag: 'boom', + page: 3, + page_size: 15, + }); + }); + + test('should not include other namespaced query params when adding', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&foo.page=3'; + const newParams = { bag: 'boom' } + expect(addParams(config, query, newParams)).toEqual({ + baz: 'bar', + bag: 'boom', + page: 1, + page_size: 15, + }); + }); + + test('should add namespaced query params with duplicates', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const newParams = { baz: 'boom' } + expect(addParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang', 'boom'], + page: 3, + page_size: 15, + }); + }); + + test('should replace namespaced query params that are defaults', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const newParams = { page: 5 } + expect(addParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang'], + page: 5, + page_size: 15, + }); + }); + + test('should add multiple namespaced params', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const newParams = { baz: 'bust', pat: 'pal' } + expect(addParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang', 'bust'], + pat: 'pal', + page: 3, + page_size: 15, + }); + }); + }); + + describe('removeParams', () => { + test('should remove query params', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?baz=bar&page=3&bag=boom'; + const newParams = { bag: 'boom' } + expect(removeParams(config, query, newParams)).toEqual({ + baz: 'bar', + page: 3, + page_size: 15, + }); + }); + + test('should remove query params with duplicates (array -> string)', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?baz=bar&baz=bang&page=3'; + const newParams = { baz: 'bar' } + expect(removeParams(config, query, newParams)).toEqual({ + baz: 'bang', + page: 3, + page_size: 15, + }); + }); + + test('should remove query params with duplicates (array -> smaller array)', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?baz=bar&baz=bang&baz=bust&page=3'; + const newParams = { baz: 'bar' } + expect(removeParams(config, query, newParams)).toEqual({ + baz: ['bang', 'bust'], + page: 3, + page_size: 15, + }); + }); + + test('should reset query params that have default keys back to default values', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?baz=bar&baz=bang&page=3'; + const newParams = { page: 3 } + expect(removeParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang'], + page: 1, + page_size: 15, + }); + }); + + test('should remove multiple params', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?baz=bar&baz=bang&baz=bust&pat=pal&page=3'; + const newParams = { baz: 'bust', pat: 'pal' } + expect(removeParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang'], + page: 3, + page_size: 15, + }); + }); + + test('should remove namespaced query params', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&item.page=3'; + const newParams = { baz: 'bar' } + expect(removeParams(config, query, newParams)).toEqual({ + page: 3, + page_size: 15, + }); + }); + + test('should not include other namespaced query params when removing', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&foo.page=3'; + const newParams = { baz: 'bar' } + expect(removeParams(config, query, newParams)).toEqual({ + page: 1, + page_size: 15, + }); + }); + + test('should remove namespaced query params with duplicates (array -> string)', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const newParams = { baz: 'bar' } + expect(removeParams(config, query, newParams)).toEqual({ + baz: 'bang', + page: 3, + page_size: 15, + }); + }); + + test('should remove namespaced query params with duplicates (array -> smaller array)', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&item.baz=bang&item.baz=bust&item.page=3'; + const newParams = { baz: 'bar' } + expect(removeParams(config, query, newParams)).toEqual({ + baz: ['bang', 'bust'], + page: 3, + page_size: 15, + }); + }); + + test('should reset namespaced query params that have default keys back to default values', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const newParams = { page: 3 } + expect(removeParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang'], + page: 1, + page_size: 15, + }); + }); + + test('should remove multiple namespaced params', () => { + const config = { + namespace: 'item', + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['page', 'page_size'], + }; + const query = '?item.baz=bar&item.baz=bang&item.baz=bust&item.pat=pal&item.page=3'; + const newParams = { baz: 'bust', pat: 'pal' } + expect(removeParams(config, query, newParams)).toEqual({ + baz: ['bar', 'bang'], + page: 3, + page_size: 15, + }); }); }); }); From 357887417c4bd4640668ecdc206d73533f2233a2 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Fri, 26 Jul 2019 09:52:20 -0400 Subject: [PATCH 2/8] working commit --- .../src/components/FilterTags/FilterTags.jsx | 14 +++++++------- .../src/components/ListHeader/ListHeader.jsx | 2 +- .../PaginatedDataList/PaginatedDataList.jsx | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/awx/ui_next/src/components/FilterTags/FilterTags.jsx b/awx/ui_next/src/components/FilterTags/FilterTags.jsx index cf0ca41575..15be557527 100644 --- a/awx/ui_next/src/components/FilterTags/FilterTags.jsx +++ b/awx/ui_next/src/components/FilterTags/FilterTags.jsx @@ -25,7 +25,7 @@ const FilterLabel = styled.span` `; // remove non-default query params so they don't show up as filter tags -const filterOutDefaultParams = (paramsArr, config) => { +const filterDefaultParams = (paramsArr, config) => { const defaultParamsKeys = Object.keys(config.defaultParams); return paramsArr.filter(key => defaultParamsKeys.indexOf(key) === -1); }; @@ -34,7 +34,7 @@ const FilterTags = ({ i18n, itemCount, qsConfig, location, onRemove, onRemoveAll const queryParams = parseQueryString(qsConfig, location.search); const queryParamsArr = []; const displayAll = true; - const nonDefaultParams = filterOutDefaultParams(Object.keys(queryParams), qsConfig); + const nonDefaultParams = filterDefaultParams(Object.keys(queryParams), qsConfig); nonDefaultParams .forEach(key => { if (Array.isArray(queryParams[key])) { @@ -50,16 +50,16 @@ const FilterTags = ({ i18n, itemCount, qsConfig, location, onRemove, onRemoveAll {`${itemCount} results`} - Active Filters: + {i18n._(t`Active Filters:`)} - {queryParamsArr.map(param => ( + {queryParamsArr.map(({ key, value }) => ( onRemove(param)} + onClick={() => onRemove(key, value)} > - {param.value} + {value} ))}
    diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.jsx index 1009de50f2..aca82ad7f1 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.jsx @@ -50,7 +50,7 @@ class ListHeader extends React.Component { this.pushHistoryState(addParams(qsConfig, search, { [key]: value })); } - handleRemove ({ key, value }) { + handleRemove (key, value) { const { history, qsConfig } = this.props; const { search } = history.location; this.pushHistoryState(removeParams(qsConfig, search, { [key]: value })); diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index 6508e0d6aa..d369598b79 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -51,7 +51,7 @@ class PaginatedDataList extends React.Component { render () { const { - hasContentError, + contentError, hasContentLoading, emptyStateControls, items, @@ -79,8 +79,8 @@ class PaginatedDataList extends React.Component { let Content; if (hasContentLoading && items.length <= 0) { Content = (); - } else if (hasContentError) { - Content = (); + } else if (contentError) { + Content = (); } else if (items.length <= 0) { Content = (); } else { @@ -150,12 +150,12 @@ PaginatedDataList.propTypes = { showPageSizeOptions: PropTypes.bool, renderToolbar: PropTypes.func, hasContentLoading: PropTypes.bool, - hasContentError: PropTypes.bool, + contentError: PropTypes.shape(), }; PaginatedDataList.defaultProps = { hasContentLoading: false, - hasContentError: false, + contentError: null, toolbarColumns: [], itemName: 'item', itemNamePlural: '', From bdfeb2cb9c421e816473399ef38878ae40c14b20 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Fri, 26 Jul 2019 10:03:46 -0400 Subject: [PATCH 3/8] updates based on pr feedback run prettier update hasContentError to contentError in all the places function naming updates --- awx/ui_next/src/api/Base.js | 22 +- awx/ui_next/src/api/Base.test.jsx | 46 ++-- .../src/api/mixins/InstanceGroups.mixin.js | 31 ++- .../src/api/mixins/Notifications.mixin.js | 151 ++++++++----- awx/ui_next/src/api/models/Organizations.js | 6 +- .../src/api/models/Organizations.test.jsx | 26 ++- .../components/AddRole/AddResourceRole.jsx | 89 ++++---- .../components/AddRole/SelectResourceStep.jsx | 5 +- .../AddRole/SelectResourceStep.test.jsx | 39 ++-- awx/ui_next/src/components/Chip/ChipGroup.jsx | 21 +- .../DataListToolbar/DataListToolbar.test.jsx | 38 +++- .../src/components/FilterTags/FilterTags.jsx | 103 +++++---- .../components/FilterTags/FilterTags.test.jsx | 9 +- .../src/components/ListHeader/ListHeader.jsx | 48 ++-- .../components/ListHeader/ListHeader.test.jsx | 11 +- .../PaginatedDataList/PaginatedDataList.jsx | 75 ++++--- .../PaginatedDataList.test.jsx | 6 +- awx/ui_next/src/components/Search/Search.jsx | 66 +++--- .../src/components/Search/Search.test.jsx | 37 ++-- awx/ui_next/src/components/Sort/Sort.test.jsx | 20 +- .../src/screens/Job/JobList/JobList.jsx | 65 +++--- .../OrganizationAccess/OrganizationAccess.jsx | 101 +++++---- .../OrganizationAccess.test.jsx | 2 +- .../OrganizationAccess.test.jsx.snap | 7 +- .../OrganizationList/OrganizationList.jsx | 88 +++++--- .../OrganizationNotifications.jsx | 42 ++-- .../OrganizationNotifications.test.jsx.snap | 1 - .../shared/InstanceGroupsLookup.jsx | 47 ++-- .../Template/TemplateList/TemplateList.jsx | 96 ++++---- awx/ui_next/src/util/qs.js | 208 ++++++++++-------- awx/ui_next/src/util/qs.test.js | 87 ++++---- 31 files changed, 929 insertions(+), 664 deletions(-) diff --git a/awx/ui_next/src/api/Base.js b/awx/ui_next/src/api/Base.js index a5f62d0cd9..ef7c53460f 100644 --- a/awx/ui_next/src/api/Base.js +++ b/awx/ui_next/src/api/Base.js @@ -1,48 +1,46 @@ import axios from 'axios'; -import { - encodeQueryString -} from '@util/qs'; +import { encodeQueryString } from '@util/qs'; const defaultHttp = axios.create({ xsrfCookieName: 'csrftoken', xsrfHeaderName: 'X-CSRFToken', paramsSerializer(params) { return encodeQueryString(params); - } + }, }); class Base { - constructor (http = defaultHttp, baseURL) { + constructor(http = defaultHttp, baseURL) { this.http = http; this.baseUrl = baseURL; } - create (data) { + create(data) { return this.http.post(this.baseUrl, data); } - destroy (id) { + destroy(id) { return this.http.delete(`${this.baseUrl}${id}/`); } - read (params) { + read(params) { return this.http.get(this.baseUrl, { params }); } - readDetail (id) { + readDetail(id) { return this.http.get(`${this.baseUrl}${id}/`); } - readOptions () { + readOptions() { return this.http.options(this.baseUrl); } - replace (id, data) { + replace(id, data) { return this.http.put(`${this.baseUrl}${id}/`, data); } - update (id, data) { + update(id, data) { return this.http.patch(`${this.baseUrl}${id}/`, data); } } diff --git a/awx/ui_next/src/api/Base.test.jsx b/awx/ui_next/src/api/Base.test.jsx index 53246dd80a..40907e79f7 100644 --- a/awx/ui_next/src/api/Base.test.jsx +++ b/awx/ui_next/src/api/Base.test.jsx @@ -3,14 +3,14 @@ import Base from './Base'; describe('Base', () => { const createPromise = () => Promise.resolve(); const mockBaseURL = '/api/v2/organizations/'; - const mockHttp = ({ + const mockHttp = { delete: jest.fn(createPromise), get: jest.fn(createPromise), options: jest.fn(createPromise), patch: jest.fn(createPromise), post: jest.fn(createPromise), - put: jest.fn(createPromise) - }); + put: jest.fn(createPromise), + }; const BaseAPI = new Base(mockHttp, mockBaseURL); @@ -18,7 +18,7 @@ describe('Base', () => { jest.clearAllMocks(); }); - test('create calls http method with expected data', async (done) => { + test('create calls http method with expected data', async done => { const data = { name: 'test ' }; await BaseAPI.create(data); @@ -28,19 +28,21 @@ describe('Base', () => { done(); }); - test('destroy calls http method with expected data', async (done) => { + test('destroy calls http method with expected data', async done => { const resourceId = 1; await BaseAPI.destroy(resourceId); expect(mockHttp.delete).toHaveBeenCalledTimes(1); - expect(mockHttp.delete.mock.calls[0][0]).toEqual(`${mockBaseURL}${resourceId}/`); + expect(mockHttp.delete.mock.calls[0][0]).toEqual( + `${mockBaseURL}${resourceId}/` + ); done(); }); - test('read calls http method with expected data', async (done) => { + test('read calls http method with expected data', async done => { const testParams = { foo: 'bar' }; - const testParamsDuplicates = { foo: ['bar', 'baz']}; + const testParamsDuplicates = { foo: ['bar', 'baz'] }; await BaseAPI.read(testParams); await BaseAPI.read(); @@ -48,25 +50,29 @@ describe('Base', () => { expect(mockHttp.get).toHaveBeenCalledTimes(3); expect(mockHttp.get.mock.calls[0][0]).toEqual(`${mockBaseURL}`); - expect(mockHttp.get.mock.calls[0][1]).toEqual({"params": {"foo": "bar"}}); + expect(mockHttp.get.mock.calls[0][1]).toEqual({ params: { foo: 'bar' } }); expect(mockHttp.get.mock.calls[1][0]).toEqual(`${mockBaseURL}`); - expect(mockHttp.get.mock.calls[1][1]).toEqual({"params": undefined}); + expect(mockHttp.get.mock.calls[1][1]).toEqual({ params: undefined }); expect(mockHttp.get.mock.calls[2][0]).toEqual(`${mockBaseURL}`); - expect(mockHttp.get.mock.calls[2][1]).toEqual({"params": {"foo": ["bar", "baz"]}}); + expect(mockHttp.get.mock.calls[2][1]).toEqual({ + params: { foo: ['bar', 'baz'] }, + }); done(); }); - test('readDetail calls http method with expected data', async (done) => { + test('readDetail calls http method with expected data', async done => { const resourceId = 1; await BaseAPI.readDetail(resourceId); expect(mockHttp.get).toHaveBeenCalledTimes(1); - expect(mockHttp.get.mock.calls[0][0]).toEqual(`${mockBaseURL}${resourceId}/`); + expect(mockHttp.get.mock.calls[0][0]).toEqual( + `${mockBaseURL}${resourceId}/` + ); done(); }); - test('readOptions calls http method with expected data', async (done) => { + test('readOptions calls http method with expected data', async done => { await BaseAPI.readOptions(); expect(mockHttp.options).toHaveBeenCalledTimes(1); @@ -74,27 +80,31 @@ describe('Base', () => { done(); }); - test('replace calls http method with expected data', async (done) => { + test('replace calls http method with expected data', async done => { const resourceId = 1; const data = { name: 'test ' }; await BaseAPI.replace(resourceId, data); expect(mockHttp.put).toHaveBeenCalledTimes(1); - expect(mockHttp.put.mock.calls[0][0]).toEqual(`${mockBaseURL}${resourceId}/`); + expect(mockHttp.put.mock.calls[0][0]).toEqual( + `${mockBaseURL}${resourceId}/` + ); expect(mockHttp.put.mock.calls[0][1]).toEqual(data); done(); }); - test('update calls http method with expected data', async (done) => { + test('update calls http method with expected data', async done => { const resourceId = 1; const data = { name: 'test ' }; await BaseAPI.update(resourceId, data); expect(mockHttp.patch).toHaveBeenCalledTimes(1); - expect(mockHttp.patch.mock.calls[0][0]).toEqual(`${mockBaseURL}${resourceId}/`); + expect(mockHttp.patch.mock.calls[0][0]).toEqual( + `${mockBaseURL}${resourceId}/` + ); expect(mockHttp.patch.mock.calls[0][1]).toEqual(data); done(); diff --git a/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js b/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js index a4fba3cc0e..e3b2be1cb5 100644 --- a/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js +++ b/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js @@ -1,15 +1,24 @@ -const InstanceGroupsMixin = (parent) => class extends parent { - readInstanceGroups (resourceId, params) { - return this.http.get(`${this.baseUrl}${resourceId}/instance_groups/`, params); - } +const InstanceGroupsMixin = parent => + class extends parent { + readInstanceGroups(resourceId, params) { + return this.http.get( + `${this.baseUrl}${resourceId}/instance_groups/`, + params + ); + } - associateInstanceGroup (resourceId, instanceGroupId) { - return this.http.post(`${this.baseUrl}${resourceId}/instance_groups/`, { id: instanceGroupId }); - } + associateInstanceGroup(resourceId, instanceGroupId) { + return this.http.post(`${this.baseUrl}${resourceId}/instance_groups/`, { + id: instanceGroupId, + }); + } - disassociateInstanceGroup (resourceId, instanceGroupId) { - return this.http.post(`${this.baseUrl}${resourceId}/instance_groups/`, { id: instanceGroupId, disassociate: true }); - } -}; + disassociateInstanceGroup(resourceId, instanceGroupId) { + return this.http.post(`${this.baseUrl}${resourceId}/instance_groups/`, { + id: instanceGroupId, + disassociate: true, + }); + } + }; export default InstanceGroupsMixin; diff --git a/awx/ui_next/src/api/mixins/Notifications.mixin.js b/awx/ui_next/src/api/mixins/Notifications.mixin.js index f7c934a0c8..e40b7983a3 100644 --- a/awx/ui_next/src/api/mixins/Notifications.mixin.js +++ b/awx/ui_next/src/api/mixins/Notifications.mixin.js @@ -1,65 +1,106 @@ -const NotificationsMixin = (parent) => class extends parent { - readOptionsNotificationTemplates(id) { - return this.http.options(`${this.baseUrl}${id}/notification_templates/`); - } - - readNotificationTemplates (id, params) { - return this.http.get(`${this.baseUrl}${id}/notification_templates/`, params); - } - - readNotificationTemplatesSuccess (id, params) { - return this.http.get(`${this.baseUrl}${id}/notification_templates_success/`, params); - } - - readNotificationTemplatesError (id, params) { - return this.http.get(`${this.baseUrl}${id}/notification_templates_error/`, params); - } - - associateNotificationTemplatesSuccess (resourceId, notificationId) { - return this.http.post(`${this.baseUrl}${resourceId}/notification_templates_success/`, { id: notificationId }); - } - - disassociateNotificationTemplatesSuccess (resourceId, notificationId) { - return this.http.post(`${this.baseUrl}${resourceId}/notification_templates_success/`, { id: notificationId, disassociate: true }); - } - - associateNotificationTemplatesError (resourceId, notificationId) { - return this.http.post(`${this.baseUrl}${resourceId}/notification_templates_error/`, { id: notificationId }); - } - - disassociateNotificationTemplatesError (resourceId, notificationId) { - return this.http.post(`${this.baseUrl}${resourceId}/notification_templates_error/`, { id: notificationId, disassociate: true }); - } - - /** - * This is a helper method meant to simplify setting the "on" or "off" status of - * a related notification. - * - * @param[resourceId] - id of the base resource - * @param[notificationId] - id of the notification - * @param[notificationType] - the type of notification, options are "success" and "error" - * @param[associationState] - Boolean for associating or disassociating, options are true or false - */ - // eslint-disable-next-line max-len - updateNotificationTemplateAssociation (resourceId, notificationId, notificationType, associationState) { - if (notificationType === 'success' && associationState === true) { - return this.associateNotificationTemplatesSuccess(resourceId, notificationId); +const NotificationsMixin = parent => + class extends parent { + readOptionsNotificationTemplates(id) { + return this.http.options(`${this.baseUrl}${id}/notification_templates/`); } - if (notificationType === 'success' && associationState === false) { - return this.disassociateNotificationTemplatesSuccess(resourceId, notificationId); + readNotificationTemplates(id, params) { + return this.http.get( + `${this.baseUrl}${id}/notification_templates/`, + params + ); } - if (notificationType === 'error' && associationState === true) { - return this.associateNotificationTemplatesError(resourceId, notificationId); + readNotificationTemplatesSuccess(id, params) { + return this.http.get( + `${this.baseUrl}${id}/notification_templates_success/`, + params + ); } - if (notificationType === 'error' && associationState === false) { - return this.disassociateNotificationTemplatesError(resourceId, notificationId); + readNotificationTemplatesError(id, params) { + return this.http.get( + `${this.baseUrl}${id}/notification_templates_error/`, + params + ); } - throw new Error(`Unsupported notificationType, associationState combination: ${notificationType}, ${associationState}`); - } -}; + associateNotificationTemplatesSuccess(resourceId, notificationId) { + return this.http.post( + `${this.baseUrl}${resourceId}/notification_templates_success/`, + { id: notificationId } + ); + } + + disassociateNotificationTemplatesSuccess(resourceId, notificationId) { + return this.http.post( + `${this.baseUrl}${resourceId}/notification_templates_success/`, + { id: notificationId, disassociate: true } + ); + } + + associateNotificationTemplatesError(resourceId, notificationId) { + return this.http.post( + `${this.baseUrl}${resourceId}/notification_templates_error/`, + { id: notificationId } + ); + } + + disassociateNotificationTemplatesError(resourceId, notificationId) { + return this.http.post( + `${this.baseUrl}${resourceId}/notification_templates_error/`, + { id: notificationId, disassociate: true } + ); + } + + /** + * This is a helper method meant to simplify setting the "on" or "off" status of + * a related notification. + * + * @param[resourceId] - id of the base resource + * @param[notificationId] - id of the notification + * @param[notificationType] - the type of notification, options are "success" and "error" + * @param[associationState] - Boolean for associating or disassociating, options are true or false + */ + // eslint-disable-next-line max-len + updateNotificationTemplateAssociation( + resourceId, + notificationId, + notificationType, + associationState + ) { + if (notificationType === 'success' && associationState === true) { + return this.associateNotificationTemplatesSuccess( + resourceId, + notificationId + ); + } + + if (notificationType === 'success' && associationState === false) { + return this.disassociateNotificationTemplatesSuccess( + resourceId, + notificationId + ); + } + + if (notificationType === 'error' && associationState === true) { + return this.associateNotificationTemplatesError( + resourceId, + notificationId + ); + } + + if (notificationType === 'error' && associationState === false) { + return this.disassociateNotificationTemplatesError( + resourceId, + notificationId + ); + } + + throw new Error( + `Unsupported notificationType, associationState combination: ${notificationType}, ${associationState}` + ); + } + }; export default NotificationsMixin; diff --git a/awx/ui_next/src/api/models/Organizations.js b/awx/ui_next/src/api/models/Organizations.js index 343c51f5b2..3cbe64c284 100644 --- a/awx/ui_next/src/api/models/Organizations.js +++ b/awx/ui_next/src/api/models/Organizations.js @@ -3,16 +3,16 @@ import NotificationsMixin from '../mixins/Notifications.mixin'; import InstanceGroupsMixin from '../mixins/InstanceGroups.mixin'; class Organizations extends InstanceGroupsMixin(NotificationsMixin(Base)) { - constructor (http) { + constructor(http) { super(http); this.baseUrl = '/api/v2/organizations/'; } - readAccessList (id, params) { + readAccessList(id, params) { return this.http.get(`${this.baseUrl}${id}/access_list/`, { params }); } - readTeams (id, params) { + readTeams(id, params) { return this.http.get(`${this.baseUrl}${id}/teams/`, { params }); } } diff --git a/awx/ui_next/src/api/models/Organizations.test.jsx b/awx/ui_next/src/api/models/Organizations.test.jsx index cb4dfbde70..cd22a09bb8 100644 --- a/awx/ui_next/src/api/models/Organizations.test.jsx +++ b/awx/ui_next/src/api/models/Organizations.test.jsx @@ -4,7 +4,7 @@ import { describeNotificationMixin } from '../../../testUtils/apiReusable'; describe('OrganizationsAPI', () => { const orgId = 1; const createPromise = () => Promise.resolve(); - const mockHttp = ({ get: jest.fn(createPromise) }); + const mockHttp = { get: jest.fn(createPromise) }; const OrganizationsAPI = new Organizations(mockHttp); @@ -12,9 +12,9 @@ describe('OrganizationsAPI', () => { jest.clearAllMocks(); }); - test('read access list calls get with expected params', async (done) => { + test('read access list calls get with expected params', async done => { const testParams = { foo: 'bar' }; - const testParamsDuplicates = { foo: ['bar', 'baz']}; + const testParamsDuplicates = { foo: ['bar', 'baz'] }; const mockBaseURL = `/api/v2/organizations/${orgId}/access_list/`; @@ -24,17 +24,19 @@ describe('OrganizationsAPI', () => { expect(mockHttp.get).toHaveBeenCalledTimes(3); expect(mockHttp.get.mock.calls[0][0]).toEqual(`${mockBaseURL}`); - expect(mockHttp.get.mock.calls[0][1]).toEqual({"params": undefined}); + expect(mockHttp.get.mock.calls[0][1]).toEqual({ params: undefined }); expect(mockHttp.get.mock.calls[1][0]).toEqual(`${mockBaseURL}`); - expect(mockHttp.get.mock.calls[1][1]).toEqual({"params": {"foo": "bar"}}); + expect(mockHttp.get.mock.calls[1][1]).toEqual({ params: { foo: 'bar' } }); expect(mockHttp.get.mock.calls[2][0]).toEqual(`${mockBaseURL}`); - expect(mockHttp.get.mock.calls[2][1]).toEqual({"params": {"foo": ["bar", "baz"]}}); + expect(mockHttp.get.mock.calls[2][1]).toEqual({ + params: { foo: ['bar', 'baz'] }, + }); done(); }); - test('read teams calls get with expected params', async (done) => { + test('read teams calls get with expected params', async done => { const testParams = { foo: 'bar' }; - const testParamsDuplicates = { foo: ['bar', 'baz']}; + const testParamsDuplicates = { foo: ['bar', 'baz'] }; const mockBaseURL = `/api/v2/organizations/${orgId}/teams/`; @@ -44,11 +46,13 @@ describe('OrganizationsAPI', () => { expect(mockHttp.get).toHaveBeenCalledTimes(3); expect(mockHttp.get.mock.calls[0][0]).toEqual(`${mockBaseURL}`); - expect(mockHttp.get.mock.calls[0][1]).toEqual({"params": undefined}); + expect(mockHttp.get.mock.calls[0][1]).toEqual({ params: undefined }); expect(mockHttp.get.mock.calls[1][0]).toEqual(`${mockBaseURL}`); - expect(mockHttp.get.mock.calls[1][1]).toEqual({"params": {"foo": "bar"}}); + expect(mockHttp.get.mock.calls[1][1]).toEqual({ params: { foo: 'bar' } }); expect(mockHttp.get.mock.calls[2][0]).toEqual(`${mockBaseURL}`); - expect(mockHttp.get.mock.calls[2][1]).toEqual({"params": {"foo": ["bar", "baz"]}}); + expect(mockHttp.get.mock.calls[2][1]).toEqual({ + params: { foo: ['bar', 'baz'] }, + }); done(); }); }); diff --git a/awx/ui_next/src/components/AddRole/AddResourceRole.jsx b/awx/ui_next/src/components/AddRole/AddResourceRole.jsx index 1cbf55a3bd..2770afe53e 100644 --- a/awx/ui_next/src/components/AddRole/AddResourceRole.jsx +++ b/awx/ui_next/src/components/AddRole/AddResourceRole.jsx @@ -8,14 +8,13 @@ import SelectRoleStep from './SelectRoleStep'; import SelectableCard from './SelectableCard'; import { TeamsAPI, UsersAPI } from '../../api'; -const readUsers = async (queryParams) => UsersAPI.read( - Object.assign(queryParams, { is_superuser: false }) -); +const readUsers = async queryParams => + UsersAPI.read(Object.assign(queryParams, { is_superuser: false })); -const readTeams = async (queryParams) => TeamsAPI.read(queryParams); +const readTeams = async queryParams => TeamsAPI.read(queryParams); class AddResourceRole extends React.Component { - constructor (props) { + constructor(props) { super(props); this.state = { @@ -25,67 +24,69 @@ class AddResourceRole extends React.Component { currentStepId: 1, }; - this.handleResourceCheckboxClick = this.handleResourceCheckboxClick.bind(this); + this.handleResourceCheckboxClick = this.handleResourceCheckboxClick.bind( + this + ); this.handleResourceSelect = this.handleResourceSelect.bind(this); this.handleRoleCheckboxClick = this.handleRoleCheckboxClick.bind(this); this.handleWizardNext = this.handleWizardNext.bind(this); this.handleWizardSave = this.handleWizardSave.bind(this); } - handleResourceCheckboxClick (user) { + handleResourceCheckboxClick(user) { const { selectedResourceRows } = this.state; - const selectedIndex = selectedResourceRows - .findIndex(selectedRow => selectedRow.id === user.id); + const selectedIndex = selectedResourceRows.findIndex( + selectedRow => selectedRow.id === user.id + ); if (selectedIndex > -1) { selectedResourceRows.splice(selectedIndex, 1); this.setState({ selectedResourceRows }); } else { this.setState(prevState => ({ - selectedResourceRows: [...prevState.selectedResourceRows, user] + selectedResourceRows: [...prevState.selectedResourceRows, user], })); } } - handleRoleCheckboxClick (role) { + handleRoleCheckboxClick(role) { const { selectedRoleRows } = this.state; - const selectedIndex = selectedRoleRows - .findIndex(selectedRow => selectedRow.id === role.id); + const selectedIndex = selectedRoleRows.findIndex( + selectedRow => selectedRow.id === role.id + ); if (selectedIndex > -1) { selectedRoleRows.splice(selectedIndex, 1); this.setState({ selectedRoleRows }); } else { this.setState(prevState => ({ - selectedRoleRows: [...prevState.selectedRoleRows, role] + selectedRoleRows: [...prevState.selectedRoleRows, role], })); } } - handleResourceSelect (resourceType) { + handleResourceSelect(resourceType) { this.setState({ selectedResource: resourceType, selectedResourceRows: [], - selectedRoleRows: [] + selectedRoleRows: [], }); } - handleWizardNext (step) { + handleWizardNext(step) { this.setState({ currentStepId: step.id, }); } - async handleWizardSave () { - const { - onSave - } = this.props; + async handleWizardSave() { + const { onSave } = this.props; const { selectedResourceRows, selectedRoleRows, - selectedResource + selectedResource, } = this.state; try { @@ -95,11 +96,17 @@ class AddResourceRole extends React.Component { for (let j = 0; j < selectedRoleRows.length; j++) { if (selectedResource === 'users') { roleRequests.push( - UsersAPI.associateRole(selectedResourceRows[i].id, selectedRoleRows[j].id) + UsersAPI.associateRole( + selectedResourceRows[i].id, + selectedRoleRows[j].id + ) ); } else if (selectedResource === 'teams') { roleRequests.push( - TeamsAPI.associateRole(selectedResourceRows[i].id, selectedRoleRows[j].id) + TeamsAPI.associateRole( + selectedResourceRows[i].id, + selectedRoleRows[j].id + ) ); } } @@ -112,25 +119,31 @@ class AddResourceRole extends React.Component { } } - render () { + render() { const { selectedResource, selectedResourceRows, selectedRoleRows, currentStepId, } = this.state; - const { - onClose, - roles, - i18n - } = this.props; + const { onClose, roles, i18n } = this.props; const userColumns = [ - { name: i18n._(t`Username`), key: 'username', isSortable: true, isSearchable: true } + { + name: i18n._(t`Username`), + key: 'username', + isSortable: true, + isSearchable: true, + }, ]; const teamColumns = [ - { name: i18n._(t`Name`), key: 'name', isSortable: true, isSearchable: true } + { + name: i18n._(t`Name`), + key: 'name', + isSortable: true, + isSearchable: true, + }, ]; let wizardTitle = ''; @@ -164,7 +177,7 @@ class AddResourceRole extends React.Component { />
    ), - enableNext: selectedResource !== null + enableNext: selectedResource !== null, }, { id: 2, @@ -195,7 +208,7 @@ class AddResourceRole extends React.Component { )} ), - enableNext: selectedResourceRows.length > 0 + enableNext: selectedResourceRows.length > 0, }, { id: 3, @@ -211,8 +224,8 @@ class AddResourceRole extends React.Component { /> ), nextButtonText: i18n._(t`Save`), - enableNext: selectedRoleRows.length > 0 - } + enableNext: selectedRoleRows.length > 0, + }, ]; const currentStep = steps.find(step => step.id === currentStepId); @@ -236,11 +249,11 @@ class AddResourceRole extends React.Component { AddResourceRole.propTypes = { onClose: PropTypes.func.isRequired, onSave: PropTypes.func.isRequired, - roles: PropTypes.shape() + roles: PropTypes.shape(), }; AddResourceRole.defaultProps = { - roles: {} + roles: {}, }; export { AddResourceRole as _AddResourceRole }; diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx index fb7b00e4f1..2599c7fd64 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx @@ -40,10 +40,7 @@ class SelectResourceStep extends React.Component { async readResourceList() { const { onSearch, location } = this.props; - const queryParams = parseQueryString( - this.qsConfig, - location.search - ); + const queryParams = parseQueryString(this.qsConfig, location.search); this.setState({ isLoading: true, diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx index 6dced8c9f9..3d32abef06 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx @@ -7,7 +7,7 @@ import SelectResourceStep from './SelectResourceStep'; describe('', () => { const columns = [ - { name: 'Username', key: 'username', isSortable: true, isSearchable: true } + { name: 'Username', key: 'username', isSortable: true, isSearchable: true }, ]; afterEach(() => { jest.restoreAllMocks(); @@ -30,9 +30,9 @@ describe('', () => { count: 2, results: [ { id: 1, username: 'foo', url: 'item/1' }, - { id: 2, username: 'bar', url: 'item/2' } - ] - } + { id: 2, username: 'bar', url: 'item/2' }, + ], + }, }); mountWithContexts( ', () => { expect(handleSearch).toHaveBeenCalledWith({ order_by: 'username', page: 1, - page_size: 5 + page_size: 5, }); }); test('readResourceList properly adds rows to state', async () => { - const selectedResourceRows = [ - { id: 1, username: 'foo', url: 'item/1' } - ]; + const selectedResourceRows = [{ id: 1, username: 'foo', url: 'item/1' }]; const handleSearch = jest.fn().mockResolvedValue({ data: { count: 2, results: [ { id: 1, username: 'foo', url: 'item/1' }, - { id: 2, username: 'bar', url: 'item/2' } - ] - } + { id: 2, username: 'bar', url: 'item/2' }, + ], + }, }); const history = createMemoryHistory({ - initialEntries: ['/organizations/1/access?resource.page=1&resource.order_by=-username'], + initialEntries: [ + '/organizations/1/access?resource.page=1&resource.order_by=-username', + ], }); const wrapper = await mountWithContexts( ', () => { onSearch={handleSearch} selectedResourceRows={selectedResourceRows} sortedColumnKey="username" - />, { context: { router: { history, route: { location: history.location } } } } + />, + { + context: { router: { history, route: { location: history.location } } }, + } ).find('SelectResourceStep'); await wrapper.instance().readResourceList(); expect(handleSearch).toHaveBeenCalledWith({ @@ -84,7 +87,7 @@ describe('', () => { }); expect(wrapper.state('resources')).toEqual([ { id: 1, username: 'foo', url: 'item/1' }, - { id: 2, username: 'bar', url: 'item/2' } + { id: 2, username: 'bar', url: 'item/2' }, ]); }); @@ -94,8 +97,8 @@ describe('', () => { count: 2, results: [ { id: 1, username: 'foo', url: 'item/1' }, - { id: 2, username: 'bar', url: 'item/2' } - ] + { id: 2, username: 'bar', url: 'item/2' }, + ], }; const wrapper = mountWithContexts( ', () => { wrapper.update(); const checkboxListItemWrapper = wrapper.find('CheckboxListItem'); expect(checkboxListItemWrapper.length).toBe(2); - checkboxListItemWrapper.first().find('input[type="checkbox"]') + checkboxListItemWrapper + .first() + .find('input[type="checkbox"]') .simulate('change', { target: { checked: true } }); expect(handleRowClick).toHaveBeenCalledWith(data.results[0]); }); diff --git a/awx/ui_next/src/components/Chip/ChipGroup.jsx b/awx/ui_next/src/components/Chip/ChipGroup.jsx index 24ffdbc220..8d6d6f0593 100644 --- a/awx/ui_next/src/components/Chip/ChipGroup.jsx +++ b/awx/ui_next/src/components/Chip/ChipGroup.jsx @@ -3,14 +3,21 @@ import { number, bool } from 'prop-types'; import styled from 'styled-components'; import Chip from './Chip'; -const ChipGroup = ({ children, className, showOverflowAfter, displayAll, ...props }) => { +const ChipGroup = ({ + children, + className, + showOverflowAfter, + displayAll, + ...props +}) => { const [isExpanded, setIsExpanded] = useState(!showOverflowAfter); const toggleIsOpen = () => setIsExpanded(!isExpanded); - const mappedChildren = React.Children.map(children, c => ( + const mappedChildren = React.Children.map(children, c => React.cloneElement(c, { component: 'li' }) - )); - const showOverflowToggle = showOverflowAfter && children.length > showOverflowAfter; + ); + const showOverflowToggle = + showOverflowAfter && children.length > showOverflowAfter; const numToShow = isExpanded ? children.length : Math.min(showOverflowAfter, children.length); @@ -30,17 +37,17 @@ const ChipGroup = ({ children, className, showOverflowAfter, displayAll, ...prop }; ChipGroup.propTypes = { showOverflowAfter: number, - displayAll: bool + displayAll: bool, }; ChipGroup.defaultProps = { showOverflowAfter: null, - displayAll: false + displayAll: false, }; export default styled(ChipGroup)` --pf-c-chip-group--c-chip--MarginRight: 10px; --pf-c-chip-group--c-chip--MarginBottom: 10px; - + > .pf-c-chip.pf-m-overflow button { padding: 3px 8px; } diff --git a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.test.jsx b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.test.jsx index e8262b3333..8beb597edb 100644 --- a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.test.jsx +++ b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.test.jsx @@ -13,7 +13,9 @@ describe('', () => { }); test('it triggers the expected callbacks', () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; + const columns = [ + { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, + ]; const search = 'button[aria-label="Search submit button"]'; const searchTextInput = 'input[aria-label="Search text input"]'; @@ -59,14 +61,16 @@ describe('', () => { test('dropdown items sortable/searchable columns work', () => { const sortDropdownToggleSelector = 'button[id="awx-sort"]'; const searchDropdownToggleSelector = 'button[id="awx-search"]'; - const sortDropdownMenuItems = 'DropdownMenu > ul[aria-labelledby="awx-sort"]'; - const searchDropdownMenuItems = 'DropdownMenu > ul[aria-labelledby="awx-search"]'; + const sortDropdownMenuItems = + 'DropdownMenu > ul[aria-labelledby="awx-sort"]'; + const searchDropdownMenuItems = + 'DropdownMenu > ul[aria-labelledby="awx-search"]'; const multipleColumns = [ { name: 'Foo', key: 'foo', isSortable: true, isSearchable: true }, { name: 'Bar', key: 'bar', isSortable: true, isSearchable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' } + { name: 'Baz', key: 'baz' }, ]; const onSort = jest.fn(); @@ -103,12 +107,16 @@ describe('', () => { ); toolbar.update(); - const sortDropdownToggleDescending = toolbar.find(sortDropdownToggleSelector); + const sortDropdownToggleDescending = toolbar.find( + sortDropdownToggleSelector + ); expect(sortDropdownToggleDescending.length).toBe(1); sortDropdownToggleDescending.simulate('click'); toolbar.update(); - const sortDropdownItemsDescending = toolbar.find(sortDropdownMenuItems).children(); + const sortDropdownItemsDescending = toolbar + .find(sortDropdownMenuItems) + .children(); expect(sortDropdownItemsDescending.length).toBe(2); sortDropdownToggleDescending.simulate('click'); // toggle close the sort dropdown @@ -134,8 +142,12 @@ describe('', () => { const downAlphaIconSelector = 'SortAlphaDownIcon'; const upAlphaIconSelector = 'SortAlphaUpIcon'; - const numericColumns = [{ name: 'ID', key: 'id', isSortable: true, isNumeric: true }]; - const alphaColumns = [{ name: 'Name', key: 'name', isSortable: true, isNumeric: false }]; + const numericColumns = [ + { name: 'ID', key: 'id', isSortable: true, isNumeric: true }, + ]; + const alphaColumns = [ + { name: 'Name', key: 'name', isSortable: true, isNumeric: false }, + ]; toolbar = mountWithContexts( ', () => { }); test('should render additionalControls', () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; + const columns = [ + { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, + ]; const onSearch = jest.fn(); const onSort = jest.fn(); const onSelectAll = jest.fn(); @@ -194,7 +208,11 @@ describe('', () => { onSearch={onSearch} onSort={onSort} onSelectAll={onSelectAll} - additionalControls={[]} + additionalControls={[ + , + ]} /> ); diff --git a/awx/ui_next/src/components/FilterTags/FilterTags.jsx b/awx/ui_next/src/components/FilterTags/FilterTags.jsx index 15be557527..ac66d97969 100644 --- a/awx/ui_next/src/components/FilterTags/FilterTags.jsx +++ b/awx/ui_next/src/components/FilterTags/FilterTags.jsx @@ -9,19 +9,19 @@ import { ChipGroup, Chip } from '@components/Chip'; import VerticalSeparator from '@components/VerticalSeparator'; const FilterTagsRow = styled.div` - display: flex; - padding: 15px 20px; - border-top: 1px solid #d2d2d2; - font-size: 14px; - align-items: center; + display: flex; + padding: 15px 20px; + border-top: 1px solid #d2d2d2; + font-size: 14px; + align-items: center; `; const ResultCount = styled.span` - font-weight: bold; + font-weight: bold; `; const FilterLabel = styled.span` - padding-right: 20px; + padding-right: 20px; `; // remove non-default query params so they don't show up as filter tags @@ -30,50 +30,59 @@ const filterDefaultParams = (paramsArr, config) => { return paramsArr.filter(key => defaultParamsKeys.indexOf(key) === -1); }; -const FilterTags = ({ i18n, itemCount, qsConfig, location, onRemove, onRemoveAll }) => { +const FilterTags = ({ + i18n, + itemCount, + qsConfig, + location, + onRemove, + onRemoveAll, +}) => { const queryParams = parseQueryString(qsConfig, location.search); const queryParamsArr = []; const displayAll = true; - const nonDefaultParams = filterDefaultParams(Object.keys(queryParams), qsConfig); - nonDefaultParams - .forEach(key => { - if (Array.isArray(queryParams[key])) { - queryParams[key].forEach(val => queryParamsArr.push({ key, value: val })); - } else { - queryParamsArr.push({ key, value: queryParams[key] }); - } - }); + const nonDefaultParams = filterDefaultParams( + Object.keys(queryParams), + qsConfig + ); + nonDefaultParams.forEach(key => { + if (Array.isArray(queryParams[key])) { + queryParams[key].forEach(val => queryParamsArr.push({ key, value: val })); + } else { + queryParamsArr.push({ key, value: queryParams[key] }); + } + }); - return (queryParamsArr.length > 0) && ( - - - {`${itemCount} results`} - - - {i18n._(t`Active Filters:`)} - - {queryParamsArr.map(({ key, value }) => ( - onRemove(key, value)} - > - {value} - - ))} -
    - -
    -
    -
    + return ( + queryParamsArr.length > 0 && ( + + {`${itemCount} results`} + + {i18n._(t`Active Filters:`)} + + {queryParamsArr.map(({ key, value }) => ( + onRemove(key, value)} + > + {value} + + ))} +
    + +
    +
    +
    + ) ); }; diff --git a/awx/ui_next/src/components/FilterTags/FilterTags.test.jsx b/awx/ui_next/src/components/FilterTags/FilterTags.test.jsx index 7ad82361f4..df681d7c68 100644 --- a/awx/ui_next/src/components/FilterTags/FilterTags.test.jsx +++ b/awx/ui_next/src/components/FilterTags/FilterTags.test.jsx @@ -26,14 +26,19 @@ describe('', () => { test('renders non-default param tags based on location history', () => { const history = createMemoryHistory({ - initialEntries: ['/foo?item.page=1&item.page_size=2&item.foo=bar&item.baz=bust'], + initialEntries: [ + '/foo?item.page=1&item.page_size=2&item.foo=bar&item.baz=bust', + ], }); const wrapper = mountWithContexts( , { context: { router: { history, route: { location: history.location } } } } + />, + { + context: { router: { history, route: { location: history.location } } }, + } ); const chips = wrapper.find('.pf-c-chip.searchTagChip'); expect(chips.length).toBe(2); diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.jsx index aca82ad7f1..19b2367cf8 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.jsx @@ -10,7 +10,7 @@ import { encodeNonDefaultQueryString, parseQueryString, addParams, - removeParams + removeParams, } from '@util/qs'; import { QSConfig } from '@types'; @@ -21,12 +21,12 @@ const EmptyStateControlsWrapper = styled.div` margin-bottom: 20px; justify-content: flex-end; - & > :not(:first-child) { + & > :not(:first-child) { margin-left: 20px; } `; class ListHeader extends React.Component { - constructor (props) { + constructor(props) { super(props); this.handleSearch = this.handleSearch.bind(this); @@ -35,7 +35,7 @@ class ListHeader extends React.Component { this.handleRemoveAll = this.handleRemoveAll.bind(this); } - getSortOrder () { + getSortOrder() { const { qsConfig, location } = this.props; const queryParams = parseQueryString(qsConfig, location.search); if (queryParams.order_by && queryParams.order_by.startsWith('-')) { @@ -44,45 +44,47 @@ class ListHeader extends React.Component { return [queryParams.order_by, 'ascending']; } - handleSearch (key, value) { + handleSearch(key, value) { const { history, qsConfig } = this.props; const { search } = history.location; this.pushHistoryState(addParams(qsConfig, search, { [key]: value })); } - handleRemove (key, value) { + handleRemove(key, value) { const { history, qsConfig } = this.props; const { search } = history.location; this.pushHistoryState(removeParams(qsConfig, search, { [key]: value })); } - handleRemoveAll () { + handleRemoveAll() { this.pushHistoryState(null); } - handleSort (key, order) { + handleSort(key, order) { const { history, qsConfig } = this.props; const { search } = history.location; - this.pushHistoryState(addParams(qsConfig, search, { - order_by: order === 'ascending' ? key : `-${key}`, - page: null, - })); + this.pushHistoryState( + addParams(qsConfig, search, { + order_by: order === 'ascending' ? key : `-${key}`, + page: null, + }) + ); } - pushHistoryState (params) { + pushHistoryState(params) { const { history, qsConfig } = this.props; const { pathname } = history.location; const encodedParams = encodeNonDefaultQueryString(qsConfig, params); history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); } - render () { + render() { const { emptyStateControls, itemCount, columns, renderToolbar, - qsConfig + qsConfig, } = this.props; const [orderBy, sortOrder] = this.getSortOrder(); return ( @@ -124,17 +126,19 @@ class ListHeader extends React.Component { ListHeader.propTypes = { itemCount: PropTypes.number.isRequired, qsConfig: QSConfig.isRequired, - columns: arrayOf(shape({ - name: string.isRequired, - key: string.isRequired, - isSortable: bool, - isSearchable: bool - })).isRequired, + columns: arrayOf( + shape({ + name: string.isRequired, + key: string.isRequired, + isSortable: bool, + isSearchable: bool, + }) + ).isRequired, renderToolbar: PropTypes.func, }; ListHeader.defaultProps = { - renderToolbar: (props) => (), + renderToolbar: props => , }; export default withRouter(ListHeader); diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx index 264c4e66b6..9435f2e8b3 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx @@ -17,7 +17,9 @@ describe('ListHeader', () => { ); @@ -33,8 +35,11 @@ describe('ListHeader', () => { , { context: { router: { history } } } + columns={[ + { name: 'name', key: 'name', isSearchable: true, isSortable: true }, + ]} + />, + { context: { router: { history } } } ); const toolbar = wrapper.find('DataListToolbar'); diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index d369598b79..5051e545d8 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -15,7 +15,7 @@ import DataListToolbar from '@components/DataListToolbar'; import { encodeNonDefaultQueryString, parseQueryString, - addParams + addParams, } from '@util/qs'; import { pluralize, ucFirst } from '@util/strings'; @@ -24,32 +24,32 @@ import { QSConfig } from '@types'; import PaginatedDataListItem from './PaginatedDataListItem'; class PaginatedDataList extends React.Component { - constructor (props) { + constructor(props) { super(props); this.handleSetPage = this.handleSetPage.bind(this); this.handleSetPageSize = this.handleSetPageSize.bind(this); } - handleSetPage (event, pageNumber) { + handleSetPage(event, pageNumber) { const { history, qsConfig } = this.props; const { search } = history.location; this.pushHistoryState(addParams(qsConfig, search, { page: pageNumber })); } - handleSetPageSize (event, pageSize) { + handleSetPageSize(event, pageSize) { const { history, qsConfig } = this.props; const { search } = history.location; this.pushHistoryState(addParams(qsConfig, search, { page_size: pageSize })); } - pushHistoryState (params) { + pushHistoryState(params) { const { history, qsConfig } = this.props; const { pathname } = history.location; const encodedParams = encodeNonDefaultQueryString(qsConfig, params); history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); } - render () { + render() { const { contentError, hasContentLoading, @@ -66,25 +66,42 @@ class PaginatedDataList extends React.Component { i18n, renderToolbar, } = this.props; - const columns = toolbarColumns.length ? toolbarColumns : [{ name: i18n._(t`Name`), key: 'name', isSortable: true, isSearchable: true }]; + const columns = toolbarColumns.length + ? toolbarColumns + : [ + { + name: i18n._(t`Name`), + key: 'name', + isSortable: true, + isSearchable: true, + }, + ]; const queryParams = parseQueryString(qsConfig, location.search); const itemDisplayName = ucFirst(pluralize(itemName)); - const itemDisplayNamePlural = ucFirst(itemNamePlural || pluralize(itemName)); + const itemDisplayNamePlural = ucFirst( + itemNamePlural || pluralize(itemName) + ); const dataListLabel = i18n._(t`${itemDisplayName} List`); - const emptyContentMessage = i18n._(t`Please add ${itemDisplayNamePlural} to populate this list `); + const emptyContentMessage = i18n._( + t`Please add ${itemDisplayNamePlural} to populate this list ` + ); const emptyContentTitle = i18n._(t`No ${itemDisplayNamePlural} Found `); let Content; if (hasContentLoading && items.length <= 0) { - Content = (); + Content = ; } else if (contentError) { - Content = (); + Content = ; } else if (items.length <= 0) { - Content = (); + Content = ( + + ); } else { - Content = ({items.map(renderItem)}); + Content = ( + {items.map(renderItem)} + ); } if (items.length <= 0) { @@ -115,12 +132,16 @@ class PaginatedDataList extends React.Component { itemCount={itemCount} page={queryParams.page || 1} perPage={queryParams.page_size} - perPageOptions={showPageSizeOptions ? [ - { title: '5', value: 5 }, - { title: '10', value: 10 }, - { title: '20', value: 20 }, - { title: '50', value: 50 } - ] : []} + perPageOptions={ + showPageSizeOptions + ? [ + { title: '5', value: 5 }, + { title: '10', value: 10 }, + { title: '20', value: 20 }, + { title: '50', value: 50 }, + ] + : [] + } onSetPage={this.handleSetPage} onPerPageSelect={this.handleSetPageSize} /> @@ -142,11 +163,13 @@ PaginatedDataList.propTypes = { itemNamePlural: PropTypes.string, qsConfig: QSConfig.isRequired, renderItem: PropTypes.func, - toolbarColumns: arrayOf(shape({ - name: string.isRequired, - key: string.isRequired, - isSortable: bool, - })), + toolbarColumns: arrayOf( + shape({ + name: string.isRequired, + key: string.isRequired, + isSortable: bool, + }) + ), showPageSizeOptions: PropTypes.bool, renderToolbar: PropTypes.func, hasContentLoading: PropTypes.bool, @@ -160,8 +183,8 @@ PaginatedDataList.defaultProps = { itemName: 'item', itemNamePlural: '', showPageSizeOptions: true, - renderItem: (item) => (), - renderToolbar: (props) => (), + renderItem: item => , + renderToolbar: props => , }; export { PaginatedDataList as _PaginatedDataList }; diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx index c03c26d1ff..8ca40de3df 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.test.jsx @@ -51,7 +51,8 @@ describe('', () => { order_by: 'name', }} qsConfig={qsConfig} - />, { context: { router: { history } } } + />, + { context: { router: { history } } } ); const pagination = wrapper.find('Pagination'); @@ -77,7 +78,8 @@ describe('', () => { order_by: 'name', }} qsConfig={qsConfig} - />, { context: { router: { history } } } + />, + { context: { router: { history } } } ); const pagination = wrapper.find('Pagination'); diff --git a/awx/ui_next/src/components/Search/Search.jsx b/awx/ui_next/src/components/Search/Search.jsx index 007e749c7c..54e434688b 100644 --- a/awx/ui_next/src/components/Search/Search.jsx +++ b/awx/ui_next/src/components/Search/Search.jsx @@ -10,11 +10,9 @@ import { DropdownItem, Form, FormGroup, - TextInput as PFTextInput + TextInput as PFTextInput, } from '@patternfly/react-core'; -import { - SearchIcon -} from '@patternfly/react-icons'; +import { SearchIcon } from '@patternfly/react-icons'; import styled from 'styled-components'; @@ -29,7 +27,8 @@ const Button = styled(PFButton)` `; const Dropdown = styled(PFDropdown)` - &&& { /* Higher specificity required because we are selecting unclassed elements */ + &&& { + /* Higher specificity required because we are selecting unclassed elements */ > button { min-height: 30px; min-width: 70px; @@ -37,17 +36,19 @@ const Dropdown = styled(PFDropdown)` padding: 0 10px; margin: 0px; - > span { /* text element */ + > span { + /* text element */ width: auto; } - > svg { /* caret icon */ + > svg { + /* caret icon */ margin: 0px; padding-top: 3px; padding-left: 3px; } } - } + } `; const NoOptionDropdown = styled.div` @@ -61,7 +62,7 @@ const InputFormGroup = styled(FormGroup)` `; class Search extends React.Component { - constructor (props) { + constructor(props) { super(props); const { sortedColumnKey } = this.props; @@ -77,11 +78,11 @@ class Search extends React.Component { this.handleSearch = this.handleSearch.bind(this); } - handleDropdownToggle (isSearchDropdownOpen) { + handleDropdownToggle(isSearchDropdownOpen) { this.setState({ isSearchDropdownOpen }); } - handleDropdownSelect ({ target }) { + handleDropdownSelect({ target }) { const { columns } = this.props; const { innerText } = target; @@ -89,7 +90,7 @@ class Search extends React.Component { this.setState({ isSearchDropdownOpen: false, searchKey }); } - handleSearch (e) { + handleSearch(e) { // keeps page from fully reloading e.preventDefault(); @@ -102,22 +103,17 @@ class Search extends React.Component { this.setState({ searchValue: '' }); } - handleSearchInputChange (searchValue) { + handleSearchInputChange(searchValue) { this.setState({ searchValue }); } - render () { + render() { const { up } = DropdownPosition; - const { - columns, - i18n - } = this.props; - const { - isSearchDropdownOpen, - searchKey, - searchValue, - } = this.state; - const { name: searchColumnName } = columns.find(({ key }) => key === searchKey); + const { columns, i18n } = this.props; + const { isSearchDropdownOpen, searchKey, searchValue } = this.state; + const { name: searchColumnName } = columns.find( + ({ key }) => key === searchKey + ); const searchDropdownItems = columns .filter(({ key, isSearchable }) => isSearchable && key !== searchKey) @@ -133,32 +129,38 @@ class Search extends React.Component { {searchDropdownItems.length > 0 ? ( {i18n._(t`Search key dropdown`)})} + label={ + + {i18n._(t`Search key dropdown`)} + + } > {searchColumnName} - )} + } dropdownItems={searchDropdownItems} /> ) : ( - - {searchColumnName} - + {searchColumnName} )} {i18n._(t`Search value text input`)})} + label={ + + {i18n._(t`Search value text input`)} + + } > ', () => { }); test('it triggers the expected callbacks', () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; + const columns = [ + { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, + ]; const searchBtn = 'button[aria-label="Search submit button"]'; const searchTextInput = 'input[aria-label="Search text input"]'; @@ -20,11 +22,7 @@ describe('', () => { const onSearch = jest.fn(); search = mountWithContexts( - + ); search.find(searchTextInput).instance().value = 'test-321'; @@ -36,14 +34,12 @@ describe('', () => { }); test('handleDropdownToggle properly updates state', async () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; + const columns = [ + { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, + ]; const onSearch = jest.fn(); const wrapper = mountWithContexts( - + ).find('Search'); expect(wrapper.state('isSearchDropdownOpen')).toEqual(false); wrapper.instance().handleDropdownToggle(true); @@ -53,18 +49,21 @@ describe('', () => { test('handleDropdownSelect properly updates state', async () => { const columns = [ { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, - { name: 'Description', key: 'description', isSortable: true, isSearchable: true } + { + name: 'Description', + key: 'description', + isSortable: true, + isSearchable: true, + }, ]; const onSearch = jest.fn(); const wrapper = mountWithContexts( - + ).find('Search'); expect(wrapper.state('searchKey')).toEqual('name'); - wrapper.instance().handleDropdownSelect({ target: { innerText: 'Description' } }); + wrapper + .instance() + .handleDropdownSelect({ target: { innerText: 'Description' } }); expect(wrapper.state('searchKey')).toEqual('description'); }); }); diff --git a/awx/ui_next/src/components/Sort/Sort.test.jsx b/awx/ui_next/src/components/Sort/Sort.test.jsx index b979ed4ee0..0e0208cd16 100644 --- a/awx/ui_next/src/components/Sort/Sort.test.jsx +++ b/awx/ui_next/src/components/Sort/Sort.test.jsx @@ -12,7 +12,9 @@ describe('', () => { }); test('it triggers the expected callbacks', () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true, isSearchable: true }]; + const columns = [ + { name: 'Name', key: 'name', isSortable: true, isSearchable: true }, + ]; const sortBtn = 'button[aria-label="Sort"]'; @@ -38,7 +40,7 @@ describe('', () => { { name: 'Foo', key: 'foo', isSortable: true }, { name: 'Bar', key: 'bar', isSortable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' } + { name: 'Baz', key: 'baz' }, ]; const onSort = jest.fn(); @@ -62,7 +64,7 @@ describe('', () => { { name: 'Foo', key: 'foo', isSortable: true }, { name: 'Bar', key: 'bar', isSortable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' } + { name: 'Baz', key: 'baz' }, ]; const onSort = jest.fn(); @@ -86,7 +88,7 @@ describe('', () => { { name: 'Foo', key: 'foo', isSortable: true }, { name: 'Bar', key: 'bar', isSortable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' } + { name: 'Baz', key: 'baz' }, ]; const onSort = jest.fn(); @@ -109,7 +111,7 @@ describe('', () => { { name: 'Foo', key: 'foo', isSortable: true }, { name: 'Bar', key: 'bar', isSortable: true }, { name: 'Bakery', key: 'bakery', isSortable: true }, - { name: 'Baz', key: 'baz' } + { name: 'Baz', key: 'baz' }, ]; const onSort = jest.fn(); @@ -133,8 +135,12 @@ describe('', () => { const downAlphaIconSelector = 'SortAlphaDownIcon'; const upAlphaIconSelector = 'SortAlphaUpIcon'; - const numericColumns = [{ name: 'ID', key: 'id', isSortable: true, isNumeric: true }]; - const alphaColumns = [{ name: 'Name', key: 'name', isSortable: true, isNumeric: false }]; + const numericColumns = [ + { name: 'ID', key: 'id', isSortable: true, isNumeric: true }, + ]; + const alphaColumns = [ + { name: 'Name', key: 'name', isSortable: true, isNumeric: false }, + ]; const onSort = jest.fn(); sort = mountWithContexts( diff --git a/awx/ui_next/src/screens/Job/JobList/JobList.jsx b/awx/ui_next/src/screens/Job/JobList/JobList.jsx index 62ca1eb5b5..6a4c44067d 100644 --- a/awx/ui_next/src/screens/Job/JobList/JobList.jsx +++ b/awx/ui_next/src/screens/Job/JobList/JobList.jsx @@ -2,17 +2,13 @@ import React, { Component } from 'react'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { - Card, - PageSection, - PageSectionVariants, -} from '@patternfly/react-core'; +import { Card, PageSection, PageSectionVariants } from '@patternfly/react-core'; import { UnifiedJobsAPI } from '@api'; import AlertModal from '@components/AlertModal'; import DatalistToolbar from '@components/DataListToolbar'; import PaginatedDataList, { - ToolbarDeleteButton + ToolbarDeleteButton, } from '@components/PaginatedDataList'; import { getQSConfig, parseQueryString } from '@util/qs'; @@ -26,12 +22,12 @@ const QS_CONFIG = getQSConfig('job', { }); class JobList extends Component { - constructor (props) { + constructor(props) { super(props); this.state = { hasContentLoading: true, - hasContentError: false, + contentError: null, deletionError: false, selected: [], jobs: [], @@ -44,28 +40,28 @@ class JobList extends Component { this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this); } - componentDidMount () { + componentDidMount() { this.loadJobs(); } - componentDidUpdate (prevProps) { + componentDidUpdate(prevProps) { const { location } = this.props; if (location !== prevProps.location) { this.loadJobs(); } } - handleDeleteErrorClose () { + handleDeleteErrorClose() { this.setState({ deletionError: false }); } - handleSelectAll (isSelected) { + handleSelectAll(isSelected) { const { jobs } = this.state; const selected = isSelected ? [...jobs] : []; this.setState({ selected }); } - handleSelect (item) { + handleSelect(item) { const { selected } = this.state; if (selected.some(s => s.id === item.id)) { this.setState({ selected: selected.filter(s => s.id !== item.id) }); @@ -74,7 +70,7 @@ class JobList extends Component { } } - async handleDelete () { + async handleDelete() { const { selected } = this.state; this.setState({ hasContentLoading: true, deletionError: false }); try { @@ -86,38 +82,37 @@ class JobList extends Component { } } - async loadJobs () { + async loadJobs() { const { location } = this.props; const params = parseQueryString(QS_CONFIG, location.search); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { - const { data: { count, results } } = await UnifiedJobsAPI.read(params); + const { + data: { count, results }, + } = await UnifiedJobsAPI.read(params); this.setState({ itemCount: count, jobs: results, selected: [], }); } catch (err) { - this.setState({ hasContentError: true }); + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } } - render () { + render() { const { - hasContentError, + contentError, hasContentLoading, deletionError, jobs, itemCount, selected, } = this.state; - const { - match, - i18n - } = this.props; + const { match, i18n } = this.props; const { medium } = PageSectionVariants; const isAllSelected = selected.length === jobs.length; const itemName = i18n._(t`Job`); @@ -125,17 +120,27 @@ class JobList extends Component { ( + renderToolbar={props => ( + />, ]} /> )} - renderItem={(job) => ( + renderItem={job => ( ( + renderToolbar={props => ( - ] : null} + additionalControls={ + canEdit + ? [ + , + ] + : null + } /> )} renderItem={accessRecord => ( diff --git a/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx index 12dd0dbc68..b0d0b1bb33 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx @@ -103,7 +103,7 @@ describe('', () => { expect(wrapper.find('OrganizationAccess').state('hasContentLoading')).toBe( false ); - expect(wrapper.find('OrganizationAccess').state('hasContentError')).toBe(false); + expect(wrapper.find('OrganizationAccess').state('contentError')).toBe(null); done(); }); diff --git a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap index 1eb2c6cc4f..0d6d397ed1 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap +++ b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap @@ -34,7 +34,7 @@ exports[` initially renders succesfully 1`] = ` } > initially renders succesfully 1`] = ` withHash={true} > initially renders succesfully 1`] = ` > s.id === row.id)) { @@ -74,16 +70,16 @@ class OrganizationsList extends Component { } } - handleDeleteErrorClose () { + handleDeleteErrorClose() { this.setState({ hasDeletionError: false }); } - async handleOrgDelete () { + async handleOrgDelete() { const { selected } = this.state; this.setState({ hasContentLoading: true, hasDeletionError: false }); try { - await Promise.all(selected.map((org) => OrganizationsAPI.destroy(org.id))); + await Promise.all(selected.map(org => OrganizationsAPI.destroy(org.id))); } catch (err) { this.setState({ hasDeletionError: true }); } finally { @@ -91,7 +87,7 @@ class OrganizationsList extends Component { } } - async loadOrganizations () { + async loadOrganizations() { const { location } = this.props; const { actions: cachedActions } = this.state; const params = parseQueryString(QS_CONFIG, location.search); @@ -108,9 +104,16 @@ class OrganizationsList extends Component { optionsPromise, ]); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { - const [{ data: { count, results } }, { data: { actions } }] = await promises; + const [ + { + data: { count, results }, + }, + { + data: { actions }, + }, + ] = await promises; this.setState({ actions, itemCount: count, @@ -118,20 +121,18 @@ class OrganizationsList extends Component { selected: [], }); } catch (err) { - this.setState(({ hasContentError: true })); + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } } - render () { - const { - medium, - } = PageSectionVariants; + render() { + const { medium } = PageSectionVariants; const { actions, itemCount, - hasContentError, + contentError, hasContentLoading, hasDeletionError, selected, @@ -139,7 +140,8 @@ class OrganizationsList extends Component { } = this.state; const { match, i18n } = this.props; - const canAdd = actions && Object.prototype.hasOwnProperty.call(actions, 'POST'); + const canAdd = + actions && Object.prototype.hasOwnProperty.call(actions, 'POST'); const isAllSelected = selected.length === organizations.length; return ( @@ -147,18 +149,33 @@ class OrganizationsList extends Component { ( + renderToolbar={props => ( , - canAdd - ? - : null, + canAdd ? ( + + ) : null, ]} /> )} - renderItem={(o) => ( + renderItem={o => ( )} emptyStateControls={ - canAdd ? - : null + canAdd ? ( + + ) : null } /> diff --git a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx index 2abe9941c1..a3861d666f 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx @@ -24,7 +24,7 @@ const COLUMNS = [ ]; class OrganizationNotifications extends Component { - constructor (props) { + constructor(props) { super(props); this.state = { contentError: null, @@ -38,22 +38,24 @@ class OrganizationNotifications extends Component { typeLabels: null, }; this.handleNotificationToggle = this.handleNotificationToggle.bind(this); - this.handleNotificationErrorClose = this.handleNotificationErrorClose.bind(this); + this.handleNotificationErrorClose = this.handleNotificationErrorClose.bind( + this + ); this.loadNotifications = this.loadNotifications.bind(this); } - componentDidMount () { + componentDidMount() { this.loadNotifications(); } - componentDidUpdate (prevProps) { + componentDidUpdate(prevProps) { const { location } = this.props; if (location !== prevProps.location) { this.loadNotifications(); } } - async loadNotifications () { + async loadNotifications() { const { id, location } = this.props; const { typeLabels } = this.state; const params = parseQueryString(QS_CONFIG, location.search); @@ -67,13 +69,13 @@ class OrganizationNotifications extends Component { this.setState({ contentError: null, hasContentLoading: true }); try { const { - data: { - count: itemCount = 0, - results: notifications = [], - } + data: { count: itemCount = 0, results: notifications = [] }, } = await OrganizationsAPI.readNotificationTemplates(id, params); - const optionsResponse = await OrganizationsAPI.readOptionsNotificationTemplates(id, params); + const optionsResponse = await OrganizationsAPI.readOptionsNotificationTemplates( + id, + params + ); let idMatchParams; if (notifications.length > 0) { @@ -122,7 +124,7 @@ class OrganizationNotifications extends Component { } } - async handleNotificationToggle (notificationId, isCurrentlyOn, status) { + async handleNotificationToggle(notificationId, isCurrentlyOn, status) { const { id } = this.props; let stateArrayName; @@ -135,13 +137,15 @@ class OrganizationNotifications extends Component { let stateUpdateFunction; if (isCurrentlyOn) { // when switching off, remove the toggled notification id from the array - stateUpdateFunction = (prevState) => ({ - [stateArrayName]: prevState[stateArrayName].filter(i => i !== notificationId) + stateUpdateFunction = prevState => ({ + [stateArrayName]: prevState[stateArrayName].filter( + i => i !== notificationId + ), }); } else { // when switching on, add the toggled notification id to the array - stateUpdateFunction = (prevState) => ({ - [stateArrayName]: prevState[stateArrayName].concat(notificationId) + stateUpdateFunction = prevState => ({ + [stateArrayName]: prevState[stateArrayName].concat(notificationId), }); } @@ -161,11 +165,11 @@ class OrganizationNotifications extends Component { } } - handleNotificationErrorClose () { + handleNotificationErrorClose() { this.setState({ toggleError: false }); } - render () { + render() { const { canToggleNotifications, i18n } = this.props; const { contentError, @@ -176,7 +180,7 @@ class OrganizationNotifications extends Component { notifications, successTemplateIds, errorTemplateIds, - typeLabels + typeLabels, } = this.state; return ( @@ -189,7 +193,7 @@ class OrganizationNotifications extends Component { itemName="notification" qsConfig={QS_CONFIG} toolbarColumns={COLUMNS} - renderItem={(notification) => ( + renderItem={notification => ( initially renders succesfully 1`] = ` InstanceGroupsAPI.read(params); +const getInstanceGroups = async params => InstanceGroupsAPI.read(params); class InstanceGroupsLookup extends React.Component { - render () { + render() { const { value, tooltip, onChange, i18n } = this.props; return ( - {i18n._(t`Instance Groups`)} - {' '} - { - tooltip && ( - - - - ) - } + {i18n._(t`Instance Groups`)}{' '} + {tooltip && ( + + + + )} - )} + } fieldId="org-instance-groups" > diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index ebc9b3b81c..aa278fedf8 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -2,21 +2,17 @@ import React, { Component } from 'react'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { - Card, - PageSection, - PageSectionVariants, -} from '@patternfly/react-core'; +import { Card, PageSection, PageSectionVariants } from '@patternfly/react-core'; import { JobTemplatesAPI, UnifiedJobTemplatesAPI, - WorkflowJobTemplatesAPI + WorkflowJobTemplatesAPI, } from '@api'; import AlertModal from '@components/AlertModal'; import DatalistToolbar from '@components/DataListToolbar'; import PaginatedDataList, { - ToolbarDeleteButton + ToolbarDeleteButton, } from '@components/PaginatedDataList'; import { getQSConfig, parseQueryString } from '@util/qs'; @@ -28,16 +24,16 @@ const QS_CONFIG = getQSConfig('template', { page: 1, page_size: 5, order_by: 'name', - type: 'job_template,workflow_job_template' + type: 'job_template,workflow_job_template', }); class TemplatesList extends Component { - constructor (props) { + constructor(props) { super(props); this.state = { hasContentLoading: true, - hasContentError: false, + contentError: null, hasDeletionError: false, selected: [], templates: [], @@ -50,28 +46,28 @@ class TemplatesList extends Component { this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this); } - componentDidMount () { + componentDidMount() { this.loadTemplates(); } - componentDidUpdate (prevProps) { + componentDidUpdate(prevProps) { const { location } = this.props; if (location !== prevProps.location) { this.loadTemplates(); } } - handleDeleteErrorClose () { + handleDeleteErrorClose() { this.setState({ hasDeletionError: false }); } - handleSelectAll (isSelected) { + handleSelectAll(isSelected) { const { templates } = this.state; const selected = isSelected ? [...templates] : []; this.setState({ selected }); } - handleSelect (template) { + handleSelect(template) { const { selected } = this.state; if (selected.some(s => s.id === template.id)) { this.setState({ selected: selected.filter(s => s.id !== template.id) }); @@ -80,20 +76,22 @@ class TemplatesList extends Component { } } - async handleTemplateDelete () { + async handleTemplateDelete() { const { selected } = this.state; this.setState({ hasContentLoading: true, hasDeletionError: false }); try { - await Promise.all(selected.map(({ type, id }) => { - let deletePromise; - if (type === 'job_template') { - deletePromise = JobTemplatesAPI.destroy(id); - } else if (type === 'workflow_job_template') { - deletePromise = WorkflowJobTemplatesAPI.destroy(id); - } - return deletePromise; - })); + await Promise.all( + selected.map(({ type, id }) => { + let deletePromise; + if (type === 'job_template') { + deletePromise = JobTemplatesAPI.destroy(id); + } else if (type === 'workflow_job_template') { + deletePromise = WorkflowJobTemplatesAPI.destroy(id); + } + return deletePromise; + }) + ); } catch (err) { this.setState({ hasDeletionError: true }); } finally { @@ -101,56 +99,70 @@ class TemplatesList extends Component { } } - async loadTemplates () { + async loadTemplates() { const { location } = this.props; const params = parseQueryString(QS_CONFIG, location.search); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { - const { data: { count, results } } = await UnifiedJobTemplatesAPI.read(params); + const { + data: { count, results }, + } = await UnifiedJobTemplatesAPI.read(params); this.setState({ itemCount: count, templates: results, selected: [], }); } catch (err) { - this.setState({ hasContentError: true }); + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } } - render () { + render() { const { - hasContentError, + contentError, hasContentLoading, hasDeletionError, templates, itemCount, selected, } = this.state; - const { - match, - i18n - } = this.props; + const { match, i18n } = this.props; const isAllSelected = selected.length === templates.length; const { medium } = PageSectionVariants; return ( ( + renderToolbar={props => ( + />, ]} /> )} - renderItem={(template) => ( + renderItem={template => ( entriesArr.reduce((acc, [key, value]) => { - if (acc[key] && Array.isArray(acc[key])) { - acc[key].push(value); - } else if (acc[key]) { - acc[key] = [acc[key], value]; - } else { - acc[key] = value; - } - return acc; -}, {}); +const toObject = entriesArr => + entriesArr.reduce((acc, [key, value]) => { + if (acc[key] && Array.isArray(acc[key])) { + acc[key].push(value); + } else if (acc[key]) { + acc[key] = [acc[key], value]; + } else { + acc[key] = value; + } + return acc; + }, {}); /** * helper function to namespace params object @@ -30,7 +31,7 @@ const namespaceParams = (namespace, params = {}) => { }); return namespaced || {}; -} +}; /** * helper function to remove namespace from params object @@ -47,7 +48,7 @@ const denamespaceParams = (namespace, params = {}) => { }); return denamespaced || {}; -} +}; /** * helper function to check the namespace of a param is what you expec @@ -59,7 +60,7 @@ const namespaceMatches = (namespace, fieldname) => { if (!namespace) return !fieldname.includes('.'); return fieldname.startsWith(`${namespace}.`); -} +}; /** * helper function to check the value of a param is equal to another @@ -72,13 +73,15 @@ const paramValueIsEqual = (one, two) => { if (Array.isArray(one) && Array.isArray(two)) { isEqual = one.filter(val => two.indexOf(val) > -1).length === 0; - } else if ((typeof(one) === "string" && typeof(two) === "string") || - (typeof(one) === "number" && typeof(two) === "number")){ + } else if ( + (typeof one === 'string' && typeof two === 'string') || + (typeof one === 'number' && typeof two === 'number') + ) { isEqual = one === two; } return isEqual; -} +}; /** * Convert query param object to url query string @@ -87,17 +90,19 @@ const paramValueIsEqual = (one, two) => { * @param {object} query param object * @return {string} url query string */ -export const encodeQueryString = (params) => { +export const encodeQueryString = params => { if (!params) return ''; return Object.keys(params) .sort() .filter(key => params[key] !== null) - .map(key => ([key, params[key]])) + .map(key => [key, params[key]]) .map(([key, value]) => { // if value is array, should return more than one key value pair if (Array.isArray(value)) { - return value.map(val => `${encodeURIComponent(key)}=${encodeURIComponent(val)}`).join('&'); + return value + .map(val => `${encodeURIComponent(key)}=${encodeURIComponent(val)}`) + .join('&'); } return `${encodeURIComponent(key)}=${encodeURIComponent(value)}`; }) @@ -115,22 +120,31 @@ export const encodeNonDefaultQueryString = (config, params) => { if (!params) return ''; const namespacedParams = namespaceParams(config.namespace, params); - const namespacedDefaults = namespaceParams(config.namespace, config.defaultParams); + const namespacedDefaults = namespaceParams( + config.namespace, + config.defaultParams + ); const namespacedDefaultKeys = Object.keys(namespacedDefaults); - const namespacedParamsWithoutDefaultsKeys = Object.keys(namespacedParams) - .filter(key => namespacedDefaultKeys.indexOf(key) === -1 || - !paramValueIsEqual(namespacedParams[key], namespacedDefaults[key])); + const namespacedParamsWithoutDefaultsKeys = Object.keys( + namespacedParams + ).filter( + key => + namespacedDefaultKeys.indexOf(key) === -1 || + !paramValueIsEqual(namespacedParams[key], namespacedDefaults[key]) + ); return namespacedParamsWithoutDefaultsKeys .sort() .filter(key => namespacedParams[key] !== null) .map(key => { - return ([key, namespacedParams[key]]) + return [key, namespacedParams[key]]; }) .map(([key, value]) => { // if value is array, should return more than one key value pair if (Array.isArray(value)) { - return value.map(val => `${encodeURIComponent(key)}=${encodeURIComponent(val)}`).join('&'); + return value + .map(val => `${encodeURIComponent(key)}=${encodeURIComponent(val)}`) + .join('&'); } return `${encodeURIComponent(key)}=${encodeURIComponent(value)}`; }) @@ -144,7 +158,7 @@ export const encodeNonDefaultQueryString = (config, params) => { * @param {array} params that are number fields * @return {object} query param object */ -export function getQSConfig ( +export function getQSConfig( namespace, defaultParams = { page: 1, page_size: 5, order_by: 'name' }, integerFields = ['page', 'page_size'] @@ -165,12 +179,16 @@ export function getQSConfig ( * @param {string} url query string * @return {object} query param object */ -export function parseQueryString (config, queryString) { +export function parseQueryString(config, queryString) { if (!queryString) return config.defaultParams; - const namespacedIntegerFields = config.integerFields.map(f => config.namespace ? `${config.namespace}.${f}` : f); + const namespacedIntegerFields = config.integerFields.map(f => + config.namespace ? `${config.namespace}.${f}` : f + ); - const keyValuePairs = queryString.replace(/^\?/, '').split('&') + const keyValuePairs = queryString + .replace(/^\?/, '') + .split('&') .map(s => s.split('=')) .map(([key, value]) => { if (namespacedIntegerFields.includes(key)) { @@ -185,37 +203,38 @@ export function parseQueryString (config, queryString) { // needs to return array for duplicate keys // ie [[k1, v1], [k1, v2], [k2, v3]] // -> [[k1, [v1, v2]], [k2, v3]] - const dedupedKeyValuePairs = Object.keys(keyValueObject) - .map(key => { - const values = keyValuePairs - .filter(([k]) => k === key) - .map(([, v]) => v); + const dedupedKeyValuePairs = Object.keys(keyValueObject).map(key => { + const values = keyValuePairs.filter(([k]) => k === key).map(([, v]) => v); - if (values.length === 1) { - return [key, values[0]]; - } + if (values.length === 1) { + return [key, values[0]]; + } - return [key, values]; - }); + return [key, values]; + }); - const parsed = Object.assign(...dedupedKeyValuePairs.map(([k, v]) => ({ - [k]: v - }))); + const parsed = Object.assign( + ...dedupedKeyValuePairs.map(([k, v]) => ({ + [k]: v, + })) + ); const namespacedParams = {}; - Object.keys(parsed) - .forEach(field => { - if (namespaceMatches(config.namespace, field)) { - let fieldname = field; - if (config.namespace) { - fieldname = field.substr(config.namespace.length + 1); - } - namespacedParams[fieldname] = parsed[field]; + Object.keys(parsed).forEach(field => { + if (namespaceMatches(config.namespace, field)) { + let fieldname = field; + if (config.namespace) { + fieldname = field.substr(config.namespace.length + 1); } - }); + namespacedParams[fieldname] = parsed[field]; + } + }); - const namespacedDefaults = namespaceParams(config.namespace, config.defaultParams); + const namespacedDefaults = namespaceParams( + config.namespace, + config.defaultParams + ); Object.keys(namespacedDefaults) .filter(key => Object.keys(parsed).indexOf(key) === -1) @@ -238,11 +257,12 @@ export function parseQueryString (config, queryString) { * @param {object} namespaced params object of default params * @return {object} namespaced params object of only defaults */ -const getDefaultParams = (params, defaults) => toObject( - Object.keys(params) - .filter(key => Object.keys(defaults).indexOf(key) > -1) - .map(key => [key, params[key]]) -); +const getDefaultParams = (params, defaults) => + toObject( + Object.keys(params) + .filter(key => Object.keys(defaults).indexOf(key) > -1) + .map(key => [key, params[key]]) + ); /** * helper function to get params that are not defaults @@ -250,11 +270,12 @@ const getDefaultParams = (params, defaults) => toObject( * @param {object} namespaced params object of default params * @return {object} namespaced params object of non-defaults */ -const getNonDefaultParams = (params, defaults) => toObject( - Object.keys(params) - .filter(key => Object.keys(defaults).indexOf(key) === -1) - .map(key => [key, params[key]]) -); +const getNonDefaultParams = (params, defaults) => + toObject( + Object.keys(params) + .filter(key => Object.keys(defaults).indexOf(key) === -1) + .map(key => [key, params[key]]) + ); /** * helper function to merge old and new params together @@ -262,9 +283,9 @@ const getNonDefaultParams = (params, defaults) => toObject( * @param {object} namespaced params object of new params * @return {object} merged namespaced params object */ -const getMergedParams = (oldParams, newParams) => toObject( - Object.keys(oldParams) - .map(key => { +const getMergedParams = (oldParams, newParams) => + toObject( + Object.keys(oldParams).map(key => { let oldVal = oldParams[key]; const newVal = newParams[key]; if (newVal) { @@ -276,7 +297,7 @@ const getMergedParams = (oldParams, newParams) => toObject( } return [key, oldVal]; }) -); + ); /** * helper function to get new params that are not in merged params @@ -284,11 +305,12 @@ const getMergedParams = (oldParams, newParams) => toObject( * @param {object} namespaced params object of new params * @return {object} remaining new namespaced params object */ -const getRemainingNewParams = (mergedParams, newParams) => toObject( - Object.keys(newParams) - .filter(key => Object.keys(mergedParams).indexOf(key) === -1) - .map(key => [key, newParams[key]]) -); +const getRemainingNewParams = (mergedParams, newParams) => + toObject( + Object.keys(newParams) + .filter(key => Object.keys(mergedParams).indexOf(key) === -1) + .map(key => [key, newParams[key]]) + ); /** * Merges existing params of search string with new ones and returns the updated list of params @@ -297,13 +319,16 @@ const getRemainingNewParams = (mergedParams, newParams) => toObject( * @param {object} object with new params to add * @return {object} query param object */ -export function addParams (config, searchString, newParams) { +export function addParams(config, searchString, newParams) { const namespacedOldParams = namespaceParams( config.namespace, parseQueryString(config, searchString) ); const namespacedNewParams = namespaceParams(config.namespace, newParams); - const namespacedDefaultParams = namespaceParams(config.namespace, config.defaultParams); + const namespacedDefaultParams = namespaceParams( + config.namespace, + config.defaultParams + ); const namespacedOldParamsNotDefaults = getNonDefaultParams( namespacedOldParams, @@ -320,7 +345,7 @@ export function addParams (config, searchString, newParams) { return denamespaceParams(config.namespace, { ...getDefaultParams(namespacedOldParams, namespacedDefaultParams), ...namespacedMergedParams, - ...getRemainingNewParams(namespacedMergedParams, namespacedNewParams) + ...getRemainingNewParams(namespacedMergedParams, namespacedNewParams), }); } @@ -331,26 +356,29 @@ export function addParams (config, searchString, newParams) { * @param {object} object with new params to remove * @return {object} query param object */ -export function removeParams (config, searchString, paramsToRemove) { +export function removeParams(config, searchString, paramsToRemove) { const oldParams = parseQueryString(config, searchString); const paramsEntries = []; - Object.entries(oldParams) - .forEach(([key, value]) => { - if (Array.isArray(value)) { - value.forEach(val => { - paramsEntries.push([key, val]); - }) - } else { - paramsEntries.push([key, value]); - } - }) + Object.entries(oldParams).forEach(([key, value]) => { + if (Array.isArray(value)) { + value.forEach(val => { + paramsEntries.push([key, val]); + }); + } else { + paramsEntries.push([key, value]); + } + }); const paramsToRemoveEntries = Object.entries(paramsToRemove); - const remainingEntries = paramsEntries - .filter(([key, value]) => paramsToRemoveEntries - .filter(([newKey, newValue]) => key === newKey && value === newValue).length === 0); + const remainingEntries = paramsEntries.filter( + ([key, value]) => + paramsToRemoveEntries.filter( + ([newKey, newValue]) => key === newKey && value === newValue + ).length === 0 + ); const remainingObject = toObject(remainingEntries); - const defaultEntriesLeftover = Object.entries(config.defaultParams) - .filter(([key]) => !remainingObject[key]); + const defaultEntriesLeftover = Object.entries(config.defaultParams).filter( + ([key]) => !remainingObject[key] + ); const finalParamsEntries = remainingEntries; defaultEntriesLeftover.forEach(value => { finalParamsEntries.push(value); diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index c6440b24dd..bdea7840a2 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -4,7 +4,7 @@ import { parseQueryString, getQSConfig, addParams, - removeParams + removeParams, } from './qs'; describe('qs (qs.js)', () => { @@ -13,14 +13,19 @@ describe('qs (qs.js)', () => { [ [null, ''], [{}, ''], - [{ order_by: 'name', page: 1, page_size: 5 }, 'order_by=name&page=1&page_size=5'], - [{ '-order_by': 'name', page: '1', page_size: 5 }, '-order_by=name&page=1&page_size=5'], - ] - .forEach(([params, expectedQueryString]) => { - const actualQueryString = encodeQueryString(params); + [ + { order_by: 'name', page: 1, page_size: 5 }, + 'order_by=name&page=1&page_size=5', + ], + [ + { '-order_by': 'name', page: '1', page_size: 5 }, + '-order_by=name&page=1&page_size=5', + ], + ].forEach(([params, expectedQueryString]) => { + const actualQueryString = encodeQueryString(params); - expect(actualQueryString).toEqual(expectedQueryString); - }); + expect(actualQueryString).toEqual(expectedQueryString); + }); }); test('encodeQueryString omits null values', () => { @@ -35,7 +40,7 @@ describe('qs (qs.js)', () => { describe('encodeNonDefaultQueryString', () => { const config = { namespace: null, - defaultParams: { page: 1, page_size: 5, order_by: 'name'}, + defaultParams: { page: 1, page_size: 5, order_by: 'name' }, integerFields: ['page'], }; @@ -45,14 +50,19 @@ describe('qs (qs.js)', () => { [{}, ''], [{ order_by: 'name', page: 1, page_size: 5 }, ''], [{ order_by: '-name', page: 1, page_size: 5 }, 'order_by=-name'], - [{ order_by: '-name', page: 3, page_size: 10 }, 'order_by=-name&page=3&page_size=10'], - [{ order_by: '-name', page: 3, page_size: 10, foo: 'bar' }, 'foo=bar&order_by=-name&page=3&page_size=10'], - ] - .forEach(([params, expectedQueryString]) => { - const actualQueryString = encodeNonDefaultQueryString(config, params); + [ + { order_by: '-name', page: 3, page_size: 10 }, + 'order_by=-name&page=3&page_size=10', + ], + [ + { order_by: '-name', page: 3, page_size: 10, foo: 'bar' }, + 'foo=bar&order_by=-name&page=3&page_size=10', + ], + ].forEach(([params, expectedQueryString]) => { + const actualQueryString = encodeNonDefaultQueryString(config, params); - expect(actualQueryString).toEqual(expectedQueryString); - }); + expect(actualQueryString).toEqual(expectedQueryString); + }); }); test('encodeNonDefaultQueryString omits null values', () => { @@ -114,7 +124,7 @@ describe('qs (qs.js)', () => { const query = ''; expect(parseQueryString(config, query)).toEqual({ page: 1, - page_size: 15 + page_size: 15, }); }); @@ -222,7 +232,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?baz=bar&page=3'; - const newParams = { bag: 'boom' } + const newParams = { bag: 'boom' }; expect(addParams(config, query, newParams)).toEqual({ baz: 'bar', bag: 'boom', @@ -238,7 +248,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?baz=bar&baz=bang&page=3'; - const newParams = { baz: 'boom' } + const newParams = { baz: 'boom' }; expect(addParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang', 'boom'], page: 3, @@ -253,7 +263,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?baz=bar&baz=bang&page=3'; - const newParams = { page: 5 } + const newParams = { page: 5 }; expect(addParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang'], page: 5, @@ -268,7 +278,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?baz=bar&baz=bang&page=3'; - const newParams = { baz: 'bust', pat: 'pal' } + const newParams = { baz: 'bust', pat: 'pal' }; expect(addParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang', 'bust'], pat: 'pal', @@ -284,7 +294,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&item.page=3'; - const newParams = { bag: 'boom' } + const newParams = { bag: 'boom' }; expect(addParams(config, query, newParams)).toEqual({ baz: 'bar', bag: 'boom', @@ -300,7 +310,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&foo.page=3'; - const newParams = { bag: 'boom' } + const newParams = { bag: 'boom' }; expect(addParams(config, query, newParams)).toEqual({ baz: 'bar', bag: 'boom', @@ -316,7 +326,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&item.baz=bang&item.page=3'; - const newParams = { baz: 'boom' } + const newParams = { baz: 'boom' }; expect(addParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang', 'boom'], page: 3, @@ -331,7 +341,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&item.baz=bang&item.page=3'; - const newParams = { page: 5 } + const newParams = { page: 5 }; expect(addParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang'], page: 5, @@ -346,7 +356,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&item.baz=bang&item.page=3'; - const newParams = { baz: 'bust', pat: 'pal' } + const newParams = { baz: 'bust', pat: 'pal' }; expect(addParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang', 'bust'], pat: 'pal', @@ -364,7 +374,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?baz=bar&page=3&bag=boom'; - const newParams = { bag: 'boom' } + const newParams = { bag: 'boom' }; expect(removeParams(config, query, newParams)).toEqual({ baz: 'bar', page: 3, @@ -379,7 +389,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?baz=bar&baz=bang&page=3'; - const newParams = { baz: 'bar' } + const newParams = { baz: 'bar' }; expect(removeParams(config, query, newParams)).toEqual({ baz: 'bang', page: 3, @@ -394,7 +404,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?baz=bar&baz=bang&baz=bust&page=3'; - const newParams = { baz: 'bar' } + const newParams = { baz: 'bar' }; expect(removeParams(config, query, newParams)).toEqual({ baz: ['bang', 'bust'], page: 3, @@ -409,7 +419,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?baz=bar&baz=bang&page=3'; - const newParams = { page: 3 } + const newParams = { page: 3 }; expect(removeParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang'], page: 1, @@ -424,7 +434,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?baz=bar&baz=bang&baz=bust&pat=pal&page=3'; - const newParams = { baz: 'bust', pat: 'pal' } + const newParams = { baz: 'bust', pat: 'pal' }; expect(removeParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang'], page: 3, @@ -439,7 +449,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&item.page=3'; - const newParams = { baz: 'bar' } + const newParams = { baz: 'bar' }; expect(removeParams(config, query, newParams)).toEqual({ page: 3, page_size: 15, @@ -453,7 +463,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&foo.page=3'; - const newParams = { baz: 'bar' } + const newParams = { baz: 'bar' }; expect(removeParams(config, query, newParams)).toEqual({ page: 1, page_size: 15, @@ -467,7 +477,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&item.baz=bang&item.page=3'; - const newParams = { baz: 'bar' } + const newParams = { baz: 'bar' }; expect(removeParams(config, query, newParams)).toEqual({ baz: 'bang', page: 3, @@ -482,7 +492,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&item.baz=bang&item.baz=bust&item.page=3'; - const newParams = { baz: 'bar' } + const newParams = { baz: 'bar' }; expect(removeParams(config, query, newParams)).toEqual({ baz: ['bang', 'bust'], page: 3, @@ -497,7 +507,7 @@ describe('qs (qs.js)', () => { integerFields: ['page', 'page_size'], }; const query = '?item.baz=bar&item.baz=bang&item.page=3'; - const newParams = { page: 3 } + const newParams = { page: 3 }; expect(removeParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang'], page: 1, @@ -511,8 +521,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&item.baz=bang&item.baz=bust&item.pat=pal&item.page=3'; - const newParams = { baz: 'bust', pat: 'pal' } + const query = + '?item.baz=bar&item.baz=bang&item.baz=bust&item.pat=pal&item.page=3'; + const newParams = { baz: 'bust', pat: 'pal' }; expect(removeParams(config, query, newParams)).toEqual({ baz: ['bar', 'bang'], page: 3, From f0ff5b190a43f52046375bad52cdd25f31b9e280 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 30 Jul 2019 10:32:20 -0400 Subject: [PATCH 4/8] update qs addParams/removeParams fns to take param object not string --- .../src/components/ListHeader/ListHeader.jsx | 9 ++- .../PaginatedDataList/PaginatedDataList.jsx | 6 +- awx/ui_next/src/util/qs.js | 17 ++-- awx/ui_next/src/util/qs.test.js | 81 +++++++++---------- 4 files changed, 58 insertions(+), 55 deletions(-) diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.jsx index 19b2367cf8..4974c001c9 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.jsx @@ -47,13 +47,15 @@ class ListHeader extends React.Component { handleSearch(key, value) { const { history, qsConfig } = this.props; const { search } = history.location; - this.pushHistoryState(addParams(qsConfig, search, { [key]: value })); + const oldParams = parseQueryString(qsConfig, search); + this.pushHistoryState(addParams(qsConfig, oldParams, { [key]: value })); } handleRemove(key, value) { const { history, qsConfig } = this.props; const { search } = history.location; - this.pushHistoryState(removeParams(qsConfig, search, { [key]: value })); + const oldParams = parseQueryString(qsConfig, search); + this.pushHistoryState(removeParams(qsConfig, oldParams, { [key]: value })); } handleRemoveAll() { @@ -63,8 +65,9 @@ class ListHeader extends React.Component { handleSort(key, order) { const { history, qsConfig } = this.props; const { search } = history.location; + const oldParams = parseQueryString(qsConfig, search); this.pushHistoryState( - addParams(qsConfig, search, { + addParams(qsConfig, oldParams, { order_by: order === 'ascending' ? key : `-${key}`, page: null, }) diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index 5051e545d8..593acb635a 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -33,13 +33,15 @@ class PaginatedDataList extends React.Component { handleSetPage(event, pageNumber) { const { history, qsConfig } = this.props; const { search } = history.location; - this.pushHistoryState(addParams(qsConfig, search, { page: pageNumber })); + const oldParams = parseQueryString(qsConfig, search); + this.pushHistoryState(addParams(qsConfig, oldParams, { page: pageNumber })); } handleSetPageSize(event, pageSize) { const { history, qsConfig } = this.props; const { search } = history.location; - this.pushHistoryState(addParams(qsConfig, search, { page_size: pageSize })); + const oldParams = parseQueryString(qsConfig, search); + this.pushHistoryState(addParams(qsConfig, oldParams, { page_size: pageSize })); } pushHistoryState(params) { diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index 848afa7021..05e8e4b695 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -315,16 +315,16 @@ const getRemainingNewParams = (mergedParams, newParams) => /** * Merges existing params of search string with new ones and returns the updated list of params * @param {object} qs config object (used for getting defaults, current query params etc.) - * @param {string} url query string + * @param {object} object with params from existing search * @param {object} object with new params to add * @return {object} query param object */ -export function addParams(config, searchString, newParams) { +export function addParams(config, oldParams, paramsToAdd) { const namespacedOldParams = namespaceParams( config.namespace, - parseQueryString(config, searchString) + oldParams ); - const namespacedNewParams = namespaceParams(config.namespace, newParams); + const namespacedParamsToAdd = namespaceParams(config.namespace, paramsToAdd); const namespacedDefaultParams = namespaceParams( config.namespace, config.defaultParams @@ -336,7 +336,7 @@ export function addParams(config, searchString, newParams) { ); const namespacedMergedParams = getMergedParams( namespacedOldParamsNotDefaults, - namespacedNewParams + namespacedParamsToAdd ); // return updated params. @@ -345,19 +345,18 @@ export function addParams(config, searchString, newParams) { return denamespaceParams(config.namespace, { ...getDefaultParams(namespacedOldParams, namespacedDefaultParams), ...namespacedMergedParams, - ...getRemainingNewParams(namespacedMergedParams, namespacedNewParams), + ...getRemainingNewParams(namespacedMergedParams, namespacedParamsToAdd), }); } /** * Removes params from the search string and returns the updated list of params * @param {object} qs config object (used for getting defaults, current query params etc.) - * @param {string} url query string + * @param {object} object with params from existing search * @param {object} object with new params to remove * @return {object} query param object */ -export function removeParams(config, searchString, paramsToRemove) { - const oldParams = parseQueryString(config, searchString); +export function removeParams(config, oldParams, paramsToRemove) { const paramsEntries = []; Object.entries(oldParams).forEach(([key, value]) => { if (Array.isArray(value)) { diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index bdea7840a2..636b97ce84 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -231,9 +231,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?baz=bar&page=3'; + const oldParams = { baz: 'bar', page: 3, page_size: 15 }; const newParams = { bag: 'boom' }; - expect(addParams(config, query, newParams)).toEqual({ + expect(addParams(config, oldParams, newParams)).toEqual({ baz: 'bar', bag: 'boom', page: 3, @@ -247,9 +247,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?baz=bar&baz=bang&page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { baz: 'boom' }; - expect(addParams(config, query, newParams)).toEqual({ + expect(addParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang', 'boom'], page: 3, page_size: 15, @@ -262,9 +262,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?baz=bar&baz=bang&page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { page: 5 }; - expect(addParams(config, query, newParams)).toEqual({ + expect(addParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang'], page: 5, page_size: 15, @@ -277,9 +277,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?baz=bar&baz=bang&page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { baz: 'bust', pat: 'pal' }; - expect(addParams(config, query, newParams)).toEqual({ + expect(addParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang', 'bust'], pat: 'pal', page: 3, @@ -293,9 +293,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&item.page=3'; + const oldParams = { baz: 'bar', page: 3, page_size: 15 }; const newParams = { bag: 'boom' }; - expect(addParams(config, query, newParams)).toEqual({ + expect(addParams(config, oldParams, newParams)).toEqual({ baz: 'bar', bag: 'boom', page: 3, @@ -309,9 +309,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&foo.page=3'; + const oldParams = { baz: 'bar', page: 1, page_size: 15 }; const newParams = { bag: 'boom' }; - expect(addParams(config, query, newParams)).toEqual({ + expect(addParams(config, oldParams, newParams)).toEqual({ baz: 'bar', bag: 'boom', page: 1, @@ -325,9 +325,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { baz: 'boom' }; - expect(addParams(config, query, newParams)).toEqual({ + expect(addParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang', 'boom'], page: 3, page_size: 15, @@ -340,9 +340,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { page: 5 }; - expect(addParams(config, query, newParams)).toEqual({ + expect(addParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang'], page: 5, page_size: 15, @@ -355,9 +355,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { baz: 'bust', pat: 'pal' }; - expect(addParams(config, query, newParams)).toEqual({ + expect(addParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang', 'bust'], pat: 'pal', page: 3, @@ -373,9 +373,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?baz=bar&page=3&bag=boom'; + const oldParams = { baz: 'bar', page: 3, bag: 'boom', page_size: 15 }; const newParams = { bag: 'boom' }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ baz: 'bar', page: 3, page_size: 15, @@ -388,9 +388,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?baz=bar&baz=bang&page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { baz: 'bar' }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ baz: 'bang', page: 3, page_size: 15, @@ -403,9 +403,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?baz=bar&baz=bang&baz=bust&page=3'; + const oldParams = { baz: ['bar', 'bang', 'bust'], page: 3, page_size: 15 }; const newParams = { baz: 'bar' }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bang', 'bust'], page: 3, page_size: 15, @@ -418,9 +418,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?baz=bar&baz=bang&page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { page: 3 }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang'], page: 1, page_size: 15, @@ -433,9 +433,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?baz=bar&baz=bang&baz=bust&pat=pal&page=3'; + const oldParams = { baz: ['bar', 'bang', 'bust'], pat: 'pal', page: 3, page_size: 15 }; const newParams = { baz: 'bust', pat: 'pal' }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang'], page: 3, page_size: 15, @@ -448,9 +448,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&item.page=3'; + const oldParams = { baz: 'bar', page: 3, page_size: 15 }; const newParams = { baz: 'bar' }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ page: 3, page_size: 15, }); @@ -462,9 +462,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&foo.page=3'; + const oldParams = { baz: 'bar', page: 1, page_size: 15 }; const newParams = { baz: 'bar' }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ page: 1, page_size: 15, }); @@ -476,9 +476,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { baz: 'bar' }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ baz: 'bang', page: 3, page_size: 15, @@ -491,9 +491,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&item.baz=bang&item.baz=bust&item.page=3'; + const oldParams = { baz: ['bar', 'bang', 'bust'], page: 3, page_size: 15 }; const newParams = { baz: 'bar' }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bang', 'bust'], page: 3, page_size: 15, @@ -506,9 +506,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = '?item.baz=bar&item.baz=bang&item.page=3'; + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; const newParams = { page: 3 }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang'], page: 1, page_size: 15, @@ -521,10 +521,9 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const query = - '?item.baz=bar&item.baz=bang&item.baz=bust&item.pat=pal&item.page=3'; + const oldParams = { baz: ['bar', 'bang', 'bust'], pat: 'pal', page: 3, page_size: 15 }; const newParams = { baz: 'bust', pat: 'pal' }; - expect(removeParams(config, query, newParams)).toEqual({ + expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang'], page: 3, page_size: 15, From 30253d21fcccc920d8696f8bbf7592dccae2cb59 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 30 Jul 2019 13:57:16 -0400 Subject: [PATCH 5/8] assorted ui_next search phase 1 pr feedback updates - remove unnecessary displayAll prop from ChipGroup - update notification api fn to be 2 with no boolean param - fix params passing to api functions --- awx/ui_next/SEARCH.md | 2 - .../src/api/mixins/Notifications.mixin.js | 53 ++++++++++++------- awx/ui_next/src/components/Chip/ChipGroup.jsx | 16 ++---- .../src/components/FilterTags/FilterTags.jsx | 3 +- .../PaginatedDataList/PaginatedDataList.jsx | 4 +- .../OrganizationAccess/OrganizationAccess.jsx | 2 +- .../OrganizationAccessItem.test.jsx.snap | 1 - .../OrganizationNotifications.jsx | 19 ++++--- .../OrganizationNotifications.test.jsx | 24 +++++---- awx/ui_next/src/util/qs.js | 5 +- awx/ui_next/src/util/qs.test.js | 26 +++++++-- awx/ui_next/testUtils/apiReusable.jsx | 35 ++++++++---- 12 files changed, 117 insertions(+), 73 deletions(-) diff --git a/awx/ui_next/SEARCH.md b/awx/ui_next/SEARCH.md index 0035213446..0e223b0fe8 100644 --- a/awx/ui_next/SEARCH.md +++ b/awx/ui_next/SEARCH.md @@ -108,8 +108,6 @@ Similar to the way the list grabs data based on changes to the react-router para Currently the filter tags do not display the key, though that data is available and they could very easily do so. -Some small changes were made to our custom ChipGroup component to make this possible--a new displayAll bool prop which bypasses all chip hiding based on number of chips. - ## QS Updates (and supporting duplicate keys) The logic that was updated to handle search tags can be found in the qs.js util file. diff --git a/awx/ui_next/src/api/mixins/Notifications.mixin.js b/awx/ui_next/src/api/mixins/Notifications.mixin.js index e40b7983a3..9625c29e04 100644 --- a/awx/ui_next/src/api/mixins/Notifications.mixin.js +++ b/awx/ui_next/src/api/mixins/Notifications.mixin.js @@ -14,14 +14,14 @@ const NotificationsMixin = parent => readNotificationTemplatesSuccess(id, params) { return this.http.get( `${this.baseUrl}${id}/notification_templates_success/`, - params + { params } ); } readNotificationTemplatesError(id, params) { return this.http.get( `${this.baseUrl}${id}/notification_templates_error/`, - params + { params } ); } @@ -54,43 +54,58 @@ const NotificationsMixin = parent => } /** - * This is a helper method meant to simplify setting the "on" or "off" status of + * This is a helper method meant to simplify setting the "on" status of * a related notification. * * @param[resourceId] - id of the base resource * @param[notificationId] - id of the notification * @param[notificationType] - the type of notification, options are "success" and "error" - * @param[associationState] - Boolean for associating or disassociating, options are true or false */ - // eslint-disable-next-line max-len - updateNotificationTemplateAssociation( + associateNotificationTemplate( resourceId, notificationId, - notificationType, - associationState + notificationType ) { - if (notificationType === 'success' && associationState === true) { + if (notificationType === 'success') { return this.associateNotificationTemplatesSuccess( resourceId, notificationId ); } - if (notificationType === 'success' && associationState === false) { - return this.disassociateNotificationTemplatesSuccess( - resourceId, - notificationId - ); - } - - if (notificationType === 'error' && associationState === true) { + if (notificationType === 'error') { return this.associateNotificationTemplatesError( resourceId, notificationId ); } - if (notificationType === 'error' && associationState === false) { + throw new Error( + `Unsupported notificationType for association: ${notificationType}` + ); + } + + /** + * This is a helper method meant to simplify setting the "off" status of + * a related notification. + * + * @param[resourceId] - id of the base resource + * @param[notificationId] - id of the notification + * @param[notificationType] - the type of notification, options are "success" and "error" + */ + disassociateNotificationTemplate( + resourceId, + notificationId, + notificationType + ) { + if (notificationType === 'success') { + return this.disassociateNotificationTemplatesSuccess( + resourceId, + notificationId + ); + } + + if (notificationType === 'error') { return this.disassociateNotificationTemplatesError( resourceId, notificationId @@ -98,7 +113,7 @@ const NotificationsMixin = parent => } throw new Error( - `Unsupported notificationType, associationState combination: ${notificationType}, ${associationState}` + `Unsupported notificationType for disassociation: ${notificationType}` ); } }; diff --git a/awx/ui_next/src/components/Chip/ChipGroup.jsx b/awx/ui_next/src/components/Chip/ChipGroup.jsx index 8d6d6f0593..58ba8ac7e5 100644 --- a/awx/ui_next/src/components/Chip/ChipGroup.jsx +++ b/awx/ui_next/src/components/Chip/ChipGroup.jsx @@ -1,15 +1,9 @@ import React, { useState } from 'react'; -import { number, bool } from 'prop-types'; +import { number } from 'prop-types'; import styled from 'styled-components'; import Chip from './Chip'; -const ChipGroup = ({ - children, - className, - showOverflowAfter, - displayAll, - ...props -}) => { +const ChipGroup = ({ children, className, showOverflowAfter, ...props }) => { const [isExpanded, setIsExpanded] = useState(!showOverflowAfter); const toggleIsOpen = () => setIsExpanded(!isExpanded); @@ -26,8 +20,8 @@ const ChipGroup = ({ return (
      - {displayAll ? mappedChildren : mappedChildren.slice(0, numToShow)} - {!displayAll && showOverflowToggle && ( + {!showOverflowAfter ? mappedChildren : mappedChildren.slice(0, numToShow)} + {showOverflowAfter && showOverflowToggle && ( {isExpanded ? expandedText : collapsedText} @@ -37,11 +31,9 @@ const ChipGroup = ({ }; ChipGroup.propTypes = { showOverflowAfter: number, - displayAll: bool, }; ChipGroup.defaultProps = { showOverflowAfter: null, - displayAll: false, }; export default styled(ChipGroup)` diff --git a/awx/ui_next/src/components/FilterTags/FilterTags.jsx b/awx/ui_next/src/components/FilterTags/FilterTags.jsx index ac66d97969..c9bc1cb4a7 100644 --- a/awx/ui_next/src/components/FilterTags/FilterTags.jsx +++ b/awx/ui_next/src/components/FilterTags/FilterTags.jsx @@ -40,7 +40,6 @@ const FilterTags = ({ }) => { const queryParams = parseQueryString(qsConfig, location.search); const queryParamsArr = []; - const displayAll = true; const nonDefaultParams = filterDefaultParams( Object.keys(queryParams), qsConfig @@ -59,7 +58,7 @@ const FilterTags = ({ {`${itemCount} results`} {i18n._(t`Active Filters:`)} - + {queryParamsArr.map(({ key, value }) => ( initially renders succesfully 1`] = ` >
        ', () => { .find('Switch') .at(0) .prop('onChange')(); - expect( - OrganizationsAPI.updateNotificationTemplateAssociation - ).toHaveBeenCalledWith(1, 2, 'success', true); + expect(OrganizationsAPI.associateNotificationTemplate).toHaveBeenCalledWith( + 1, + 2, + 'success' + ); await sleep(0); wrapper.update(); expect( @@ -122,9 +124,11 @@ describe('', () => { .find('Switch') .at(1) .prop('onChange')(); - expect( - OrganizationsAPI.updateNotificationTemplateAssociation - ).toHaveBeenCalledWith(1, 1, 'error', true); + expect(OrganizationsAPI.associateNotificationTemplate).toHaveBeenCalledWith( + 1, + 1, + 'error' + ); await sleep(0); wrapper.update(); expect( @@ -149,8 +153,8 @@ describe('', () => { .at(0) .prop('onChange')(); expect( - OrganizationsAPI.updateNotificationTemplateAssociation - ).toHaveBeenCalledWith(1, 1, 'success', false); + OrganizationsAPI.disassociateNotificationTemplate + ).toHaveBeenCalledWith(1, 1, 'success'); await sleep(0); wrapper.update(); expect( @@ -175,8 +179,8 @@ describe('', () => { .at(1) .prop('onChange')(); expect( - OrganizationsAPI.updateNotificationTemplateAssociation - ).toHaveBeenCalledWith(1, 2, 'error', false); + OrganizationsAPI.disassociateNotificationTemplate + ).toHaveBeenCalledWith(1, 2, 'error'); await sleep(0); wrapper.update(); expect( diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index 05e8e4b695..b14f1bc27e 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -320,10 +320,7 @@ const getRemainingNewParams = (mergedParams, newParams) => * @return {object} query param object */ export function addParams(config, oldParams, paramsToAdd) { - const namespacedOldParams = namespaceParams( - config.namespace, - oldParams - ); + const namespacedOldParams = namespaceParams(config.namespace, oldParams); const namespacedParamsToAdd = namespaceParams(config.namespace, paramsToAdd); const namespacedDefaultParams = namespaceParams( config.namespace, diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index 636b97ce84..9a9bb76515 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -403,7 +403,11 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const oldParams = { baz: ['bar', 'bang', 'bust'], page: 3, page_size: 15 }; + const oldParams = { + baz: ['bar', 'bang', 'bust'], + page: 3, + page_size: 15, + }; const newParams = { baz: 'bar' }; expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bang', 'bust'], @@ -433,7 +437,12 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const oldParams = { baz: ['bar', 'bang', 'bust'], pat: 'pal', page: 3, page_size: 15 }; + const oldParams = { + baz: ['bar', 'bang', 'bust'], + pat: 'pal', + page: 3, + page_size: 15, + }; const newParams = { baz: 'bust', pat: 'pal' }; expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang'], @@ -491,7 +500,11 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const oldParams = { baz: ['bar', 'bang', 'bust'], page: 3, page_size: 15 }; + const oldParams = { + baz: ['bar', 'bang', 'bust'], + page: 3, + page_size: 15, + }; const newParams = { baz: 'bar' }; expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bang', 'bust'], @@ -521,7 +534,12 @@ describe('qs (qs.js)', () => { defaultParams: { page: 1, page_size: 15 }, integerFields: ['page', 'page_size'], }; - const oldParams = { baz: ['bar', 'bang', 'bust'], pat: 'pal', page: 3, page_size: 15 }; + const oldParams = { + baz: ['bar', 'bang', 'bust'], + pat: 'pal', + page: 3, + page_size: 15, + }; const newParams = { baz: 'bust', pat: 'pal' }; expect(removeParams(config, oldParams, newParams)).toEqual({ baz: ['bar', 'bang'], diff --git a/awx/ui_next/testUtils/apiReusable.jsx b/awx/ui_next/testUtils/apiReusable.jsx index 11f9fca7c7..aa66015afd 100644 --- a/awx/ui_next/testUtils/apiReusable.jsx +++ b/awx/ui_next/testUtils/apiReusable.jsx @@ -8,23 +8,36 @@ export function describeNotificationMixin (Model, name) { jest.clearAllMocks(); }); - const parameters = [ - ['success', true], - ['success', false], - ['error', true], - ['error', false], - ]; - parameters.forEach(([type, state]) => { - const label = `[notificationType=${type}, associationState=${state}]`; - const testName = `updateNotificationTemplateAssociation ${label} makes expected http calls`; + const parameters = ['success', 'error']; + + parameters.forEach((type) => { + const label = `[notificationType=${type}, associationState=true`; + const testName = `associateNotificationTemplate ${label} makes expected http calls`; test(testName, async (done) => { - await ModelAPI.updateNotificationTemplateAssociation(1, 21, type, state); + await ModelAPI.associateNotificationTemplate(1, 21, type); const expectedPath = `${ModelAPI.baseUrl}1/notification_templates_${type}/`; expect(mockHttp.post).toHaveBeenCalledTimes(1); - const expectedParams = state ? { id: 21 } : { id: 21, disassociate: true }; + const expectedParams = { id: 21 }; + expect(mockHttp.post.mock.calls.pop()).toEqual([expectedPath, expectedParams]); + + done(); + }); + }); + + parameters.forEach((type) => { + const label = `[notificationType=${type}, associationState=false`; + const testName = `disassociateNotificationTemplate ${label} makes expected http calls`; + + test(testName, async (done) => { + await ModelAPI.disassociateNotificationTemplate(1, 21, type); + + const expectedPath = `${ModelAPI.baseUrl}1/notification_templates_${type}/`; + expect(mockHttp.post).toHaveBeenCalledTimes(1); + + const expectedParams = { id: 21, disassociate: true }; expect(mockHttp.post.mock.calls.pop()).toEqual([expectedPath, expectedParams]); done(); From 0276a37e8dc6f27786f213098aaf13e28ecbc259 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Wed, 31 Jul 2019 10:09:11 -0400 Subject: [PATCH 6/8] small updates to qs syntax based on feedback --- awx/ui_next/src/util/qs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index b14f1bc27e..8e19afc978 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -47,7 +47,7 @@ const denamespaceParams = (namespace, params = {}) => { denamespaced[key.substr(namespace.length + 1)] = params[key]; }); - return denamespaced || {}; + return denamespaced; }; /** From 2e777368bf49d3442c3c944b1ec2e87c5905498c Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 1 Aug 2019 12:13:34 -0400 Subject: [PATCH 7/8] Update SEARCH.md --- awx/ui_next/SEARCH.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/awx/ui_next/SEARCH.md b/awx/ui_next/SEARCH.md index 0e223b0fe8..5e3e0869da 100644 --- a/awx/ui_next/SEARCH.md +++ b/awx/ui_next/SEARCH.md @@ -35,6 +35,7 @@ - make default params only accept page, page_size and order_by - support custom order_by being typed in the url bar - fix up which keys are displayed in the various lists (note this will also require non-string widgetry to the right of the search key dropdown, for integers, dates, etc.) +- fix any spacing issues like collision with action buttons and overall width of the search bar ## Lists affected in 3.6 timeframe @@ -250,4 +251,4 @@ Smart search will be able to craft the tag through various states. Note that th - isnull: Check whether the given field or related object is null; expects a boolean value. - in: Check whether the given field's value is present in the list provided; expects a list of items. - PHASE 5: The value. Based on options, we can give hints or validation based on type of value (like number fields don't accept "foo" or whatever) \ No newline at end of file + PHASE 5: The value. Based on options, we can give hints or validation based on type of value (like number fields don't accept "foo" or whatever) From c0dcba91f5886389e7ab5d03b3cf740e7cf031e9 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 1 Aug 2019 12:14:31 -0400 Subject: [PATCH 8/8] Update SEARCH.md --- awx/ui_next/SEARCH.md | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/awx/ui_next/SEARCH.md b/awx/ui_next/SEARCH.md index 5e3e0869da..16088827cc 100644 --- a/awx/ui_next/SEARCH.md +++ b/awx/ui_next/SEARCH.md @@ -21,15 +21,12 @@ - DONE make filter and list header tests - DONE change api paramsSerializer to handle duplicate key stuff - DONE update qs update function to be smaller, simple param functions, as opposed to one big one with a lot of params - -## TODO for this PR - -- add search filter removal test for qs. -- bug: remove button for search tags of duplicate keys are broken, fix that +- DONE add search filter removal test for qs. +- DONE remove button for search tags of duplicate keys are broken, fix that ## TODO later on in 3.6: stuff to be finished for search iteration 1 (I'll card up an issue to tackle this. I plan on doing this after I finished the awx project branch work) -- currently handleSearch in Search.jsx always appends the __icontains post-fix to make the filtering ux expected work right. Once we start adding number-based params we will won't to change this behavior. +- currently handleSearch in Search.jsx always appends the `__icontains` post-fix to make the filtering ux expected work right. Once we start adding number-based params we will won't to change this behavior. - utilize new defaultSearchKey prop instead of relying on sort key - make access have username as the default key? - make default params only accept page, page_size and order_by