wordpress-mobile / wordpress-mobile/WordPress-Android
Push notification: review consistency of comment actions and comment body availability.
Nobody has claimed this yet.
- Dominant language
- Kotlin
- Stars
- 3.2k
- Forks
- 1.4k
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 69
Description
This is a reminder issue after the #17026 has been addressed with the containment fix in #17044. We should go deeper into the investigation, but wanted to report here what we were discussing so it's tracked meanwhile for all of us.
The received notification does not show the comment body (h/t @thehenrybyrd for noting and reporting 🙇 )
so the user does not get the right context to effectively use the reply action.
Note that the getCommentText function attempts a recover of the comment text from the body. By network inspection though, the body seems not to be part (or maybe not part any more?) of the notification payload from the backend.
Click here to see the example payload
{
"id": <NNNNNNNN>,
"type": "comment",
"unread": null,
"snippet": null,
"meta": {
"ids": {
"user": 0,
"comment": 33,
"post": 21,
"site": <NNNNNNNN>
},
"links": {
"user": "https:\/\/public-api.wordpress.com\/rest\/v1\/users\/0",
"comment": "https:\/\/public-api.wordpress.com\/rest\/v1\/comments\/33",
"post": "https:\/\/public-api.wordpress.com\/rest\/v1\/posts\/21",
"site": "https:\/\/public-api.wordpress.com\/rest\/v1\/sites\/<NNNNNNNN>"
}
},
"status": null,
"subject": [
{
"text": "Testuser commented on Test"
},
{
"text": "test0987\n"
}
],
"timestamp": "2022-08-16T14:12:12+00:00",
"title": "Comment",
"icon": "https:\/\/2.gravatar.com\/avatar\/<omissis>",
"noticon": ""
}
A good start would be to check how iOS behaves in this context.
If in iOS the comment message is shown in the notification we should reproduce the same in Android. If it is not and we cannot infer or get the comment body from the notification (or by fetching the comment itself on the go, even though we could have a slow timing response for the user while fetching and be exposed to fetching errors scenarios! so not sure it's a great path; I would in case consider adding (back?) the body to the notification pyload tbh 🤔 ), as last resort, we could consider not presenting the actions in that comment notification. User tapping on the comment currently is already landed into the comment details where all actions are allowed with the right context.
cc @RenanLukas , @mkevins
Tested on [Pixel 4 Emu], Android [API 28], WPAndroid [20.5 but also in current trunk]
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with getCommentText in WordPress/src/main/java/org/wordpress/android/models/Note.java and inspect the example notification payload to confirm whether comment text is available. Compare how iOS presents the same notification; done means Android provides the comment context for its actions or does not present those actions when the context is unavailable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- android, kotlin
- Domain
- mobile
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100