Fixes in notifications for pending issues #1272

maskaravivek · 2018-03-07T22:12:36Z

Description

Fixes #7

Points (3) and (4) in the comment left by @misaochan have been fixed thanks to the help from @whym!

Tests performed

Have tested manually that the notification work properly.

Open the correct link pointing to the message instead of the talk page
Show details about possible deletion

Screenshots showing what changed

codecov-io · 2018-03-07T22:40:53Z

Codecov Report

Merging #1272 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #1272      +/-   ##
=========================================
- Coverage    3.73%    3.7%   -0.03%     
=========================================
  Files         127     127              
  Lines        6054    6103      +49     
  Branches      588     601      +13     
=========================================
  Hits          226     226              
- Misses       5813    5862      +49     
  Partials       15      15

Impacted Files	Coverage Δ
app/src/main/java/fr/free/nrw/commons/Utils.java	`24.19% <ø> (ø)`	⬆️
...rw/commons/mwapi/ApacheHttpClientMediaWikiApi.java	`5.49% <0%> (+0.05%)`	⬆️
...nrw/commons/notification/NotificationRenderer.java	`0% <0%> (ø)`	⬆️
...fr/free/nrw/commons/notification/Notification.java	`0% <0%> (ø)`	⬆️
...ee/nrw/commons/notification/NotificationUtils.java	`0% <0%> (ø)`	⬆️
...nrw/commons/notification/NotificationActivity.java	`0% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe1f0b5...365bbc4. Read the comment docs.

misaochan · 2018-03-08T10:40:49Z

app/build.gradle

@@ -98,6 +98,12 @@ dependencies {
    compile 'com.borjabravo:readmoretextview:2.1.0'
 }

+repositories {


Could I ask why these were included in this PR?

It got added by mistake. Removing. :)

misaochan · 2018-03-08T13:49:43Z

Hi @maskaravivek ,

Thanks for the PR. Unfortunately I'm still having some issues with it:

As seen in the first screenshot below, the deletion talk messages are not distinguishable from regular talk messages in the overall view. Only when expanded (second screenshot) do they show up, and even then not very clearly - you have to be familiar with Commons to guess that it is a deletion message. The deletion message should say "Misaochan nominated your file Apron from Swiss Souvenir Shop.jpg for deletion".
The deletion messages do indeed bring me to the appropriate talk page section when tapped, nice! 👍 But the talk messages just bring me to the top of my talk page when I tap them, similar to how it was before this PR. For instance, if I tap the top message, I should be brought to https://commons.wikimedia.org/wiki/User_talk:Misaochan2#Test_message_2 , but instead I am brought to https://commons.wikimedia.org/wiki/User_talk:Misaochan2?markasread=8326873
Sorry, this is a new request, but it would be great if we could get a loading progress dialog so that users know that the notifications are loading. I wouldn't let that block the merging of this PR though, but would be good for the next PR.

Logcat:

03-08 23:36:37.929: D/NotificationActivity(4904): Number of notifications is 8
03-08 23:36:55.895: D/NotificationActivity(4904): Notification clicked https://commons.wikimedia.org/wiki/User_talk:Misaochan2?markasread=8326873
03-08 23:36:56.116: D/EGL_emulation(4904): eglMakeCurrent: 0xa0723b80: ver 2 0 (tinfo 0x927128c0)
03-08 23:36:56.164: D/OpenGLRenderer(4904): endAllActiveAnimators on 0x8ec74100 (RelativeLayout) with handle 0x8f11cde0
03-08 23:37:32.101: D/EGL_emulation(4904): eglMakeCurrent: 0xa0723b80: ver 2 0 (tinfo 0x927128c0)
03-08 23:38:51.113: D/NotificationActivity(4904): Notification clicked https://commons.wikimedia.org/wiki/User_talk:Misaochan2?markasread=8326834#File:Test_image_for_deletion.jpg
03-08 23:38:51.336: D/EGL_emulation(4904): eglMakeCurrent: 0xa0723b80: ver 2 0 (tinfo 0x927128c0)
03-08 23:38:51.357: D/OpenGLRenderer(4904): endAllActiveAnimators on 0x8ec73300 (RelativeLayout) with handle 0x8ec65aa0
03-08 23:38:56.217: D/EGL_emulation(4904): eglMakeCurrent: 0xa0723b80: ver 2 0 (tinfo 0x927128c0)
03-08 23:38:58.978: D/NotificationActivity(4904): Notification clicked https://commons.wikimedia.org/wiki/User_talk:Misaochan2?markasread=7706109#File:Apron_from_Swiss_souvenir_shop.jpg
03-08 23:38:59.164: D/EGL_emulation(4904): eglMakeCurrent: 0xa0723b80: ver 2 0 (tinfo 0x927128c0)
03-08 23:38:59.175: D/OpenGLRenderer(4904): endAllActiveAnimators on 0x8ec75980 (RelativeLayout) with handle 0x8eb4cff0
03-08 23:39:04.523: D/EGL_emulation(4904): eglMakeCurrent: 0xa0723b80: ver 2 0 (tinfo 0x927128c0)
03-08 23:39:05.662: D/NotificationActivity(4904): Notification clicked https://commons.wikimedia.org/wiki/User_talk:Misaochan2?markasread=8326873
03-08 23:39:05.867: D/EGL_emulation(4904): eglMakeCurrent: 0xa0723b80: ver 2 0 (tinfo 0x927128c0)
03-08 23:39:05.875: D/OpenGLRenderer(4904): endAllActiveAnimators on 0x8ec74100 (RelativeLayout) with handle 0x8ec652c0
03-08 23:39:09.519: D/EGL_emulation(4904): eglMakeCurrent: 0xa0723b80: ver 2 0 (tinfo 0x927128c0)

misaochan

Please see comment for screenshots/logs

maskaravivek · 2018-03-13T23:18:29Z

I have fixed the following issues:

Added progress bar while the notifications load.
Show proper messages for deletion requests. I could figure out the patterns in various deletion request messages, so I am simply displaying the body of the notification for talk messages. It seems to serve the purpose.

misaochan · 2018-03-14T10:17:58Z

Hi @maskaravivek ,

When testing with Misaochan2 on API 25, no notifications load (and no progress bar), and I get the error java.lang.NullPointerException: Attempt to invoke interface method 'org.w3c.dom.NodeList org.w3c.dom.Node.getChildNodes()' on a null object reference (but it doesn't crash). Logs below:

03-14 20:16:23.437: D/NotificationActivity(3955): Add notifications
03-14 20:16:23.492: E/RecyclerView(3955): No adapter attached; skipping layout
03-14 20:16:23.522: D/EGL_emulation(3955): eglMakeCurrent: 0x8f0ffc40: ver 2 0 (tinfo 0x8e8d0880)
03-14 20:16:23.577: D/EGL_emulation(3955): eglMakeCurrent: 0x8f0ffc40: ver 2 0 (tinfo 0x8e8d0880)
03-14 20:16:23.578: E/RecyclerView(3955): No adapter attached; skipping layout
03-14 20:16:23.587: D/EGL_emulation(3955): eglMakeCurrent: 0x8f0ffc40: ver 2 0 (tinfo 0x8e8d0880)
03-14 20:16:24.865: E/NotificationActivity(3955): Error occurred while loading notifications
03-14 20:16:24.865: E/NotificationActivity(3955): java.lang.NullPointerException: Attempt to invoke interface method 'org.w3c.dom.NodeList org.w3c.dom.Node.getChildNodes()' on a null object reference
03-14 20:16:24.865: E/NotificationActivity(3955): 	at fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApi.getNotifications(ApacheHttpClientMediaWikiApi.java:417)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at fr.free.nrw.commons.notification.NotificationController.getNotifications(NotificationController.java:34)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at fr.free.nrw.commons.notification.NotificationActivity.lambda$addNotifications$0$NotificationActivity(NotificationActivity.java:60)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at fr.free.nrw.commons.notification.NotificationActivity$$Lambda$0.call(Unknown Source)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at io.reactivex.internal.operators.observable.ObservableFromCallable.subscribeActual(ObservableFromCallable.java:42)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at io.reactivex.Observable.subscribe(Observable.java:10901)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at io.reactivex.internal.operators.observable.ObservableSubscribeOn$SubscribeTask.run(ObservableSubscribeOn.java:96)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at io.reactivex.Scheduler$DisposeTask.run(Scheduler.java:452)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:61)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:52)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at java.util.concurrent.FutureTask.run(FutureTask.java:237)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:272)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1133)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:607)
03-14 20:16:24.865: E/NotificationActivity(3955): 	at java.lang.Thread.run(Thread.java:761)

Logging out and in again fixes the issue.

misaochan · 2018-03-14T10:30:23Z

After logging back in, I do get notifications. And the progress bar works well, nice. 👍 However, now the deletion requests are not displayed at all for me, and in fact even the talk messages are not displayed :/. For instance, you can see I have 2 deletion requests and 2 talk messages here: https://commons.wikimedia.org/wiki/User_talk:Misaochan2

But the notifications page shows me:

Logs:

03-14 20:31:04.459: D/NotificationActivity(3955): Add notifications
03-14 20:31:04.496: E/RecyclerView(3955): No adapter attached; skipping layout
03-14 20:31:04.518: D/EGL_emulation(3955): eglMakeCurrent: 0x8f0ffc40: ver 2 0 (tinfo 0x8e8d0880)
03-14 20:31:04.535: D/EGL_emulation(3955): eglMakeCurrent: 0x8f0ffc40: ver 2 0 (tinfo 0x8e8d0880)
03-14 20:31:04.536: E/RecyclerView(3955): No adapter attached; skipping layout
03-14 20:31:04.542: D/EGL_emulation(3955): eglMakeCurrent: 0x8f0ffc40: ver 2 0 (tinfo 0x8e8d0880)
03-14 20:31:05.870: D/NotificationActivity(3955): Number of notifications is 3

misaochan · 2018-03-14T10:35:30Z

When testing using my main username (Misaochan), it gets even worse lol. No notifications are found at all and a blank screen is showed, despite there not being any NPE error as shown in my first review post. But clearly, there are plenty of notifications which SHOULD be displayed: https://commons.wikimedia.org/wiki/User_talk:Misaochan

03-14 20:33:23.125: D/NotificationActivity(3955): Add notifications
03-14 20:33:23.163: E/RecyclerView(3955): No adapter attached; skipping layout
03-14 20:33:23.175: D/EGL_emulation(3955): eglMakeCurrent: 0x8f0ffc40: ver 2 0 (tinfo 0x8e8d0880)
03-14 20:33:23.190: D/EGL_emulation(3955): eglMakeCurrent: 0x8f0ffc40: ver 2 0 (tinfo 0x8e8d0880)
03-14 20:33:23.191: E/RecyclerView(3955): No adapter attached; skipping layout
03-14 20:33:23.194: D/EGL_emulation(3955): eglMakeCurrent: 0x8f0ffc40: ver 2 0 (tinfo 0x8e8d0880)
03-14 20:33:23.778: D/NotificationActivity(3955): Number of notifications is 0

Suggestion: I wonder if we could just use your previous code, but display "File nominated for deletion" in front of the message for deletions when a particular pattern is detected, instead of switching to displaying the body.

misaochan · 2018-03-15T15:12:30Z

Still getting the same crash for misaochan that we discussed last night with the latest commit unfortunately:

03-16 01:10:38.036: E/AndroidRuntime(13219): FATAL EXCEPTION: main
03-16 01:10:38.036: E/AndroidRuntime(13219): Process: fr.free.nrw.commons.debug, PID: 13219
03-16 01:10:38.036: E/AndroidRuntime(13219): java.lang.StringIndexOutOfBoundsException: String index out of range: 241
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at java.lang.AbstractStringBuilder.getChars(AbstractStringBuilder.java:353)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at java.lang.StringBuilder.getChars(StringBuilder.java)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.text.TextUtils.getChars(TextUtils.java:87)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.text.SpannableStringBuilder.<init>(SpannableStringBuilder.java:66)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at com.borjabravo.readmoretextview.ReadMoreTextView.updateCollapsedText(ReadMoreTextView.java:137)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at com.borjabravo.readmoretextview.ReadMoreTextView.getTrimmedText(ReadMoreTextView.java:114)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at com.borjabravo.readmoretextview.ReadMoreTextView.getDisplayableText(ReadMoreTextView.java:90)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at com.borjabravo.readmoretextview.ReadMoreTextView.setText(ReadMoreTextView.java:84)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at com.borjabravo.readmoretextview.ReadMoreTextView.access$200(ReadMoreTextView.java:33)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at com.borjabravo.readmoretextview.ReadMoreTextView$1.onGlobalLayout(ReadMoreTextView.java:206)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:912)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2106)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1254)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:6337)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:874)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.view.Choreographer.doCallbacks(Choreographer.java:686)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.view.Choreographer.doFrame(Choreographer.java:621)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:860)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.os.Handler.handleCallback(Handler.java:751)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.os.Handler.dispatchMessage(Handler.java:95)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.os.Looper.loop(Looper.java:154)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at android.app.ActivityThread.main(ActivityThread.java:6119)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at java.lang.reflect.Method.invoke(Native Method)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
03-16 01:10:38.036: E/AndroidRuntime(13219): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)

If it helps, if I open the app again, it loads the notifications from the previous account that I logged in to (misaochan_test). I am sure that I'm logged in to misaochan, as my contributions display appropriately.

Also, these lines are never called:


                    Timber.e(throwable, "Error occurred while loading notifications");
                    ViewUtil.showLongToast(this, R.string.error_notifications);

maskaravivek · 2018-03-21T07:01:41Z

@misaochan The crash is now fixed and ready for testing

misaochan · 2018-03-25T08:04:49Z

Works wonderfully for me! Great job @maskaravivek 👍

misaochan reviewed Mar 8, 2018

View reviewed changes

misaochan requested changes Mar 8, 2018

View reviewed changes

maskaravivek force-pushed the notificationFixes branch from a7bb83d to e6f98e7 Compare March 13, 2018 23:10

commons-app deleted a comment Mar 14, 2018

maskara added 5 commits March 21, 2018 10:26

Fixes in notifications for pending issues

21a6b1f

Suggested changes for notification fixes

e5c8e40

Minor notification fixes

59abd02

With support for bundled notifications

f34af1e

Null fix for notifications

528faa1

maskaravivek force-pushed the notificationFixes branch from 58bf005 to 365bbc4 Compare March 21, 2018 07:00

Fix crash due to read more text view

365bbc4

commons-app deleted a comment Mar 21, 2018

Merge branch 'master' into notificationFixes

6eaac1d

commons-app deleted a comment Mar 24, 2018

misaochan merged commit bbf159b into commons-app:master Mar 25, 2018

maskaravivek deleted the notificationFixes branch September 12, 2018 20:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixes in notifications for pending issues #1272

Fixes in notifications for pending issues #1272

Uh oh!

maskaravivek commented Mar 7, 2018

Uh oh!

codecov-io commented Mar 7, 2018 •

edited

Loading

Uh oh!

misaochan Mar 8, 2018

Uh oh!

maskaravivek Mar 8, 2018

Uh oh!

misaochan commented Mar 8, 2018

Uh oh!

misaochan left a comment

Uh oh!

maskaravivek commented Mar 13, 2018

Uh oh!

misaochan commented Mar 14, 2018 •

edited

Loading

Uh oh!

misaochan commented Mar 14, 2018 •

edited

Loading

Uh oh!

misaochan commented Mar 14, 2018 •

edited

Loading

Uh oh!

misaochan commented Mar 15, 2018 •

edited

Loading

Uh oh!

maskaravivek commented Mar 21, 2018

Uh oh!

misaochan commented Mar 25, 2018

Uh oh!

Uh oh!

Fixes in notifications for pending issues #1272

Fixes in notifications for pending issues #1272

Uh oh!

Conversation

maskaravivek commented Mar 7, 2018

Description

Tests performed

Screenshots showing what changed

Uh oh!

codecov-io commented Mar 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

misaochan Mar 8, 2018

Choose a reason for hiding this comment

Uh oh!

maskaravivek Mar 8, 2018

Choose a reason for hiding this comment

Uh oh!

misaochan commented Mar 8, 2018

Uh oh!

misaochan left a comment

Choose a reason for hiding this comment

Uh oh!

maskaravivek commented Mar 13, 2018

Uh oh!

misaochan commented Mar 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

misaochan commented Mar 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

misaochan commented Mar 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

misaochan commented Mar 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maskaravivek commented Mar 21, 2018

Uh oh!

misaochan commented Mar 25, 2018

Uh oh!

Uh oh!

codecov-io commented Mar 7, 2018 •

edited

Loading

misaochan commented Mar 14, 2018 •

edited

Loading

misaochan commented Mar 14, 2018 •

edited

Loading

misaochan commented Mar 14, 2018 •

edited

Loading

misaochan commented Mar 15, 2018 •

edited

Loading