Overriding in packages does not seem to work for me(anymore)

Permalink
Somehow overriding in packages ends up being a big pain whenever I'm building something for C5.
You got this link:
https://documentation.concrete5.org/tutorials/override-almost-any-co...
Which doesn't seem to work for me at all.

Path based block controller overrides in a package simply won't work.
If i for example copy the autonav block controller to /packages/my_package/blocks/autonav, change the namespace to Concrete\Package\MyPackage\Block\Autonav nothing happens.

Same goes for the AccountMenu controller to change the menu in the users profile.

Before 8.2 you could still override stuff doing:
$objEnv = Environment::get();
$objEnv->overrideCoreByPackage('blocks/page_attribute_display/templates/date_time.php', $this);
$objEnv->overrideCoreByPackage('blocks/autonav/controller.php', $this);

This is deprecated now and thus stopped working.

So I am at a loss now because there doesn't seem to be an decent documentation on the matter.
I would really appreciate if someone could explain to me how I can override block controllers, block templates and single page controllers. Or point me towards some documentation.

bleenders
 
hutman replied on at Permalink Reply
hutman
Is there a reason you're trying to override those in a package rather than just in the application directory? As far as I'm aware overriding (almost) anything from the core into the application directory works like a charm, just as described in that documentation link.
bleenders replied on at Permalink Reply
bleenders
Because I'm creating a package. If I would make the package available on a marketplace or something like that it should all be plug and play. You should not have to manually add files to the application folder or edit /concrete/config/app.php to change the theme for the users account section, it should all be contained in the package itself.

I also like to keep my project structure organized and clear. Having to add a portion of the files to a different file structure makes no sense to me.

At the moment there is official documentation on the matter that simply does not work anymore and hasn't been updated in the past two years. I think this a crucial part of the framework for developers and it's lacking documentation.
And if you deprecate something like:
$objEnv = Environment::get();
$objEnv->overrideCoreByPackage('blocks/page_list/controller.php', $this);

Why not add the new alternative for the method instead of just adding
* Deprecated. Use Concrete\Core\Filesystem\FileLocator instead.

Because I still haven't figured out how I use the FileLocator the achieve the same result.

We run over two hundred Concrete5 websites at the moment and for most tailor made jobs I first see if I can develop it in Concrete5 because I don't have to worry about the routing editing of pages etc. But lately I find myself having to move to Laravel more and more while I rather use Concrete5 with all of it's functionalities that come with it out of the box.
hutman replied on at Permalink Reply
hutman
I apologize as I have never used this, did you put that code into your package's install method or where did you put it?

I'm hoping if I can replicate it in 5.7 I can maybe figure out how to do it in 5.8.
bleenders replied on at Permalink Reply
bleenders
The on_start method of my package controller.
This method is deprecated now. Works fine in all my 5.7 projects.
hutman replied on at Permalink Reply
hutman
I'm able to replicate your issue but I can't seem to solve it. Somebody put this forum post into the C5 Slack documentation channel so hopefully it will get documented. Sorry I can't help.
JohntheFish replied on at Permalink Reply
JohntheFish
I find packages are always a good way to develop functionality, even if for single use on one project. Packages are much easier to manage and deploy than a disparate spread of code throughout /application.

When an override can only be achieved by code in /application (or even a core patch), I write an installer in a package that copies files from the package to /application (or core). The installer also backs up any original files and reverses the process on uninstall, with a few little niceties such as detecting whether each 'version' matches what the installer thinks should be there.

If you are considering the marketplace, any package that overrides the core is always looked at sceptically and the review begins from the assumption the package is unacceptable, and can then be reasoned into 'black', or perhaps red or amber if its a very benign override that precedes a change that is coming to the core anyway.
bleenders replied on at Permalink Reply
bleenders
Copying files over in the installer is a good option. I already use an installer that creates the entities, attibutes, blocks and single pages when installing or upgrading. I guess I could also copy over files to the application folder. Copying to the core will take away the option to upgrade Concrete5 it self if I'm not mistaken.
Still, I would rather see the path based overrides working. Makes it a lot more organized.

