Bugfix/two factor notification

[camilasan]

Before:

After:

@nimishavijay We are always using 'Mark as Read' in all notifications. May I suggest to simply always use what comes with the notification, like in this case it should be 'Cancel'. That was one of the reasons that we had this bug.

[mgallien]

some small comments

[claucambra]

The activitylistmodel test is failing, probably needs some small changes

[nimishavijay]

Good catch!
You are right, we should not use "Mark as read" for everything and "Approve" and "Cancel" work here :)

Is this wording provided by the API itself or it is something we have to explicitly decide every time?

[camilasan]

Is this wording provided by the API itself or it is something we have to explicitly decide every time?

It is provided by the api.

[camilasan]

we should not use "Mark as read" for everything and "Approve" and "Cancel" work here :)

that is the thing: we shouldn't be checking for these texts. We should just display what is in the notification.

I will remove the 'Mark as Read' and just use what comes with the API.

However if nothing is provided we will use 'Dismiss', is that ok @nimishavijay?

[nimishavijay]

Sounds good! 🚀

[camilasan]

The activitylistmodel test is failing, probably needs some small changes

Yes, I am trying to understand why it is failing which is making me afraid that I might be breaking something else while fixing this issue...

[jancborchardt]

Btw @camilasan, every notification with actions also needs a clear primary action button. In this case that would be "Approve". :)

[mgallien]

Btw @camilasan, every notification with actions also needs a clear primary action button. In this case that would be "Approve". :)

going to merge it as is and create a follow up issue as we need this to fix the bug that prevent people from using 2FA with Nextcloud notifications

thanks for the feedback 😄

[mgallien]

/backport to stable-3.5

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

28.6% Coverage
0.0% Duplication

[nextcloud-desktop-bot]

AppImage file: Nextcloud-PR-4518-290851c33a996fd4a9d89c60b9f5be5ae0af627e-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.