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

Theme system is not detecting the module overrides #1901

Open
CaptainGodual opened this issue Nov 29, 2024 · 26 comments
Open

Theme system is not detecting the module overrides #1901

CaptainGodual opened this issue Nov 29, 2024 · 26 comments

Comments

@CaptainGodual
Copy link

CaptainGodual commented Nov 29, 2024

Describe the bug
A clear and concise description of what the bug is. Please upload the Laravel logs as well from storage/logs/laravel.log (or the file with the correct date)
Badges are not showing up properly in PHPVMS7, I have tried changing the CSS, made sure PHPVMS7 was up to date and still have not been able to get the text-bg-info, text-bg-warning,text-bg-success to work with the IFATours Module. Have been working with Taylor on this as I'm using his apex theme he created, he suggested I open a bug report to have this investigated to see if it could be reproduced.
Version
Please enter the version
2a2602f
To Reproduce
Steps to reproduce the behavior:

  1. Go to phpvms7
  2. Goto Admin Panel
  3. Install IFATours Module, and activate. It can be downloaded from this github https://github.com/FnordLord/IfaTours
  4. Sign in to your Phpvms7 Account
  5. Click Tours & Charters
  6. See No Badges Showing Properly

Expected behavior
Badges should display correctly within the "Tours & Charters" section after the installation of modules or themes that utilize badges.
Screenshots
If applicable, add screenshots to help explain your problem.
image

Additional context
Add any other context and attach logs (from storage/logs/laravel-<date>.log)

@CaptainGodual CaptainGodual changed the title Badges not displaying properly Theme system is not detecting the module overrides Nov 29, 2024
@BossOfGames
Copy link
Contributor

I'm going to add some additional context for this, since I was helping to debug this with him.

I did breakpoint the code on his hosting provider to verify a few things, and did find that the module was, in fact, recognizing the theme was there.

We also tried this with one of my modules, which we know these overrides work fine, and phpvms didn't recognize the extra blade files in the theme (I distribute modified files as part of Apex, and I know this works).

Basically, it appears that under specific hosting environments, code that is used to override a 3rd party module's blade files from a theme (I.e. the theme's modules directory) is not being properly loaded.

I have another VA I'm setting up on this hosting provider next week. If I encounter issues with that site, then we'll get in touch with them to investigate further. Then, we'll have a better idea of the impact of this issue.

@FatihKoz
Copy link
Contributor

What makes you think that this is a phpvms issue then ?

Apex is a 3rd party theme, said module is also 3rd party and you say that you know it works on some others.

Can't this be some server side setting or worse some 3rd party module service provider issue ?

I suspect module's view path definitions, like they are not being loaded properly.

But this still makes them a module problem, not a phpvms v7 issue.

@BossOfGames
Copy link
Contributor

I did breakpoint those. Doesn't work. Including your modules.

Everything works fine on my hosts. I think this is a hosting provider issue, but I'm wondering if it's an issue caused by an edge case in phpvms that's not properly accounted for.

That still needs to be resolved.

Anyways it's the holiday weekend, so I won't be able to figure this out until next week.

@FatihKoz
Copy link
Contributor

Good luck then :) And have a nice holiday.

https://github.com/FnordLord/IfaTours/blob/main/Providers/IfaToursServiceProvider.php#L115-L133

https://github.com/cardinalhorizon/CHEvents/blob/main/Providers/AppServiceProvider.php#L68-L84

https://github.com/FatihKoz/DisposableBasic/blob/main/Providers/DB_ServiceProvider.php#L168-L185

It can be an open_basedir issue on some hosts, which makes this a server/php settings issue.

PS: I would kindly suggest you to remove the is_dir() and file_exists() as they can cause problems on some servers too.

@FatihKoz
Copy link
Contributor

And I would suggest users of this module should inform the developer directly, instead of opening an issue here.

https://github.com/FnordLord/IfaTours/blob/main/Providers/IfaToursServiceProvider.php#L124-L129

