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] Align case avatars #140227

Merged
merged 24 commits into from
Sep 14, 2022

Conversation

jonathan-buttner
Copy link
Contributor

@jonathan-buttner jonathan-buttner commented Sep 7, 2022

This PR refactors the case avatars to use the same style designed for the case assignments feature. This PR does not adjust the reporters or participants section.

It also does not address that the backend is not storing the profile uid when a user is stored. This will be addressed is a separate PR.

This PR is stacked on top of the feature branch PR here: #140208

Example
uniform.avatars.mov

cnasikas and others added 15 commits August 24, 2022 11:26
…38108)

* 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 (elastic#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]>
* 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]>
* Init

* Render users

* Assign yourself

* Add tests

* Fix tests

* PR feedback
* 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
@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 Sep 7, 2022
@jonathan-buttner jonathan-buttner marked this pull request as ready for review September 12, 2022 16:36
@jonathan-buttner jonathan-buttner requested review from a team as code owners September 12, 2022 16:36
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

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

Security solution changes!

@@ -126,6 +126,7 @@ export interface ElasticUser {
readonly email?: string | null;
readonly fullName?: string | null;
readonly username?: string | null;
readonly profileUid?: string | null;
Copy link
Member

Choose a reason for hiding this comment

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

nit: Is it possible to use SnakeToCamelCase<User> to define the ElasticUser interface?

aria-label={i18n.SEND_EMAIL_ARIA(fullName ? fullName : username ?? '')}
isDisabled={isEmpty(email)}
aria-label={i18n.SEND_EMAIL_ARIA(
userInfo.user?.full_name ? userInfo.user?.full_name : userInfo.user?.username ?? ''
Copy link
Member

Choose a reason for hiding this comment

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

If I remember correctly we have a utility function to return us a valid name, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's one slight difference, the utility function will do full_name -> email -> username -> unknown. This logic does full_name -> username -> ''. Using the utility function should be fine though right?

);
});
appMockRender.render(<UserActions {...props} />);
// there are two because one is for the
Copy link
Member

Choose a reason for hiding this comment

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

nit: for the? 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea what I was going for here haha, will remove.

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.

Code LGTM! Tested locally without any issues

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
cases 484 489 +5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cases 367.5KB 368.8KB +1.4KB

History

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

@jonathan-buttner jonathan-buttner merged commit a58778f into elastic:main Sep 14, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 14, 2022
@jonathan-buttner jonathan-buttner deleted the cases-avatars branch September 14, 2022 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting 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.

6 participants