matiasdelellis / facerecognition

Nextcloud app that implement a basic facial recognition system.
GNU Affero General Public License v3.0
517 stars 46 forks source link

Review people found does not work on Nextcloud Hub 9 (30.0.0) #779

Open xegby opened 3 weeks ago

xegby commented 3 weeks ago

Face Recognition app uses the core/templates/message.html to show the clusters found. The template was removed from the Nextcloud server some time ago. So the webserver answers 404 (Not Found) to the app:

изображение

изображение

Returning back the template from older Nexctloud version to my installation helped to solve the problem.

But I guess we need more centralized solution in the app.

vwbusguy commented 2 weeks ago

Seeing the same thing here on NC30 as well. I assume some of this may be fixed with https://github.com/matiasdelellis/facerecognition/pull/778 ?

vwbusguy commented 2 weeks ago

The issue here seems to be that the "message.html" template no longer exists in core/templates in Nextcloud as of NC30.

It was removed with this commit: https://github.com/nextcloud/server/commit/8d5c0135dc7a0cc02c3346e2da48f6a152b783c3

The OC.dialogs was replaced with a new @nextcloud/dialogs, so facerecognition just needs updated to call the new method.

vwbusguy commented 2 weeks ago

The good news is that this is only in two places. If @matiasdelellis doesn't beat me to it, I might test out swapping those two calls and submit a PR for it a bit later on.

image

vwbusguy commented 2 weeks ago

OC.dialogs.confirm() hasn't been removed upstream. Rather, it looks like it just needs the updated templating refs for Nextcloud 30. I'm going to wait until https://github.com/matiasdelellis/facerecognition/pull/778 gets merged or @matiasdelellis takes a look before I spend any more time on this.

The dev docs note that the OC javascript object can have breaking changes from one release to the next, so I think we've simply found one of those.