I do not think you can have custom blades without array_merge()

@CaptainGodual
Copy link
Author

May I speak up, with due respect I was informed by Taylor to open a issue here because he thought it was a phpvms bug. So I was only doing as I was told. Based on that note, I feel like closing this issue and working it out with Taylor directly would benficial at this time. Thank you Faith for your suggestions.

@FatihKoz
Copy link
Contributor

Just for your information, default install works just fine.

image

  • Go to phpvms7 // latest dev installed with git commands
  • Install IFATours Module, and activate. // done as per developers instructions
  • Click Tours & Charters // done
  • See No Badges Showing Properly // Badges working properly

Even with a modified blade (see image above), badges load up fine. I can not reproduce the error with your steps.

If it works on your end with default theme too then this can be a 3rd party theme (Apex) issue because it also works for another 3rd party theme too. (After using bs5 compatible classes for the badges of course.)

image

Hope you can solve this with @BossOfGames on Apex Theme

@nabeelio
Copy link
Owner

Please leave this open until we can confirm it's not a phpvms bug. Iirc, we forked the theme engine so it's possible it's a bug. I'm on holiday so I'll look in detail after

@nabeelio nabeelio reopened this Nov 29, 2024
@BossOfGames
Copy link
Contributor

BossOfGames commented Nov 29, 2024

Just for your information, default install works just fine.

image

  • Go to phpvms7 // latest dev installed with git commands
  • Install IFATours Module, and activate. // done as per developers instructions
  • Click Tours & Charters // done
  • See No Badges Showing Properly // Badges working properly

Even with a modified blade (see image above), badges load up fine. I can not reproduce the error with your steps.

If it works on your end with default theme too then this can be a 3rd party theme (Apex) issue because it also works for another 3rd party theme too. (After using bs5 compatible classes for the badges of course.)

image

Hope you can solve this with @BossOfGames on Apex Theme

It works on my theme. I think I alluded to that in my original post above, but I will re-assert that here. In fact, I have plans on distributing the modified theme files with Apex in the next update.

Screenshot 2024-11-30 at 00 23 08

When copying and pasting and giving him my exact working files for Apex, it doesn't work on his setup. That's where we're at right now.

Granted, his setup is doing something a bit special, which I have yet to try locally (and just realized it before going to bed). I instructed him to extend the Apex theme with his own copy so that it makes his upgrading life easier when custom changes happen. He threw the modified files into the extended theme, and is loading that. There might be a possibility that, with that setup, that could be a edge case that's not covered.

Edit: Nope, that wasn't it. Just tried that and it worked fine on my local install.

This is looking to be more and more related to the hosting environment, and a nasty edge case at that.

@nabeelio I'm also on vacation with a busy weekend. I am also planning on figuring this out when I get back, and I'll report my findings if you don't beat me to figuring it out.

@FatihKoz
Copy link
Contributor

FatihKoz commented Nov 29, 2024

Hi @BossOfGames , i am not sure what is going on on that server but I am extending my own theme and advising people to extend DispoTheme for their own edits/customizations.

So I do not think that extending a theme will be the issue here (when it is done properly)

I wish you a nice holiday and good luck during digging this out. If you need any php or server settings needs to be tested (to reproduce), I will be happy to help though :)

@nabeelio
Copy link
Owner

Have you tried turning on E_NOTICE and turning up error reporting?

@BossOfGames
Copy link
Contributor

BossOfGames commented Nov 30, 2024

Have you tried turning on E_NOTICE and turning up error reporting?

I just tried that. Nothing is coming back. I have verified this issue is affecting multiple customers of the hosting company, and I have now roped in their support/admins to get help on this.

@BossOfGames
Copy link
Contributor

BossOfGames commented Dec 2, 2024

@nabeelio @FatihKoz I have a update for y'all.

I applied logging to Laravel's view loading code to get these log outputs, but I think I figured out the issue.

So, it does appear that the bug may be related to my modules. Upon checking DisposableBasic and Special modules, they appear to be loading correctly. However, my modules are not loading correctly on this host.

For some reason my module code appears to be treating the path as follows:

[2024-12-02 10:57:39] production.DEBUG: Checking Path: /home/deltavi5/dv_phpvms_production/resources/views/layouts/dv_2024/modules/CHEvents/Providers/../Resources/views  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/index.blade.php  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/index.php  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/index.css  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/index.html  
[2024-12-02 10:57:39] production.DEBUG: Checking Path: /home/deltavi5/dv_phpvms_production/resources/views/layouts/ch_apex/modules/CHEvents/Providers/../Resources/views  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/index.blade.php  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/index.php  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/index.css  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/index.html  
[2024-12-02 10:57:39] production.DEBUG: Checking Path: /home/deltavi5/dv_phpvms_production/modules/CHEvents/Providers/../Resources/views  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/index.blade.php  
[2024-12-02 10:57:39] production.DEBUG: Checking Path: /home/deltavi5/dv_phpvms_production/vendor/laracasts/flash/src/Laracasts/Flash/../../views  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: message.blade.php  
[2024-12-02 10:57:39] production.DEBUG: Checking Path: /home/deltavi5/dv_phpvms_production/resources/views/layouts/dv_2024/modules/CHEvents/Providers/../Resources/views  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/table.blade.php  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/table.php  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/table.css  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/table.html  
[2024-12-02 10:57:39] production.DEBUG: Checking Path: /home/deltavi5/dv_phpvms_production/resources/views/layouts/ch_apex/modules/CHEvents/Providers/../Resources/views  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/table.blade.php  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/table.php  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/table.css  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/table.html  
[2024-12-02 10:57:39] production.DEBUG: Checking Path: /home/deltavi5/dv_phpvms_production/modules/CHEvents/Providers/../Resources/views  
[2024-12-02 10:57:39] production.DEBUG: Checking if file exists: frontend/table.blade.php  

As you can see from the above code, the interesting part is that the view folder the module is returning is CHEvents/Providers/../Resources/views on the end of the theme's module folder. That certainly ain't right.

It's very important to note that I use the module generator (php artisan module:make {ModuleName}) to create new modules, so if it's the module itself, then there's a good chance that the module boilerplate needs to be updated.

Right now, DisposableBasic's view loading code is as follows:

    public function registerViews()
    {
        $viewPath = resource_path('views/modules/DisposableBasic');
        $sourcePath = __DIR__ . '/../Resources/views';

        $this->publishes([$sourcePath => $viewPath,], 'views');

        $this->loadViewsFrom(array_merge(array_map(function ($path) {
            return $path . '/modules/DisposableBasic';
        }, \Config::get('view.paths')), [$sourcePath]), 'DBasic');
    }

The boilerplate I'm getting with generating a new module uses the following:

    public function registerViews()
    {
        $viewPath = resource_path('views/modules/chevents');
        $sourcePath = __DIR__.'/../Resources/views';

        $this->publishes([$sourcePath => $viewPath], 'views');

        $this->loadViewsFrom(array_merge(array_filter(array_map(function ($path) {
            $path = str_replace('default', setting('general.theme'), $path);
            // Check if the directory exists before adding it
            if (file_exists($path.'/modules/chevents') && is_dir($path.'/modules/chevents')) {

                return $path.'/modules/chevents';
            }

            return null;
        }, \Config::get('view.paths'))), [$sourcePath]), 'chevents');
    }

I'm going to implement Dispo's version of the code and see if that rectifies the issue. If so, we'll need to update the module boilerplate and let other devs know of this.

EDIT: Confirmed, it is the boilerplate.

@FatihKoz
Copy link
Contributor

FatihKoz commented Dec 4, 2024

I remember having some problems back in time, when we were first working on that bit of code for customizations, an experienced Symfony developer assisted me and found the proper way, since then had no problems (except those is_directory or file_exists checks)

