Differential D18479
Add a setting for 'mark all read' notifications Authored by epriestley on Aug 25 2017, 9:11 PM. Tags None Referenced Files
Subscribers
Details
Adds a setting to toggle the prompt when marking all notifications read. If users opt into the toggle, we no longer show a dialog with a verification button first, and instead just mark them all read (as of time of render) and reload the page. This does not update the button on /notifications/ which I'll follow up in next diff. Save setting, spawn some notifications from a second account. Use Mark All Read from dropdown. See page reload and count vanish.
Diff Detail
Event TimelineComment Actions This approach seemed cleaner as simpler than my last over-engineered attempt. It just reloads the page after it sends the request off. Is this a decent approach? I'll follow up with adding a checkbox directly on the dialog for the setting. As for the button on /notifications/ I'm having some issue finding a clean solution since search results always want a PHUIButtonView and not a form. I'll think more about the best approach there, but I don't think it's any priority. Comment Actions I believe this allows an attacker to clear a user's notifications with a CSRF attack, because it executes a write action without calling validateCSRF() or any method which calls that for you (like isFormPost() or isDialogFormPost()). I'll just write this piece, since we don't have any similar behavior anywhere else and it isn't something you can really guess or figure out. |