I also need to edit /concrete/config/app.php to change:
'theme_paths' => [
        '/account' => VIEW_CORE_THEME,
        '/account/*' => VIEW_CORE_THEME,
        '/install' => VIEW_CORE_THEME,
        '/login' => [
            VIEW_CORE_THEME,
            VIEW_CORE_THEME_TEMPLATE_BACKGROUND_IMAGE,
        ],
        '/register' => VIEW_CORE_THEME,
    ],

I want it to use my own theme instead of the "concrete" theme. Overriding the theme in a package does not seem to work.

Do you have a way to change the values in app.php in an installer?
MrKDilkington replied on at Permalink Reply
MrKDilkington
@JohntheFish

Would you happen to have any code samples for your copying files to application process?

Are you using League Flysytem or Illuminate Filesystem?
https://flysystem.thephpleague.com/api/...
https://laravel.com/api/4.2/Illuminate/Filesystem/Filesystem.html...
JohntheFish replied on at Permalink Reply
JohntheFish
Last time I needed to do it I used a mix of the file helper (to get content and compare) and php file copy, rename and delete. If it was a marketplace package I would have looked at doing the same with one of the core vendor filesystem classes.

The core patching algorithm I generally use is:

get the current core directory (to allow for updates)
for each patch file
-compare to current core file (a string compare or MD5 compare)
- if not identical:
-- note the current core file path
-- copy current core file into a backup within the package
-- copy the patch file into a temporary
-- rename the temporary to replace the core file

For placing overrides in /application:

get the current core directory (to allow for updates)
for each patch file
-compare to current core file
-if not identical:
-- determine the relevant /application file path
-- if there is already a file there, complain to user
-- if necessary, make directories
-- copy the patch file into a temporary
-- rename the temporary to become the /application override

Then on uninstall, apply a similar process in reverse, with a check that only the same files we have patched or overridden are removed (just in case someone else has subsequently done something).

For core files that are pending on github for a future core update, I also include a core version check against the future version.
MrKDilkington replied on at Permalink Reply
MrKDilkington
@JohntheFish

Thank you for explaining your copy override steps.

A few questions related to this tutorial:
https://documentation.concrete5.org/tutorials/override-almost-any-co...
- Have you been able to use "Service Provider Overrides" to override services successfully?
- Have you tried overriding services in a package?
- For non-services (not found in the app.php providers list), like PageList, have you been able to override a file like this?

I tried creating an autoloader for files placed under the application/src/ directory. The autoloader worked successfully with my own custom files, but not for overriding core files.
https://documentation.concrete5.org/developers/extending-concrete5-w...
JohntheFish replied on at Permalink Reply
JohntheFish
No, no, tried and failed, hence the above.
I can't say categorically that overrides don't work. Last time I tried I got to the point (of lack of success) where my time was better spent on the solution above because I had confidence that it would work and it was appropriate to the problem at the time.
MrKDilkington replied on at Permalink Reply
MrKDilkington
@JohntheFish

I think the current "Override (almost) any core file in 5.7" tutorial would benefit from more examples, instructions on how to override non-services (like PageList), and how/if this can be used in packages.
https://documentation.concrete5.org/tutorials/override-almost-any-co...

It turns out that the "Override (almost) any core file in 5.7" tutorial was edited and its original content was very different. If you visit the Way Back Machine, you can get a copy of it. If I am correct, the original instructions showed how to override "almost" any core file, unlike the current version.
https://web.archive.org/web/20151120024447/https://documentation.con...

Did you happen to try or see this original tutorial?
JohntheFish replied on at Permalink Reply
JohntheFish
I remember the original and was saddened by its being removed and watered down. I didn't know if that was because the original would no longer work, or if it was a policy. If it still works, that wayback post could be useful. Perhaps the entire original could be cut/pasted into the conversation beneath.
MrKDilkington replied on at Permalink Reply
MrKDilkington
@JohntheFish

It would be useful to see if the original still works with 5.7.

I think Mainio said it might need to be updated for v8. We could ask him to update it and post it here. Also, if the code can be used in a package on_start() method.
mnakalay replied on at Permalink Reply
mnakalay
He did say "DO NOT abuse this method and always when there is a better way to override something, you should use that instead (e.g. service/helper overrides)."
MrKDilkington replied on at Permalink Reply
MrKDilkington
I messaged Mainio and attempted to make the case for him to update the tutorial for v8 and post the instructions here (in the forum as a reply, not in a tutorial which requires approval).