Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ResponseOps][Cases] Filter by assignees #139441

Conversation

jonathan-buttner
Copy link
Contributor

@jonathan-buttner jonathan-buttner commented Aug 24, 2022

This PR adds filtering for cases on the cases list page using assignees.

Notable other changes:

  • Only single selection is allowed, we can improve on this in the future
  • When searching in the filtering selectable and assignees selectable the current user is brought to the front
  • Added a new backend route _find_assignees that retrieves all assigned users and also uses the suggestions api to support searching
    • We are not using the hints feature of the elasticsearch api so it is possible that the search could return users that aren't actually assigned to the case, these will be filtered out by the _find_assignees API
Example of filtering
filtering.assignees.mov

@jonathan-buttner jonathan-buttner added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.5.0 labels Aug 24, 2022
@jonathan-buttner jonathan-buttner marked this pull request as ready for review August 31, 2022 21:30
@jonathan-buttner jonathan-buttner requested review from a team as code owners August 31, 2022 21:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops-cases (Feature:Cases)

Copy link
Contributor

@watson watson left a comment

Choose a reason for hiding this comment

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

The Kibana Platform Security changes LGTM 👍 I haven't looked into the failing tests though, but wanted to unblock this from our end for now

@legrego legrego requested a review from kc13greiner September 1, 2022 11:48
size: schema.maybe(schema.number()),
}),
},
handler: async ({ request, response, context }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this only returning users who have a case assigned to them? I think it can be useful to allow a search on a user who has nothing currently assigned to them. Here for example I wanted to check if I had any cases assigned to the user who I am currently logged in as:

image

GitHub seems to let you run a search on a user who has no issues / PRs assigned to them in the current repo.

Copy link
Member

@cnasikas cnasikas Sep 5, 2022

Choose a reason for hiding this comment

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

@arisonl @shanisagiv1 @MichaelMarcialis What is your opinion on this? In this PR we search only against users that are at least assigned to a case and not to all available users (assigned or not). Do you think we should search across all users (assigned or not) and show no cases if they are not assigned to any?

Copy link
Member

Choose a reason for hiding this comment

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

We discussed it offline and we decided to show all users assigned or not.

});

export const NO_MATCHING_USERS = i18n.translate('xpack.cases.userProfiles.noMatchingUsers', {
defaultMessage: 'No matching users with required access.',
Copy link
Contributor

Choose a reason for hiding this comment

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

The with required access can be misleading here as it is displayed when running a search for a user with the correct privileges, but who doesn't currently have any cases assigned to them (see also earlier comment about behavior when searching for a user with no cases assigned).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, after chatting with Aris and Michael we're going to display all users regardless of whether they've actually been assigned to a case or not.

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and overall looks good. Just wonder if the behavior when no users match the search needs changing, to return users who aren't currently assigned to cases? But maybe that is difficult given the current APIs?

@cnasikas cnasikas mentioned this pull request Sep 6, 2022
7 tasks
Copy link
Member

@MadameSheema MadameSheema left a comment

Choose a reason for hiding this comment

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

security-engineering-productivity LGTM

const [search, setSearch] = useState(initial.search);
const [selectedTags, setSelectedTags] = useState(initial.tags);
const [selectedOwner, setSelectedOwner] = useState(initial.owner);
const [selectedAssignees, setSelectedAssignees] = useState<UserProfileWithAvatar[]>([]);
const fetchAssignees = useRef<() => void>();
Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to remove the fetchAssignees ref. It is not needed. queryClient.refetchQueries is sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, thank you.


const refetch = useCallback(() => {
fetchTags();
fetchReporters();
}, [fetchReporters, fetchTags]);
queryClient.refetchQueries([USER_PROFILES_CACHE_KEY, USER_PROFILES_SUGGEST_CACHE_KEY]);
Copy link
Member

Choose a reason for hiding this comment

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

I think the cache key should be [USER_PROFILES_CACHE_KEY, USER_PROFILES_BULK_GET_CACHE_KEY]

@@ -16,6 +16,8 @@ import { AssigneeWithProfile } from '../../user_profiles/types';

jest.mock('../../../containers/user_profiles/api');

jest.useFakeTimers();
Copy link
Member

Choose a reason for hiding this comment

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

We should remove it if not needed.

@@ -0,0 +1,22 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add a test for this component too?

@@ -31,4 +32,87 @@ describe('sort', () => {
]);
});
});

