Alert improvements #480
Alert improvements #480
Conversation
Co-Authored-By: parostatkiem <[email protected]>
…lert-improvements # Conflicts: # client/src/luigi-client.js # docs/luigi-client-api.md
parostatkiem-zz
requested review from
antiheld,
dariadomagala-sap,
hardl,
jesusreal,
JohannesDoberer,
kazydek,
klaudiagrz,
kwiatekus and
marynaKhromova
as code owners
There was a problem hiding this comment.
Please take a look at the code-related comments plus this one:
If multiple alerts are shown coming from older clients (without IDs) and you close one of them, then all of them are closed at once. We need to accept alerts without IDs (backwards-compatibility) but we need to distinguish them on the core side so that they can by closed one by one.
core/src/Alerts.html
Outdated
| settings: Object.assign(this.get().settings, { | ||
| text: processedData.sanitizedText | ||
| }), | ||
| alertQueue: processedAlerts, | ||
| dataSanitized: true |
There was a problem hiding this comment.
We need the 'dataSanitized' flag per alert - not globally. Otherwise when a new alert comes it will be displayed unprocessed and unsanitized for a moment (if malicious client sends us an alert with 'sanitizedText' property already set and containing malicious code) which is enough for injected code to get executed.
| } | ||
|
|
||
| if (settings.closeAfter) { | ||
| setTimeout(() => { |
There was a problem hiding this comment.
If the alert gets closed manually before the timeout an error is shown in the console: "An unexisting alert has been dismissed.". There is nothing wrong with closing an alert manually so there should not be any error shown.
There was a problem hiding this comment.
I'll investigate it.
parostatkiem-zz
force-pushed
the
alert-improvements
branch
from
04401d0 to
06b08a6
Compare
client/src/luigi-client.js
Outdated
| @@ -1,6 +1,7 @@ | |||
| var crypto = require("crypto"); | |||
There was a problem hiding this comment.
we have singleQuote set to true in the prettier's options. somehow it didn't work here - please check if your hooks are installed properly and run prettier again.
Co-Authored-By: parostatkiem <[email protected]>
…luigi into alert-improvements
parostatkiem-zz
force-pushed
the
alert-improvements
branch
from
19e6a29 to
5c5f2b2
Compare
* Assign IDs; add ability to remove alerts * Add 'closeAfter' mechanizm * Update docu * Correct linter errors * e2e tests * Update TTL mechanizm to respect the queue; tiny update of Alert usage from Core * Fix error with closing already closed alerts * Make 'dataSanitized' flag individual per alert * Generate ID on Core side in case it hasn't came from Client * Fix bug with Example app disabling Type input field * formatting * Pass alert id to link IDs * Fix failing test
Description
Changes proposed in this pull request:
closeAfterproperty can be provided via Luigi API to (surprise!) close the Alert after the desired timeRelated issue(s)