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

fix: notifications link for mobile / desktop error generating errors in v30 #317

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Sep 24, 2024

Fixes #316

EDIT: I guess this has been broken for awhile, but wasn't super apparent until the new notification logging added in nextcloud/server#44770

@joshtrichards
Copy link
Member Author

/backport to stable30

@joshtrichards joshtrichards added this to the Nextcloud 30.0.1 milestone Sep 24, 2024
@joshtrichards joshtrichards changed the title fix: notifications link for mobile / desktop fix: notifications link for mobile / desktop error generating errors in v30 Sep 24, 2024
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Will fix tomorrow

@nickvergessen nickvergessen force-pushed the jtr/fix-issue-316-mobile-url-notifications branch from 41acbbd to a984156 Compare September 26, 2024 07:58
@nickvergessen
Copy link
Member

/backport to stable30

@nickvergessen
Copy link
Member

/backport to stable29

@nickvergessen
Copy link
Member

/backport to stable28

@joshtrichards
Copy link
Member Author

Thanks for bringing the URL generation up-to-date, @nickvergessen. Looks like I did it the old way.

@La-Fabrique-Info
Copy link

Warnings are still happening... doesn't seem fixed for me.
NC Version: 30.0.0.14
survey_client version: 2.0.0

{
    "reqId": "QYss6puhYbfWtBEO2goJ",
    "level": 2,
    "time": "2024-09-28T10:22:53+00:00",
    "remoteAddr": "********",
    "user": "*******",
    "app": "no app in context",
    "method": "GET",
    "url": "/ocs/v2.php/apps/notifications/api/v2/notifications",
    "message": "Icon of notification is not an absolute URL and does not work in mobile and desktop clients [app: survey_client, subject: updated]",
    "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/129.0.0.0 Safari/537.36",
    "version": "30.0.0.14",
    "data": [],
    "id": "66f7d914e838a"
}

@joshtrichards
Copy link
Member Author

joshtrichards commented Sep 28, 2024

Well, yeah - the code was just merged. :) You have to wait until the next maintenance release (or apply it manually early if you're into that sort of thing: https://docs.nextcloud.com/server/latest/admin_manual/issues/applying_patch.html)

@La-Fabrique-Info
Copy link

Sorry for not having mentioned it in my previous message but the modif file lib/BackgroundJobs/AdminNotification.php has been patched in my setup.

@nickvergessen
Copy link
Member

We didn't fix the icon. I can do it Monday.
Thanks for reporting back!

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