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 deprecated calls in files_reader app #108

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

e-alfred
Copy link

Hello,

I fixed up the deprecated OCP\JSON and OCP\Config calls for Nextcloud 14+.

@@ -12,20 +12,20 @@

namespace OCA\Files_Reader;

\OCP\JSON::callCheck();
\OCP\JSON::checkLoggedIn();
\OC_JSON::callCheck();

Choose a reason for hiding this comment

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

FYI: you replaced a deprecated public API with a private API that is subject to change at any time without warning (can even happen within minor version of the Nextcloud server because these are not supposed to be used in apps).

Choose a reason for hiding this comment

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

These files should be replaced with proper controllers, then you don't have to use any of these private APIs. See https://docs.nextcloud.com/server/14/developer_manual/app/controllers.html for more info on that.

@onliniak
Copy link

onliniak commented Oct 4, 2018

Hmm … now I have problem with OPDS.

Internal server error → https://github.com/Yetangitu/owncloud-apps/tree/cf484a2ab4a77c350be63522cd03bb8e7fd76074/files_reader

Błąd wewnętrzny serwera
Serwer nie był w stanie dokończyć Twojego żądania.

Jeśli to się powtórzy, wyślij proszę szczegóły techniczne administratorowi serwera.

Więcej szczegółów można znaleźć w logu serwera.

Szczegóły techniczne
Adres zdalny: my ip
ID żądania: kHDtPoAPvyQwwtDJx2RK
Typ: Error
Kod: 0
Komunikat: Class 'OCP\Config' not found
Plik: /usr/home/www/nextcloud/public_html/apps/files_opds/lib/config.php
Linia: 28

Ślad
#0 /usr/home/www/nextcloud/public_html/apps/files_opds/personal.php(20): OCA\Files_Opds\Config::get('enable', false)
#1 /usr/home/www/nextcloud/public_html/lib/private/legacy/app.php(622): include('/usr/home/onlin...')
#2 /usr/home/www/nextcloud/public_html/lib/private/Settings/Manager.php(365): OC_App::getForms('personal')
#3 /usr/home/www/nextcloud/public_html/settings/Controller/CommonSettingsTrait.php(93): OC\Settings\Manager->getPersonalSections()
#4 /usr/home/www/nextcloud/public_html/settings/Controller/CommonSettingsTrait.php(46): OC\Settings\Controller\PersonalSettingsController->formatPersonalSections('personal', 'personal-info')
#5 /usr/home/www/nextcloud/public_html/settings/Controller/CommonSettingsTrait.php(125): OC\Settings\Controller\PersonalSettingsController->getNavigationParameters('personal', 'personal-info')
#6 /usr/home/www/nextcloud/public_html/settings/Controller/PersonalSettingsController.php(57): OC\Settings\Controller\PersonalSettingsController->getIndexResponse('personal', 'personal-info')
#7 /usr/home/www/nextcloud/public_html/lib/private/AppFramework/Http/Dispatcher.php(166): OC\Settings\Controller\PersonalSettingsController->index('personal-info')
#8 /usr/home/www/nextcloud/public_html/lib/private/AppFramework/Http/Dispatcher.php(99): OC\AppFramework\Http\Dispatcher->executeController(Object(OC\Settings\Controller\PersonalSettingsController), 'index')
#9 /usr/home/www/nextcloud/public_html/lib/private/AppFramework/App.php(118): OC\AppFramework\Http\Dispatcher->dispatch(Object(OC\Settings\Controller\PersonalSettingsController), 'index')
#10 /usr/home/www/nextcloud/public_html/lib/private/AppFramework/Routing/RouteActionHandler.php(47): OC\AppFramework\App::main('OC\\Settings\\Con...', 'index', Object(OC\AppFramework\DependencyInjection\DIContainer), Array)
#11 [internal function]: OC\AppFramework\Routing\RouteActionHandler->__invoke(Array)
#12 /usr/home/www/nextcloud/public_html/lib/private/Route/Router.php(297): call_user_func(Object(OC\AppFramework\Routing\RouteActionHandler), Array)
#13 /usr/home/www/nextcloud/public_html/lib/base.php(987): OC\Route\Router->match('/settings/user')
#14 /usr/home/www/nextcloud/public_html/index.php(42): OC::handleRequest()
#15 {main}

Of course without fixed files_reader OPDS works. For info.

@scurrvy2020
Copy link

I'm getting this error with these commits:
ArgumentCountError: Too few arguments to function OCP\Util::connectHook(), 3 passed in /srv/http/nextcloud/apps/files_reader/lib/Hooks.php on line 22 and exactly 4 expected

