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

AppKernel is now able to detect the admin folder #35417

Merged
merged 1 commit into from Feb 20, 2024

Conversation

jolelievre
Copy link
Contributor

@jolelievre jolelievre commented Feb 19, 2024

Questions Answers
Branch? develop
Description? AppKernel is now able to detect the admin folder in case the PS_ADMIN_DIR is not defined, the admin dir path is now set in the prestashop.admin_dir parameter that must be favored from now on in DI
Type? improvement
Category? CO
BC breaks? no
Deprecations? no
How to test? CI and UI tests green, see description below for manual checks
UI Tests https://github.com/jolelievre/ga.tests.ui.pr/actions/runs/7962314884
Fixed issue or discussion? Fixes #35354
Related PRs ~
Sponsor company ~

Steps to reproduce

  1. Install the nightly with a folder admin-dev present (if you rename it prior installation, it will fail)
  2. Access your back office. It should load
  3. Rename admin-dev as anything you want
  4. Access your back ffice. It should load

Alternative:

You can use ./bin/console debug:container --parameters and check that prestashop.admin_dir and prestashop.admin_folder_name are correctly updated when the admin folder is renamed In dev environment it should be automatic thanks to the resource checking, but in prod you probably have to clean the cache after every modification

@jolelievre jolelievre requested a review from a team as a code owner February 19, 2024 15:46
@prestonBot prestonBot added develop Branch Improvement Type: Improvement labels Feb 19, 2024
…DIR is not defined, the admin dir path is now set in the prestashop.admin_dir parameter that must be favored from now on in DI
foreach ($finder as $adminIndexFile) {
$adminDir = $adminIndexFile->getPath();
// Container freshness depends on this file existence
$container->addResource(new FileExistenceResource($adminIndexFile->getRealPath()));
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, I didn't know about this FileExistenceResource feature

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep me neither, just found out about it!

@jolelievre jolelievre changed the title AppKernel is now able to detect the admin folder in case the PS_ADMIN_… AppKernel is now able to detect the admin folder Feb 19, 2024
@ps-jarvis ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Feb 19, 2024
@FabienPapet
Copy link
Member

Less coupling to legacy constants, nice !

@Progi1984 Progi1984 added this to the 9.0.0 milestone Feb 20, 2024
@matthieu-rolland
Copy link
Contributor

matthieu-rolland commented Feb 20, 2024

@jolelievre I guess this can be merged as long as CI and ui tests are fine ?

there's only one UI test failing, but it doesn't look like it's related to your PR

@jolelievre
Copy link
Contributor Author

@FabienPapet yep, step by step 😅
I'll propose a refacto PR after this one is merged to clean the modern code

Comment on lines +233 to +261
// Define parameter for admin folder path
if (defined('PS_ADMIN_DIR') && is_dir(PS_ADMIN_DIR)) {
$adminDir = PS_ADMIN_DIR;
} elseif (defined('_PS_ADMIN_DIR_') && is_dir(_PS_ADMIN_DIR_)) {
$adminDir = _PS_ADMIN_DIR_;
} else {
// Look for potential admin folders, condition to meet:
// - first level folders in the project folder
// - contains a PHP file that define the const PS_ADMIN_DIR or _PS_ADMIN_DIR_
// - the first folder found is used (alphabetical order, but files named index.php have the highest priority)
$finder = new Symfony\Component\Finder\Finder();
$finder->files()
->name('*.php')
->contains('/define\([\'\"](_)?PS_ADMIN_DIR(_)?[\'\"]/')
->depth('== 1')
->sort(function (SplFileInfo $a, SplFileInfo $b): int {
// Prioritize files named index.php
if ($a->getFilename() === 'index.php') {
return -1;
}

return strcmp($a->getRealPath(), $b->getRealPath());
})
->in($this->getProjectDir())
;
foreach ($finder as $adminIndexFile) {
$adminDir = $adminIndexFile->getPath();
// Container freshness depends on this file existence
$container->addResource(new FileExistenceResource($adminIndexFile->getRealPath()));
Copy link
Member

Choose a reason for hiding this comment

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

Nice @jolelievre, thanks :)
I think we should put this in a dedicated function or class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's room for a small refacto indeed, I guess it will have to be done in a dedicated PR then

Copy link
Contributor

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

Hi @jolelievre

Thank you for your PR, I tested it and it seems to works as you can see :

image

image

Link to the auto test : https://github.com/jolelievre/ga.tests.ui.pr/actions/runs/7962314884

Because the auto test is 🟢 and the PR seems to works as expected, It's QA ✔️

Thank you

@AureRita AureRita added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Feb 20, 2024
@FabienPapet FabienPapet merged commit d76505f into PrestaShop:develop Feb 20, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
develop Branch Improvement Type: Improvement QA ✔️ Status: check done, code approved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Hardcoded path in config: The "/var/www/html/admin-dev/themes/new-theme" directory does not exist
9 participants