-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Alerting] Add alert.updatedAt
field to represent date of last user edit
#83784
Conversation
…ing/updated-at
…ing/updated-at
…ing/updated-at
…ing/updated-at
…ing/updated-at
💚 Build SucceededMetrics [docs]Saved Objects .kibana field count
History
To update your PR or re-run it, just comment with: |
alert.updatedAt
field to represent date of last user edit
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the cypress fixes!
So the only change from the original PR is the additional security cypress test stuff? Does that mean the original PR was reverted? I would have expected a reference to the reverted commit somehow in that PR, if so. But didn't see one.
Seems like we need to get that fixed, or it'll happen again the next time we update the alert SO mappings ... fixed as in "those tests need to run if anything in alerting changes (and maybe actions and task manager and event log ...)". Not sure who we talk to about that tho ... |
@pmuellr Yes, I reverted the original PR. Revert commit on master here: 6a2c415 I have talked to @dhurley14 and he believes there is an issue open already for updating the paths that will trigger the Security Solutions Cypress tests. If he's unable to find it, then I will open an issue for it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Issue created here: #83851 |
… edit (elastic#83784) * Adding alert.updatedAt field that only updates on user edit * Updating unit tests * Functional tests * Updating alert attributes excluded from AAD * Fixing test * PR comments * Unskipping tests and updating es archiver data
Resolves #83578
Summary
Re-do of this PR: #83578
The original PR did not touch any code that triggered the Security Solutions cypress test suite so those tests were not run on the PR. Once merged into master, the cypress tests failed because they were loading alert saved objects from the esarchiver without the
updatedAt
field (something which would be taken care of by the migration)This is the commit that updates the esarchive data for the security solutions tests (and unskips the test suites) c150217
This is the output of the cypress tests on this branch https://github.com/elastic/kibana/runs/1425449442
Checklist
Delete any items that are not applicable to this PR.