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

Feature/brands #85

Open
wants to merge 7 commits into
base: hybrid-runway
Choose a base branch
from
Open

Feature/brands #85

wants to merge 7 commits into from

Conversation

BobWez98
Copy link
Collaborator

  • Refactored the brands import
  • Allow a Hybrid runway collection to have a route
  • Structured blueprints of runway resources
  • Removed old import buttons

config/rapidez/statamic.php Outdated Show resolved Hide resolved
config/rapidez/statamic.php Outdated Show resolved Hide resolved
config/rapidez/statamic/builder.php Outdated Show resolved Hide resolved
resources/views/page_builder/content.blade.php Outdated Show resolved Hide resolved
resources/views/utilities/import_utility/imports.blade.php Outdated Show resolved Hide resolved
foreach (Brand::get() as $brand) {
$extraData = [];

if (Schema::hasTable('amasty_amshopby_option_setting')) {
Copy link
Member

Choose a reason for hiding this comment

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

We need this documented somewhere, so we do support Amasty Brands out of the box?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have some doubts if we even want this anymore. Let's discuss internally.

Comment on lines +10 to +18
public static function handle(): string
{
return 'base';
}

public function title(): string
{
return __('Base');
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe not specify these as you want these to be required?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sadly we have to. The baseCollection of Statamic builder has abstract methods, this requires us to implement it. Can't do it like public function title(): string; either cause it expects an implementation.

src/Contracts/ImportsBrands.php Outdated Show resolved Hide resolved
src/RapidezStatamicServiceProvider.php Outdated Show resolved Hide resolved
src/RapidezStatamicServiceProvider.php Outdated Show resolved Hide resolved
@BobWez98 BobWez98 requested a review from royduin October 31, 2024 14:37
@@ -37,7 +37,7 @@
"statamic-rad-pack/runway": "^7.11",
"statamic/cms": "^5.0",
"statamic/eloquent-driver": "^4.9",
"tdwesten/statamic-builder": "^1.0",
"tdwesten/statamic-builder": "dev-main",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Put it on dev-main for now, there's no release yet but i added tdwesten/statamic-builder#37

Copy link
Member

Choose a reason for hiding this comment

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

I suggest flipping this function around for readability.

Could you add a comment to the function on why we cannot do the entry query when running in console

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.

4 participants