Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch all notification to V2 #11140

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Switch all notification to V2 #11140

wants to merge 1 commit into from

Conversation

tobiasKaminsky
Copy link
Member

@tobiasKaminsky tobiasKaminsky commented Dec 5, 2022

Needs: nextcloud/android-library#453

Signed-off-by: tobiasKaminsky [email protected]

  • Tests written, or not not needed

Copy link
Member

@AlvaroBrey AlvaroBrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12-12 13:03:35.263  9843 10371 D GetNotificationRemoteOperation: Not yet implemented
12-12 13:03:35.264  9843 10371 E NotificationWork: Error creating account
12-12 13:03:35.264  9843 10371 E NotificationWork: java.lang.UnsupportedOperationException: Not yet implemented
12-12 13:03:35.264  9843 10371 E NotificationWork: 	at com.owncloud.android.lib.common.operations.RemoteOperation.run(RemoteOperation.java:110)
12-12 13:03:35.264  9843 10371 E NotificationWork: 	at com.owncloud.android.lib.common.operations.RemoteOperation.execute(RemoteOperation.java:220)
12-12 13:03:35.264  9843 10371 E NotificationWork: 	at com.nextcloud.client.jobs.NotificationWork.fetchCompleteNotification(NotificationWork.kt:256)
12-12 13:03:35.264  9843 10371 E NotificationWork: 	at com.nextcloud.client.jobs.NotificationWork.doWork(NotificationWork.kt:125)
12-12 13:03:35.264  9843 10371 E NotificationWork: 	at androidx.work.Worker$1.run(Worker.java:86)
12-12 13:03:35.264  9843 10371 E NotificationWork: 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1137)
12-12 13:03:35.264  9843 10371 E NotificationWork: 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:637)
12-12 13:03:35.264  9843 10371 E NotificationWork: 	at java.lang.Thread.run(Thread.java:1012)
12-12 13:03:35.266  9843 10119 I WM-WorkerWrapper: Worker result SUCCESS for Work [ id=00a1825a-379d-4fb2-b0d3-b87c2bbcee0a, tags={ name:notification, com.nextcloud.client.jobs.NotificationWork, timestamp:1670846615204, * }

Getting this when testing with occ notification:test-push <username>

@tobiasKaminsky
Copy link
Member Author

12-12 13:03:35.263  9843 10371 D GetNotificationRemoteOperation: Not yet implemented

This should have been detected by tests in library's PR.
Let me check.

@tobiasKaminsky
Copy link
Member Author

@AlvaroBrey indeed this was not covered by library tests.
Now it should work.

@AlvaroBrey
Copy link
Member

/rebase

@tobiasKaminsky
Copy link
Member Author

Manually rebased

@tobiasKaminsky
Copy link
Member Author

image

@AlvaroBrey is this sort of a Java<->Kotlin "problem"?

@AlvaroBrey
Copy link
Member

AlvaroBrey commented Dec 13, 2022

image

@AlvaroBrey is this sort of a Java<->Kotlin "problem"?

Looks like it, yeah. From a short research it looks like something that cannot be solved, so either work around (by defining it as RemoteOperationResult<List<? extends Notification>> and then casting getResultData to List<Notification>) or convert the class to Kotlin seem to be the only options.

@AlvaroBrey
Copy link
Member

/rebase

@AlvaroBrey
Copy link
Member

/rebase

@github-actions
Copy link

github-actions bot commented Feb 8, 2023

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/11140.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

Signed-off-by: tobiasKaminsky <[email protected]>
Copy link

test-Unit test failed, but no output was generated. Maybe a preliminary stage failed.

Copy link

blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants