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

Load Magento urls from website/default base_url if enabled #567

Merged
merged 14 commits into from
Dec 3, 2024

Conversation

indykoning
Copy link
Member

This allows custom magento backend urls
Especially useful for multistore multidomain setups.
Where you may want to obfuscate that different sites belong to the same Magento backend.

The config system is also updated to be functionally compatible with default Magento config functions.
Now requesting default config will give you only config from default
Requesting website config will give you config from the website, and falls back to default
Requesting store config will give you the config from the store, falling back to the website, and then default.

@royduin
Copy link
Member

royduin commented Sep 11, 2024

Looking good! 3 things

  1. Is it fully backwards compatible?
  2. We need some docs on this
  3. See internal RAP-710, maybe something to combine with this? Or first merge this and refactor later?

@royduin
Copy link
Member

royduin commented Sep 24, 2024

@indykoning 😇

@indykoning
Copy link
Member Author

The config method is fully backwards compatible, all original functions still exist.
@config and Rapidez::config will not be deprecated. The new config method is for more advanced usage like getting the urls from Website level and up, instead of store level and up.

docs: rapidez/docs#51

Perhaps we could add a config setting to define which caching driver is used for these caches, wether it's the array driver, redis, default.

config/rapidez/system.php Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
src/Models/OptionValue.php Outdated Show resolved Hide resolved
src/Models/Store.php Outdated Show resolved Hide resolved
src/Models/Config.php Outdated Show resolved Hide resolved
@@ -10,6 +10,9 @@
// Elasticsearch prefix.
'es_prefix' => env('ELASTICSEARCH_PREFIX', 'rapidez'),

// Get Magento url from Database
'magento_url_from_db' => env('GET_MAGENTO_URL_FROM_DATABASE', false),
Copy link
Member

Choose a reason for hiding this comment

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

Pros/cons of having this true/false by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

Pro for false:

  • no breaking changes by default, only when you opt into this functionality will it behave differently
  • after upgrading you have the time to first get your database in order before enabling this feature.
    Pro for true:
  • You get a much better experience with url handling out of the box since you no longer need to define the urls in Rapidez itself with subfolders working

If your database has the correct urls by default, and you don't have them set using env variables or config.php.
But this is an assumption we'd have to make when defaulting it to true

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! Just one thing left; documenting this option here: https://docs.rapidez.io/3.x/configuration.html#base-url

@royduin
Copy link
Member

royduin commented Nov 27, 2024

What about the Rapidez url? Should we set that from a Magento config? Could be useful for multisites. Currently multisites do work; as with the url() helper the request will be used to determinate the base, but that's not working within the console; config('app.url') != url('/'). For those situations the root will be forced with code like this:

Event::listen('rapidez:store-set', function ($store) {
    $baseUrl = config('rapidez.url.' . $store['code']);
    if ($baseUrl) {
        URL::forceRootUrl($baseUrl);
        config()->set('frontend.base_url', url('/'));
    }
});

Reference: #398. Maybe we should have URL::forceRootUrl() somewhere in the core?


Having a forced url could be useful for: rapidez/blade-directives#18

@indykoning indykoning changed the base branch from master to 2.x December 3, 2024 14:47
@indykoning
Copy link
Member Author

What about the Rapidez url? Should we set that from a Magento config? Could be useful for multisites. Currently multisites do work; as with the url() helper the request will be used to determinate the base, but that's not working within the console; config('app.url') != url('/'). For those situations the root will be forced with code like this:

Event::listen('rapidez:store-set', function ($store) {
    $baseUrl = config('rapidez.url.' . $store['code']);
    if ($baseUrl) {
        URL::forceRootUrl($baseUrl);
        config()->set('frontend.base_url', url('/'));
    }
});

Reference: #398. Maybe we should have URL::forceRootUrl() somewhere in the core?

Having a forced url could be useful for: rapidez/blade-directives#18

I've added this as well so as long as your store url is not the same as the website url then it will use forceRootUrl to set the url.

@indykoning
Copy link
Member Author

indykoning commented Dec 3, 2024

PS, since 2.x is no longer on master i've updated the branch #668

@royduin royduin merged commit 66d519c into 2.x Dec 3, 2024
15 checks passed
@royduin royduin deleted the feature/load-magento-url-from-website-2.x branch December 3, 2024 15:36
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.

2 participants