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.

Sign up for GitHub

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

Jump to bottom

Make alerts list sortable by name and status #93426

Merged
merged 5 commits into from
Mar 8, 2021
Merged

Make alerts list sortable by name and status #93426

merged 5 commits into from
Mar 8, 2021

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Mar 3, 2021

Fixes #60584.

This PR allows sorting the alerts list by name and status.

*PR done as a fix-it task I've assigned to myself.

@mikecote mikecote added release_note:enhancement Feature:Alerting v8.0.0 UX Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.13.0 labels Mar 3, 2021
@mikecote mikecote self-assigned this Mar 3, 2021
@mikecote mikecote changed the title Initial commit Make alerts list sortable by name and status. Mar 3, 2021
@mikecote mikecote changed the title Make alerts list sortable by name and status. Make alerts list sortable by name and status Mar 3, 2021
mikecote
mikecote commented Mar 3, 2021
View reviewed changes
x-pack/plugins/alerts/server/alerts_client/lib/map_sort_field.ts Outdated
Comment on lines 8 to 14
const sortFieldMap: Record<string, string> = {
name: 'name.keyword',
};

export function mapSortField(field?: string): string | undefined {
return field ? sortFieldMap[field] || field : undefined;
}
Copy link
Contributor Author

@mikecote mikecote Mar 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

To de-couple our mappings from HTTP API parameters, I decided to create this function, so people don't have to know they need to pass name.keyword when they want to sort by name. We still need text mapping for free-text searching.

mikecote
mikecote commented Mar 3, 2021
View reviewed changes
x-pack/plugins/triggers_actions_ui/public/types.ts
@@ -191,6 +196,7 @@ export type AlertUpdates = Omit<Alert, 'id' | 'executionStatus'>;
export interface AlertTableItem extends Alert {
alertType: AlertType['name'];
tagsText: string;
actionsCount: number;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To work with the EuiTableSortingType interface, I had to add actionsCount here. I also renamed it from actionsText to avoid confusion because it contains a number value.

@mikecote mikecote marked this pull request as ready for review March 3, 2021 17:03
@mikecote mikecote requested a review from a team as a code owner March 3, 2021 17:03
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@mikecote mikecote added the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 3, 2021
ymao1
ymao1 approved these changes Mar 4, 2021
View reviewed changes
Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@mikecote
Copy link
Contributor Author

mikecote commented Mar 4, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
triggersActionsUi 1.5MB 1.5MB +263.0B

History

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

cc @mikecote

pmuellr
pmuellr approved these changes Mar 8, 2021
View reviewed changes
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM, works as expected

@mikecote mikecote merged commit edb1c4b into elastic:master Mar 8, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Mar 8, 2021
@mikecote @kibanamachine
Make alerts list sortable by name and status (elastic#93426)
431057c 
* Initial commit

* Update docs

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine mentioned this pull request Mar 8, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

✅ 7.x / #93959

Successful backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Mar 8, 2021
@kibanamachine @mikecote
Make alerts list sortable by name and status (#93426) (#93959)
c4011f8 
* Initial commit

* Update docs

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmuellr pmuellr pmuellr approved these changes

@ymao1 ymao1 ymao1 approved these changes

Assignees

@mikecote mikecote

Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Alerting release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) UX v7.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow sorting the alert list by name
5 participants
@mikecote @elasticmachine @kibanamachine @pmuellr @ymao1