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

Please read this if you want a working version on NC 27 #49

Open
Xefir opened this issue Jun 18, 2023 · 10 comments
Open

Please read this if you want a working version on NC 27 #49

Xefir opened this issue Jun 18, 2023 · 10 comments

Comments

@Xefir
Copy link

Xefir commented Jun 18, 2023

Hello.

I rewrite a good part of the app on my own repository here : https://git.crystalyx.net/Xefir/epubreader
You can download the latest release here and unzip/untar the archive or clone the repository in your apps directory to get the new working version.

On top of making the app works with the latest version of Nextcloud, I've done some other things like :

  • Typing everything to have a robust and easy development for the future
  • Follow the Nextcloud Coding Standard
  • Add PSALM to have a strict static analysis of the code and detect code mistakes
  • Move the vendor folder to public because vendor is usually used by composer and not for storing publicly JS assets
  • Add a development environment via composer

I won't provide a PR for this or publish my work on the Nextcloud AppStore, for a bunch of reasons :

  • There are way too much modifications to review it properly
  • There is already 3 different apps for epubreader on the store, and I don't want to become this xkcd
  • Moving the vendor folder seems to have broken the nextcloud transifex bot and I don't known how I can fix it (and it won't work without Github anyway)
  • I don't like hosting sources on Github when I can have it at home, that's why we use Nextcloud isn't it ? :D

But I won't make this app mine and I don't want to.
Do not hesitate to grab my work, fork it and put it back on Github if you want !
I will be glad if anyone (and maybe @e-alfred too !) want to continue my work !

Thank you for reading this and have a good time reading your precious books :)

@benjaminfrombe
Copy link

benjaminfrombe commented Jun 21, 2023

thanks for your work ! unfortunately I can't get it to work. After enabling I also am getting an internal server error on my admin page. This is what I find in the logs: Any help would be appreciated !

`[index] Error: Exception: Class "OCA\Epubreader\AppInfo\Application" not found in file '/var/www/html/custom_apps/epubreader/lib/Settings/Personal.php' line 57 at <>

  1. /var/www/html/lib/private/AppFramework/App.php line 183
    OC\AppFramework\Http\Dispatcher->dispatch(["OCA\Settings\ ... "], "index")
  2. /var/www/html/lib/private/Route/Router.php line 315
    OC\AppFramework\App::main("OCA\Settings\ ... r", "index", ["OC\AppFramewo ... "], ["overview","set ... "])
  3. /var/www/html/lib/base.php line 1064
    OC\Route\Router->match("/settings/admin/overview")
  4. /var/www/html/index.php line 36
    OC::handleRequest()

Caused by:

Error: Class "OCA\Epubreader\AppInfo\Application" not found at <>

  1. /var/www/html/lib/private/Settings/Manager.php line 220
    OCA\Epubreader\Settings\Personal->getSection()
  2. /var/www/html/lib/private/Settings/Manager.php line 334
    OC\Settings\Manager->getSettings("personal", "additional")
  3. /var/www/html/lib/private/Settings/Manager.php line 295
    OC\Settings\Manager->getPersonalSettings("additional")
  4. /var/www/html/apps/settings/lib/Controller/CommonSettingsTrait.php line 111
    OC\Settings\Manager->getPersonalSections()
  5. /var/www/html/apps/settings/lib/Controller/CommonSettingsTrait.php line 62
    OCA\Settings\Controller\AdminSettingsController->formatPersonalSections("admin", "overview")
  6. /var/www/html/apps/settings/lib/Controller/CommonSettingsTrait.php line 148
    OCA\Settings\Controller\AdminSettingsController->getNavigationParameters("admin", "overview")
  7. /var/www/html/apps/settings/lib/Controller/AdminSettingsController.php line 68
    OCA\Settings\Controller\AdminSettingsController->getIndexResponse("admin", "overview")
  8. /var/www/html/lib/private/AppFramework/Http/Dispatcher.php line 230
    OCA\Settings\Controller\AdminSettingsController->index("overview")
  9. /var/www/html/lib/private/AppFramework/Http/Dispatcher.php line 137
    OC\AppFramework\Http\Dispatcher->executeController(["OCA\Settings\ ... "], "index")
  10. /var/www/html/lib/private/AppFramework/App.php line 183
    OC\AppFramework\Http\Dispatcher->dispatch(["OCA\Settings\ ... "], "index")
  11. /var/www/html/lib/private/Route/Router.php line 315
    OC\AppFramework\App::main("OCA\Settings\ ... r", "index", ["OC\AppFramewo ... "], ["overview","set ... "])
  12. /var/www/html/lib/base.php line 1064
    OC\Route\Router->match("/settings/admin/overview")
  13. /var/www/html/index.php line 36
    OC::handleRequest()

GET /settings/admin/overview
from 172.18.0.1 by admin at 2023-06-21T09:06:34+00:00`