I think this is the relevant section of Hooks.php:
class Hooks {

20
21     public static function register() {
22         Util::connectHook('js', 'OCA\Files_Reader\Hooks', 'announce_settings');
23
24         \OC::$server->getRootFolder()->listen('\OC\Files', 'preDelete', function (Node $node) {
25             $fileId = $node->getId();
26             $connection = \OC::$server->getDatabaseConnection();
27             self::deleteFile($connection, $fileId);
28         });
29         \OC::$server->getUserManager()->listen('\OC\User', 'preDelete', function (User $user) {
30             $userId = $user->getUID();
31             $connection = \OC::$server->getDatabaseConnection();
32             self::deleteUser($connection, $userId);
33         });
34     }
35

@thomas2403
Copy link

@manvalls
Sorry, but files_opds still seems to have a problem. When I try to enter settings as admin I'm getting:

Error: Call to undefined method OCP\User::getDisplayName()

@manvalls
Copy link

manvalls commented Oct 8, 2018

Right, didn't test opds. But hey, at least the reader is working! :D

@e-alfred
Copy link
Author

e-alfred commented Oct 8, 2018

@manvalls Your version works well, except that it doesn't open *.cbr/cbz files even if they are enabled in the settings menu.

@hcharbonnier
Copy link

I fixed files_opds, if any of you is interested :
https://github.com/hcharbonnier/owncloud-apps

@manvalls
Copy link

Alright guys I've merged @hcharbonnier's changes and fixed the cbr/cbz issue:

https://github.com/manvalls/owncloud-apps/blob/master/dist/files_reader-1.2.3-patched-5-NC.tar.gz?raw=true

https://github.com/manvalls/owncloud-apps/blob/master/dist/files_opds-1.2.3-patched-5-NC.tar.gz?raw=true

@scurrvy2020
Copy link

@manvalls are all of your changes in a pull request? (I'm just curious)

Thanks so much for making these changes so it will work in 14.

@psychodogg
Copy link

Seems to be having issues saving settings and opening to last page opened.

@scurrvy2020
Copy link

How is it looking for this app and NC15? Has anyone tried it?

@e-alfred
Copy link
Author

The new grid view button introduced by Nextcloud 15 overlaps the close button of this app, otherwise it works pretty well.

@scurrvy2020
Copy link

Ok, I've upgraded and it works just fine. The grid view button gets in the way of other apps as well.

@rlitman rlitman mentioned this pull request Feb 20, 2019
@scurrvy2020
Copy link

scurrvy2020 commented Nov 24, 2019

@e-alfred I've been using your patches through NC16 and it was working great. However, with NC17 there are some changes and it seems the frame to display the book isn't being handled correctly.

Are you still trying to maintain this app?

I noticed on mobile (chrome and firefox) the app will open in a new tab and everything renders correctly. Do you know of a way to get it to open a new tab on desktop?

@noci2012
Copy link

noci2012 commented Nov 24, 2019

i took a stab @ files_opds #121

@e-alfred
Copy link
Author

@scurrvy2020 I still use @manvalls release which is more complete than my quick fix. I can reproduce your problem and mobile works because it loads the viewer in a new tab without the Files app interfering.

@e-alfred
Copy link
Author

@scurrvy2020 I fixed this in e-alfred@0678c79. It loads in a new tab now instead of an Iframe.

@scurrvy2020
Copy link

Thanks a lot! I'll add it to @manvalis fork.

@e-alfred
Copy link
Author

@scurrvy2020 I could start to maintain this app and push a new version to the app store (under a different name) if some people want to help me fix problems that arise.

Maybe @manvalls @noci2012 @hcharbonnier want to help out?

@noci2012
Copy link

why not. (i am no wizard on the Nextcloud API, maybe an apprentice, if a can help i will).

@scurrvy2020
Copy link

@e-alfred I'm in! I think it would be great. I was thinking of doing that myself, but I'm still too novice.

@e-alfred
Copy link
Author

I will publish the app into the app store once again and therefore already requested a certificate for it: nextcloud/app-certificate-requests#293

If anynone wants to help me maintaining the app, all PRs are welcome on my repository.

@scurrvy2020
Copy link

scurrvy2020 commented Mar 24, 2020 via email

@e-alfred
Copy link
Author

Future app development will happen in this repository: https://github.com/e-alfred/epubreader

@manvalls @noci2012 @everybody Any help is appreciated to continue the the development of this app.

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

Successfully merging this pull request may close these issues.

9 participants