And by the way, your example referring DisposableBasic is not recent @BossOfGames (probably you are working with an old version or a somehow modified version), I linked the recent code bit above before, here it is again.

    public function registerViews()
    {
        $viewPath = resource_path('views/modules/DisposableBasic');
        $sourcePath = __DIR__ . '/../Resources/views';

        $this->publishes([$sourcePath => $viewPath,], 'views');

        $this->loadViewsFrom(array_merge(array_map(function ($path) {
            return str_replace('default', setting('general.theme'), $path) . '/modules/DisposableBasic';
        }, \Config::get('view.paths')), [$sourcePath]), 'DBasic');
    }

That str_replace() is/was needed as you know ;)

Only difference I can see is; I am using array map then array merge you are using array map then array filter then array merge, rest is same.

https://www.php.net/manual/en/function.array-filter.php

As you did not defined a callback for the array_filter(), it is simply eliminating empty entries. Therefore I still think that the problem lies within your below code and server returns null for some reason, causing your path not being added to the array.

            // Check if the directory exists before adding it
            if (file_exists($path.'/modules/chevents') && is_dir($path.'/modules/chevents')) {

                return $path.'/modules/chevents';
            }

            return null;

If that additional file and directory checks are provided by phpVMS when module creation commands are used then yes, it needs to removed (as they are the root cause of errors for some servers)...

In any case developers should be informed, I agree on that.

@arthurpar06
Copy link
Contributor

Yes, I added those checks to ensure the directory exists before adding it to the path, as this was preventing some errors when trying to cache the views.

I remember discussing this with Dispo a while ago; we couldn't understand why those two weren't working on some hosting providers. Dispo decided to remove them from his modules, but I forgot to update the boilerplate. I'll do some research tonight to see if I can figure out why it's not working. Otherwise, the best course of action is probably to revert the boilerplate.

@BossOfGames
Copy link
Contributor

Yes, I added those checks to ensure the directory exists before adding it to the path, as this was preventing some errors when trying to cache the views.

I remember discussing this with Dispo a while ago; we couldn't understand why those two weren't working on some hosting providers. Dispo decided to remove them from his modules, but I forgot to update the boilerplate. I'll do some research tonight to see if I can figure out why it's not working. Otherwise, the best course of action is probably to revert the boilerplate.

Check my logs in my post. It appears the issue is that the directory that's being sent to Laravel's code is not good. Look very closely at it and you'll see the problem.

@nabeelio
Copy link
Owner

nabeelio commented Dec 5, 2024

@BossOfGames how is this installed? Which method? This path does seem suspicious:

/resources/views/layouts/ch_apex/modules/CHEvents/Providers/../Resources/views

IIRC, it should just be:

/modules/CHEvents/Providers/../Resources/views

Since it needs to find the views from the module folder, they shouldn't need to be copied over to the main view (unless it's a theme-specific override).

@nabeelio
Copy link
Owner

nabeelio commented Dec 5, 2024

As you did not defined a callback for the array_filter(), it is simply eliminating empty entries. Therefore I still think that the problem lies within your below code and server returns null for some reason, causing your path not being added to the array.

It's not really anything he's doing, I think the module generator stubs need to be updated to match essentially what you're doing. Or there's just another bug elsewhere in the pathing

@FatihKoz
Copy link
Contributor

FatihKoz commented Dec 5, 2024

As you did not defined a callback for the array_filter(), it is simply eliminating empty entries. Therefore I still think that the problem lies within your below code and server returns null for some reason, causing your path not being added to the array.

It's not really anything he's doing, I think the module generator stubs need to be updated to match essentially what you're doing. Or there's just another bug elsewhere in the pathing

We still need to understand why those file_exists and is_dir functions fail to work on some servers.

Even if a module is installed properly and constructed properly, including namespaces, definitions, providers etc. those two php functions do return false on some servers, thus not returning a path for the views, and as a result module blade customizations via a theme fails.