@Xefir
Copy link
Author

Xefir commented Jun 21, 2023

Hello @benjaminfrombe

Can you provide me your Nextcloud and PHP versions please ?

@ummon29
Copy link

ummon29 commented Jun 23, 2023

Hello,

after a quick test this (1.4.9) runs fine for me with Nextcloud 26.0.3 and PHP 8.1.
Thanks for the great work. I'm happy to have a usable version of this App again :-)

@benjaminfrombe
Copy link

benjaminfrombe commented Jun 24, 2023 via email

@devnoname120
Copy link

@Xefir Ah I just heard about your fork. Too bad because I already forked the project: https://github.com/devnoname120/epubviewer

I'm not sure yet how actively I will maintain epubviewer but I may take a look at your repo. Publishing the app in the NextCloud App Store is important because not all users have access to the filesystem/configuration of the server/VPS on which their instance runs.

For example Hetzner Storage Share (managed NextCloud) only allows you to install apps from the App Store, not import your own.

@Xefir
Copy link
Author

Xefir commented Aug 2, 2023

I push an update that fix a bug I introduce on folder sharing (and refacto again to have a even nicer stack).

@devnoname120 I see what you did and I understand that not all users will use my fork, but that's not my goal either.
In fact, if we can take over the apps that is already on the store (or remove them), I'll be happy to push my fork on it.
But renaming again like you did will ultimately make another app appear in it and the end users won't install any of them, it's just frustrating.

I'm working on another Nextcloud project that take all my time and I work on the epubreader fork when I have time.
I have finished to clean the PHP part, this part is done for me.
But the JS part is ... Erk ... Ugly as fuck and need to be redo from scratch IMHO.
Maybe one day ...

@devnoname120
Copy link

@Xefir I agree with your sentiment that it's annoying to have so many outdated forks on the store. However I would rather have n+1 forks with 1 that works, rather than n forks with 0 that works. Having it on the store is important for me because my NextCloud instance is managed by Hetzner and I can't sideload any apps, they must be installed from the store.

Nonetheless it should be possible to remove (or redirect) the older entries from the store by contacting NextCloud people. Ultimately I would like to move my repository to the NextCloud organization so that ownership can be more easily transferred in the future without creating yet another fork.

@Xefir
Copy link
Author

Xefir commented Aug 20, 2023

@devnoname120 If you app is moved to a official Nextcloud repository, I'll be glad to make you a Pull Request with ALL my changes.
It'll be very annoying for you to review, but I prefer to have an official fork settled once and for all like you say.

Take me in touch when the repository is ready :)

@devnoname120
Copy link

devnoname120 commented Aug 20, 2023

@Xefir Sounds good to me. Note that it may be easier to just grab your changes, push them to my repo, publish a new release, and only then apply for moving the repo to the NextCloud organization. It should be easier to have them accept a code base that is closer to their standards.

@devnoname120
Copy link

devnoname120 commented Sep 15, 2023

@Xefir git.crystalyx.net is down right now. Do you have a mirror?

Edit: looks like it's back up. For future visitors, here is a mirror of the source code in case it goes down again (maybe for good this time).

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

No branches or pull requests

4 participants