[Reporting] Make CSV authentication failure a warning #126358
Conversation
jloleysens
added
(Deprecated) Feature:Reporting
|
Pinging @elastic/kibana-reporting-services (Team:Reporting Services) |
|
Pinging @elastic/kibana-app-services (Team:AppServicesUx) |
|
|
||
| import { i18n } from '@kbn/i18n'; | ||
|
|
||
| export const i18nTexts = { |
There was a problem hiding this comment.
This is very good clean-up! 👍
| throw new AuthenticationExpiredError(); | ||
| reportingError = new AuthenticationExpiredError(); | ||
| warnings.push( | ||
| totalRecords > 0 ? i18nTexts.partialResultsMessage : i18nTexts.noResultsMessage |
There was a problem hiding this comment.
I'm not sure we need to have an extra message to state there were no results due to the authentication error. The partialResultsMessage seems suitable to me for both cases. It risks us showing some false positives, since there could be other explanations for the CSV not having any results.
There was a problem hiding this comment.
I gave this more thought, the primary issue for me is that if a user downloads a CSV that we say has partial results but then contains no results it could create some confusion.
However, this will only ever occur when a user sends a report request that either contains an expired auth header (unlikely) or that expires before it gets back the first set of results (also unlikely, there is probably something bigger wrong).
Happy to remove the extra i18n for now.
|
@elasticmachine merge upstream |
|
@tsullivan , I think I've addressed your feedback. Would you mind taking another look? |
💚 Build SucceededMetrics [docs]Module Count
Page load bundle
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
With help from @tsullivan, here are some suggestions:
Report contains errors
This report contains partial CSV results because the authentication token expired. Export a smaller amount of data or increase the timeout of the authentication token.
CSV completed with issues
Download the report now, or go to Stack Management > Reporting for more on the issues.
Notes:
- Removed the "This report completed with issues" because it repeats the title
- Removed "Analytics and Insights" because it is not clickable in the UI.
- Other suggestions:
- Download the report now, or go to Stack Management > Reporting for details.
|
🤦🏼♂️ i'm sorry, I accidentally merged this. I'll open up a follow up PR for the copy updates! |
Summary
Partially addresses #125139
When generating a CSV we want to give users the ability to download the data that we were able to export before hitting the auth error.
Additionally, this PR adds a new "warning" notification so that reports that complete with warnings are shown as such by the notification.
Screenshots
Checklist