describe('bringCurrentUserToFrontAndSort', () => {
const unsortedProfiles = [...userProfiles].reverse();
Copy link
Member

Choose a reason for hiding this comment

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

😄

import { Assignee, AssigneeWithProfile } from '../../components/user_profiles/types';

interface PartitionedAssignees {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add a basic test for the hook?

onClick={togglePopover}
isLoading={isLoadingData}
isSelected={isPopoverOpen}
numFilters={userProfiles?.length ?? 0}
Copy link
Contributor

Choose a reason for hiding this comment

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

The count shown when there are no active filters is a bit misleading I think - it currently seems to show the number of users that will be displayed in the popover on opening. Do you have access to the total number of user profiles? If not, can that badge be hidden if there are no active filters?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We chatted about this offline and we're going to hide this because we can't get the total number of profiles easily right now.

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Latest changes LGTM

@jonathan-buttner jonathan-buttner merged commit 8251340 into elastic:cases-user-assignment Sep 7, 2022
@jonathan-buttner jonathan-buttner deleted the cases-filter-by-assignees branch September 7, 2022 16:01
@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 7, 2022

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #1 / EditableTitle renders
  • [job] [logs] Jest Tests #1 / EditConnector displays the callout message when none is selected
  • [job] [logs] Jest Tests #1 / HeaderPage it renders
  • [job] [logs] FTR Configs #45 / machine learning - anomaly detection anomaly explorer with farequote based multi metric job Anomaly Swim Lane as embeddable attaches swim lane embeddable to a case
  • [job] [logs] FTR Configs #45 / machine learning - anomaly detection anomaly explorer with farequote based multi metric job Anomaly Swim Lane as embeddable attaches swim lane embeddable to a case

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [8103bd3]

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

jonathan-buttner added a commit that referenced this pull request Sep 12, 2022
* [ResponseOps][Cases] Assign users on cases sidebar section (#138108)

* Add reusable user profile selector component

* Refactoring services, auth

* Adding suggest api and tests

* Move to package and add examples

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Working integration tests

* Switching suggest api tags

* Adding assignees field

* Adding tests for size and owner

* Add server side example

* CI Fixes

* Adding assignee integration tests

* fix tests

* Addd tests

* Starting user actions changes

* Adding api tag tests

* Addressing feedback

* Using lodash for array comparison logic and tests

* Fixing type error

* Create suggest query

* Adding assignees user action

* [ResponseOps][Cases] Refactoring client args and authentication (#137345)

* Refactoring services, auth

* Fixing type errors

* Adding assignees migration and tests

* Fixing types and added more tests

* Fixing cypress test

* Fixing test

* Add tests

* Add security as dependency and fix types

* Add bulk get profiles query

* Rename folder

* Addressed suggestions from code review

* Fix types

* Adding migration for assignees field and tests

* Adding comments and a few more tests

* Updating comments and spelling

* Revert security solution optional

* PR feedback

* Updated user avatar component

* Reduce size

* Make security required

* Fix tests

* Addressing feedback

* Do not retry suggestions

* Assign users to a case

* Restructure components

* Move assignees to case view

* Show assigned users

* Refactoring bulk get and display name

* Adding tests for user tooltip

* Adding tests

* Hovering and tests

* Fixing errors

* Some pr clean up

* Fixing tests and adding more

* Adding functional tests

* Fixing jest test failure

* Passing in current user profile

* Refactoring assignment with useEffect

* Fixing icon alignment and removal render bug

* Fixing type errors

* Fixing test errors

* Adding bulk get privs and tests

* Fixing popover tests

* Handling unknown users

* Adding tests for unknown users

* Adding wait for popover calls

* Addressing design feedback

* Addressing remaining feedback

* Refactoring css and name prop

* Refactoring popover

* Refactoring search component

* Addressing some feedback

* Adjusting sorting

* Fixing tests

* Fixing type error

* Fixing test error

* Fixing test errors

* Removing display name

Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>

* [ResponseOps][Cases] Add assignee user actions (#139392)

* Adding user actions for assignees

* Fixing merge issues

* Fixing test mock errors

* Fixing display name errors and themselves

* Fixing test for time being

* Addressing feedback

* Addressing comma and uniq feedback

* Using core getUserDisplayName

* Fixing import and removing flickering

* Fixing tests

Co-authored-by: Kibana Machine <[email protected]>

* Detect when user is typing

* Use select to tranform data

* [Cases] Assign users when creating a case (#139754)

* Init

* Render users

* Assign yourself

* Add tests

* Fix tests

* PR feedback

* [ResponseOps][Cases] Filter by assignees (#139441)

* Starting the filtering

* Rough draft working for assignees filtering

* Adding integration tests for new route

* Starting to write tests

* Fixing tests

* Cleaning up more tests

* Removing duplicate call for current user

* Fixing type errors and tests

* Adding tests for filtering

* Adding rbac tests

* Fixing translations

* Fixing api integration tests

* Fixing severity tests

* Really fixing arrays equal

* Fixing ml tests and refactoring find assignees

* Fixing cypress tests

* Fixing types

* Fix tests

* Addressing first round of feedback

* Reverting the recent cases changes

* Fixing tests

* Fixing more tests and types

* Allowing multi select

* Fixing attachment framework issue

* Addressing feedback

* Fixing type error

* Fixing tests

* Sort users and improve loading

* Fix ml security dependecies

* Fix permissions when fetching suggestions

* Fixing read permissions and suggest api

* Hiding assignee delete icon

* Hiding the assign yourself text when undefined

* Create page fixes

Co-authored-by: Christos Nasikas <[email protected]>
Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Thom Heymann <[email protected]>
jonathan-buttner added a commit that referenced this pull request Sep 14, 2022
* [ResponseOps][Cases] Assign users on cases sidebar section (#138108)

* Add reusable user profile selector component

* Refactoring services, auth

* Adding suggest api and tests

* Move to package and add examples

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Working integration tests

* Switching suggest api tags

* Adding assignees field

* Adding tests for size and owner

* Add server side example

* CI Fixes

* Adding assignee integration tests

* fix tests

* Addd tests

* Starting user actions changes

* Adding api tag tests

* Addressing feedback

* Using lodash for array comparison logic and tests

* Fixing type error

* Create suggest query

* Adding assignees user action

* [ResponseOps][Cases] Refactoring client args and authentication (#137345)

* Refactoring services, auth

* Fixing type errors

* Adding assignees migration and tests

* Fixing types and added more tests

* Fixing cypress test

* Fixing test

* Add tests

* Add security as dependency and fix types

* Add bulk get profiles query

* Rename folder

* Addressed suggestions from code review

* Fix types

* Adding migration for assignees field and tests

* Adding comments and a few more tests

* Updating comments and spelling

* Revert security solution optional

* PR feedback

* Updated user avatar component

* Reduce size

* Make security required

* Fix tests

* Addressing feedback

* Do not retry suggestions

* Assign users to a case

* Restructure components

* Move assignees to case view

* Show assigned users

* Refactoring bulk get and display name

* Adding tests for user tooltip

* Adding tests

* Hovering and tests

* Fixing errors

* Some pr clean up

* Fixing tests and adding more

* Adding functional tests

* Fixing jest test failure

* Passing in current user profile

* Refactoring assignment with useEffect

* Fixing icon alignment and removal render bug

* Fixing type errors

* Fixing test errors

* Adding bulk get privs and tests

* Fixing popover tests

* Handling unknown users

* Adding tests for unknown users

* Adding wait for popover calls

* Addressing design feedback

* Addressing remaining feedback

* Refactoring css and name prop

* Refactoring popover

* Refactoring search component

* Addressing some feedback

* Adjusting sorting

* Fixing tests

* Fixing type error

* Fixing test error

* Fixing test errors

* Removing display name

Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>

* [ResponseOps][Cases] Add assignee user actions (#139392)

* Adding user actions for assignees

* Fixing merge issues

* Fixing test mock errors

* Fixing display name errors and themselves

* Fixing test for time being

* Addressing feedback

* Addressing comma and uniq feedback

* Using core getUserDisplayName

* Fixing import and removing flickering

* Fixing tests

Co-authored-by: Kibana Machine <[email protected]>

* Detect when user is typing

* Use select to tranform data

* [Cases] Assign users when creating a case (#139754)

* Init

* Render users

* Assign yourself

* Add tests

* Fix tests

* PR feedback

* [ResponseOps][Cases] Filter by assignees (#139441)

* Starting the filtering

* Rough draft working for assignees filtering

* Adding integration tests for new route

* Starting to write tests

* Fixing tests

* Cleaning up more tests

* Removing duplicate call for current user

* Fixing type errors and tests

* Adding tests for filtering

* Adding rbac tests

* Fixing translations

* Fixing api integration tests

* Fixing severity tests

* Really fixing arrays equal

* Fixing ml tests and refactoring find assignees

* Fixing cypress tests

* Fixing types

* Fix tests

* Addressing first round of feedback

* Reverting the recent cases changes

* Fixing tests

* Fixing more tests and types

* Allowing multi select

* Fixing attachment framework issue

* Addressing feedback

* Fixing type error

* Fixing tests

* Refactoring components

* Refactoring the users lists

* Switching all user action avatars

* Fixing merge issues

* Fixing types

* Addressing feedback

* Fixing type errors

Co-authored-by: Christos Nasikas <[email protected]>
Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Thom Heymann <[email protected]>
cnasikas added a commit that referenced this pull request Sep 14, 2022
* [ResponseOps][Cases] Assign users on cases sidebar section (#138108)

* Add reusable user profile selector component

* Refactoring services, auth

* Adding suggest api and tests

* Move to package and add examples

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Working integration tests

* Switching suggest api tags

* Adding assignees field

* Adding tests for size and owner

* Add server side example

* CI Fixes

* Adding assignee integration tests

* fix tests

* Addd tests

* Starting user actions changes

* Adding api tag tests

* Addressing feedback

* Using lodash for array comparison logic and tests

* Fixing type error

* Create suggest query

* Adding assignees user action

* [ResponseOps][Cases] Refactoring client args and authentication (#137345)

* Refactoring services, auth

* Fixing type errors

* Adding assignees migration and tests

* Fixing types and added more tests

* Fixing cypress test

* Fixing test

* Add tests

* Add security as dependency and fix types

* Add bulk get profiles query

* Rename folder

* Addressed suggestions from code review

* Fix types

* Adding migration for assignees field and tests

* Adding comments and a few more tests

* Updating comments and spelling

* Revert security solution optional

* PR feedback

* Updated user avatar component

* Reduce size

* Make security required

* Fix tests

* Addressing feedback

* Do not retry suggestions

* Assign users to a case

* Restructure components

* Move assignees to case view

* Show assigned users

* Refactoring bulk get and display name

* Adding tests for user tooltip

* Adding tests

* Hovering and tests

* Fixing errors

* Some pr clean up

* Fixing tests and adding more

* Adding functional tests

* Fixing jest test failure

* Passing in current user profile

* Refactoring assignment with useEffect

* Fixing icon alignment and removal render bug

* Fixing type errors

* Fixing test errors

* Adding bulk get privs and tests

* Fixing popover tests

* Handling unknown users

* Adding tests for unknown users

* Adding wait for popover calls

* Addressing design feedback

* Addressing remaining feedback

* Refactoring css and name prop

* Refactoring popover

* Refactoring search component

* Addressing some feedback

* Adjusting sorting

* Fixing tests

* Fixing type error

* Fixing test error

* Fixing test errors

* Removing display name

Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>

* [ResponseOps][Cases] Add assignee user actions (#139392)

* Adding user actions for assignees

* Fixing merge issues

* Fixing test mock errors

* Fixing display name errors and themselves

* Fixing test for time being

* Addressing feedback

* Addressing comma and uniq feedback

* Using core getUserDisplayName

* Fixing import and removing flickering

* Fixing tests

Co-authored-by: Kibana Machine <[email protected]>

* Detect when user is typing

* Use select to tranform data

* [Cases] Assign users when creating a case (#139754)

* Init

* Render users

* Assign yourself

* Add tests

* Fix tests

* PR feedback

* [ResponseOps][Cases] Filter by assignees (#139441)

* Starting the filtering

* Rough draft working for assignees filtering

* Adding integration tests for new route

* Starting to write tests

* Fixing tests

* Cleaning up more tests

* Removing duplicate call for current user

* Fixing type errors and tests

* Adding tests for filtering

* Adding rbac tests

* Fixing translations

* Fixing api integration tests

* Fixing severity tests

* Really fixing arrays equal

* Fixing ml tests and refactoring find assignees

* Fixing cypress tests

* Fixing types

* Fix tests

* Addressing first round of feedback

* Reverting the recent cases changes

* Fixing tests

* Fixing more tests and types

* Allowing multi select

* Fixing attachment framework issue

* Addressing feedback

* Fixing type error

* Fixing tests

* Sort users and improve loading

* Fix ml security dependecies

* Fix permissions when fetching suggestions

* Fixing read permissions and suggest api

* Hiding assignee delete icon

* Hiding the assign yourself text when undefined

* Show licensing callout to the all cases page

* Hide assignees column

* Hide assignees & connectors column

* Renames

* Check licensing when assigning users to a case

* Hide assinee from create case page

* Hide assignee filtering

* Check license when filtering by assignees

* Add UI tests & fixes

* Add integration tests

* Fix tests

* Fix i18n

* Revert core changes

* PR feedback and fix tests

* PR feedback

Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Thom Heymann <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Cases Cases feature release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.