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

Refactor: Change docroot to public to align with Symfony's best practice structure #101

Open
wants to merge 4 commits into
base: 6.x
Choose a base branch
from

Conversation

matbcvo
Copy link

@matbcvo matbcvo commented Jan 28, 2025

This PR updates the project structure to align with Symfony's best practices by renaming the docroot directory to public. This change ensures consistency with Symfony's default directory structure, as outlined in the Symfony Best Practices documentation.

Fixes #6

Changes

  • Updated composer.json to reference public instead of docroot.
  • Updated README.md to replace all references to docroot with public.

A PR needs to be created in the mautic/user-documentation repository to update the Composer installation steps related to the web root for the 6.x version.

@RCheesley
Copy link
Member

cc @mautic/education-team-leaders for when you've made the 6.x branch - let us know :)

Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

I see no issues from the code change perspecitve. Will this work for people upgrading from Mautic 5?

@RCheesley
Copy link
Member

I guess we would have to do some magic in the upgrade process to move/rename the folder, and/or make it very clear about it being a breaking change? I don't know why it was called docroot in the first place, but I suspect it's because the whole composer process took a lot of inspiration from Drupal's implementation, which I believe uses docroot.

@matbcvo
Copy link
Author

matbcvo commented Jan 29, 2025

I don’t see any reason why it wouldn’t work for users upgrading from Mautic 5.

Users upgrading from Mautic 5 have two choices:

  • Keep the web root set to docroot/ (or another directory, depending on the hosting provider) in the composer.json file, or
  • Switch the web root from docroot/ to public/ using the instructions provided in the README.md file (these instructions will also be included in the user documentation).

When a user upgrading from Mautic 5 replaces the entire composer.json file with the newer version, the web root will change from docroot/ to public/ unless the user manually changes it back. The user documentation should highlight this web root change so users know what actions they need to take - either revert the web root to docroot/ in the composer.json file or proceed with the switch to public/ using the provided instructions.

@matbcvo
Copy link
Author

matbcvo commented Jan 29, 2025

I could add a Composer script that updates the web root from docroot/ to public/ in the composer.json file and renames the web root directory accordingly. Users could then run this script (eg, composer migrate-webroot) during the upgrade process if they choose to switch to public/. I don’t think it would be ideal to include this in the composer update command, as some users might prefer to keep docroot/.

@escopecz
Copy link
Member

That would be helpful. Is the docroot present also in config/local.php file when Mautic is installed via Composer?

@matbcvo
Copy link
Author

matbcvo commented Jan 29, 2025

Config parameters containing docroot/, which are related to the media directory, appear in config/local.php when saving the configuration after installing Mautic via Composer. After changing the web root and re-saving the configuration, those paths are not overwritten. So, they need to be manually updated in config/local.php. This can also be done using a Composer script.

<?php
$parameters = array(
	// ...
	'upload_dir' => '/var/www/vhosts/test.example.com/docroot/media/files',
	// ...
	'form_upload_dir' => '/var/www/vhosts/test.example.com/docroot/media/files/form',
	// ...
	'contact_export_dir' => '/var/www/vhosts/test.example.com/docroot/media/files/temp',
	// ...
	'report_temp_dir' => '/var/www/vhosts/test.example.com/docroot/media/files/temp',
	// ...
);

@matbcvo matbcvo force-pushed the web-root branch 4 times, most recently from c827946 to 3155248 Compare February 1, 2025 10:45
composer script for migrating web root

Update README.md
README.md Outdated

This project supports PHP 7.4 as minimum version, however it's possible that a `composer update` will upgrade some package that will then require PHP 7+ or 8+.
This project supports PHP 8.1 as minimum version, however it's possible that a `composer update` will upgrade some package that will then require PHP 7+ or 8+.
Copy link
Member

Choose a reason for hiding this comment

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

When in it, can we also drop the mention of PHP 7?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it makes sense to drop it. Done.

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

Successfully merging this pull request may close these issues.

3 participants