Back in time I suspected the php's open_basedir and apache config, but was not able to find a sweet spot to pinpoint the real cause. This is why I removed those checks from my modules. To be honest, I was busy enough and did not worked on this later.

As far as I figured out, Plesk Panel (default/basic settings) on a VPS have this issue, similarly Plesk on a shared host can also cause problems (I saw one working fine but another one was failing). Also on a cPanel I saw both working and not working installs.

This the reason I concluded that this really depends on how the server is configured and how the access to paths is granted to users.

Right now, as Arthur mentioned above, reverting back the boilerplate will help.

@nabeelio
Copy link
Owner

nabeelio commented Dec 5, 2024

Is it possible to get the phpinfo for the broken server? Instead of using those functions, we might have to use a Symfony function which might have some extra code to account for differences with the OS and other security settings. I'll try to do some research around that

@FatihKoz
Copy link
Contributor

FatihKoz commented Dec 5, 2024

I can provide you a phpinfo from Plesk VPS (where it fails too)

@BossOfGames
Copy link
Contributor

BossOfGames commented Dec 5, 2024

@BossOfGames how is this installed? Which method? This path does seem suspicious:

/resources/views/layouts/ch_apex/modules/CHEvents/Providers/../Resources/views

IIRC, it should just be:

/modules/CHEvents/Providers/../Resources/views

Since it needs to find the views from the module folder, they shouldn't need to be copied over to the main view (unless it's a theme-specific override).

My module itself is installed in the correct directory, /modules/CHEvents.

You can see that on my repo https://github.com/cardinalhorizon/CHEvents

When I do the theme overrides, the views are loaded in /resources/views/layouts/ch_apex/modules/CHEvents with my modification of files found in the Resources/views folder.

Screenshot 2024-12-05 at 20 48 34

The interesting thing, and the reason why I pointed out the weird directory structure, is that on my local computer and a server that's been setup from scratch with php fpm and the other goodies, the above setup works fine. On a restrictive host, i get /resources/views/layouts/ch_apex/modules/CHEvents/Providers/../Resources/views for the pathing.

@BossOfGames
Copy link
Contributor

Is it possible to get the phpinfo for the broken server? Instead of using those functions, we might have to use a Symfony function which might have some extra code to account for differences with the OS and other security settings. I'll try to do some research around that

I wouldn't use the word broken here, just very strict on how it handles files in the hosting env.

Anyways, here's a dump:
phpinfo.html.zip

@arthurpar06
Copy link
Contributor

arthurpar06 commented Dec 5, 2024

Is it possible to get the phpinfo for the broken server? Instead of using those functions, we might have to use a Symfony function which might have some extra code to account for differences with the OS and other security settings. I'll try to do some research around that

Here’s what I found after a quick search:
This is from the Laravel documentation and needs to be tested. I'm not sure if it will work because we might need to specify a "disk." A "disk" only refers to where files are stored (s3, public/uploads etc), not the main application path.

Otherwise, this is a Symfony function that might work.

@FatihKoz
Copy link
Contributor

FatihKoz commented Dec 6, 2024

file_exists($path) returns true
is_dir($path) returns false
is_file($path) returns false

Using only file_exists($path) in the method fails to load customized blade path too (which is also strange)

And honestly I do not think that adding a new disk to Laravel's filesystem will help, as it will use the same functions listed above in the background when we will try to check.

Which server config blocks this is still a mystery 😸

@nabeelio
Copy link
Owner

nabeelio commented Dec 6, 2024

IMO, just change the code to skip the directory check:

$this->loadViewsFrom(array_merge(array_filter(array_map(function ($path) {
    $path = str_replace('default', setting('general.theme'), $path);
    return $path.'/modules/chevents';
}, \Config::get('view.paths'))), [$sourcePath]), 'chevents');

If the directory doesn't exist, it implies there are no views, so none would be called. Laravel does checks internally, anyway. Might be better for that to get exposed.

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

5 participants