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

Moves the random admin folder name generation at the end of the install process #35998

Merged
merged 9 commits into from Apr 29, 2024

Conversation

matthieu-rolland
Copy link
Contributor

@matthieu-rolland matthieu-rolland commented Apr 23, 2024

Questions Answers
Branch? develop
Description? Due to a modification in the way the path for the admin folder is set, we have a cache issue after an install in production mode, this PR moves the random admin folder name generation at the end of the install process instead of when displaying the BO login page for the first time. Both in UI and CLI install.
Type? refacto
Category? CO
BC breaks? no
Deprecations? no
How to test? 1/ Generate a release from this PR
2/ Install the release manually
3/ It should work as before.
4/ Same test with CLI install, from a release from this PR as well
UI Tests click here
Related PRs
Sponsor company PrestaShop SA

Review guide:

Before this PR, in production, the admin folder name is not generated at install, it is generated when we first display the BO login page.
With this PR, the admin folder name is generated at the end of the install process

In a nutshell, this PR dos:

  • Add an install process called "Finalization", that takes care of creating the admin folder *
  • Move the admin folder detection from AppKernel.php in a dedicated class, so that it can be used at install process

*The random admin folder name is generated at the beginning of the install process so that the name of the folder is available in the final install page, where the link is displayed, because this page is generated before the install process.

@matthieu-rolland matthieu-rolland requested a review from a team as a code owner April 23, 2024 09:55
@prestonBot prestonBot added develop Branch Refactoring Type: Refactoring labels Apr 23, 2024
tleon
tleon previously approved these changes Apr 23, 2024
boherm
boherm previously approved these changes Apr 23, 2024
@M0rgan01 M0rgan01 added this to the 9.0.0 milestone Apr 23, 2024
M0rgan01
M0rgan01 previously approved these changes Apr 23, 2024
@matthieu-rolland matthieu-rolland added the Waiting for QA Status: action required, waiting for test feedback label Apr 23, 2024
matks
matks previously approved these changes Apr 23, 2024
@matthieu-rolland matthieu-rolland removed the Waiting for QA Status: action required, waiting for test feedback label Apr 24, 2024
@matthieu-rolland matthieu-rolland dismissed stale reviews from matks, M0rgan01, boherm, and tleon via 25c8a75 April 24, 2024 09:35
jolelievre
jolelievre previously approved these changes Apr 24, 2024
Copy link
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

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

@matks matks changed the title Fix prod install mMoves the random admin folder name generation at the end of the install process Apr 25, 2024
matks
matks previously approved these changes Apr 25, 2024
jolelievre
jolelievre previously approved these changes Apr 25, 2024
Copy link
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

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

@matks matks changed the title mMoves the random admin folder name generation at the end of the install process Moves the random admin folder name generation at the end of the install process Apr 25, 2024
@matthieu-rolland
Copy link
Contributor Author

@matthieu-rolland matthieu-rolland added the Waiting for QA Status: action required, waiting for test feedback label Apr 25, 2024
AureRita
AureRita previously approved these changes Apr 25, 2024
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 @matthieu-rolland

Thank you for your PR, I tested it with you and we seen that works as expected

Link to the auto test : https://github.com/matthieu-rolland/ga.tests.ui.pr/actions/runs/8830417094

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 Apr 25, 2024
@jolelievre jolelievre dismissed stale reviews from AureRita, matks, and themself via c205667 April 26, 2024 14:53
@jolelievre
Copy link
Contributor

I made a little modification because the output in CLI didn't include the physical URI, all good now:
Capture d’écran 2024-04-26 à 16 53 32

@jolelievre jolelievre removed the QA ✔️ Status: check done, code approved label Apr 26, 2024
@matthieu-rolland
Copy link
Contributor Author

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 @matks

Thank you for your PR, I tested it and it seems to works, except the Print of the new name of the admin as you can see :

image

If this event is good for you, It'll be QA ✔️

Link to the auto test : https://github.com/matthieu-rolland/ga.tests.ui.pr/actions/runs/8850250859

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

Thank you

@AureRita AureRita added the QA ✔️ Status: check done, code approved label Apr 29, 2024
@nicosomb nicosomb merged commit 45bea4e into PrestaShop:develop Apr 29, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
develop Branch QA ✔️ Status: check done, code approved Refactoring Type: Refactoring
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

None yet

9 participants