Skip to content

Conversation

@huzaifaarain
Copy link

@huzaifaarain huzaifaarain commented Oct 8, 2024

Refactor the code as per Laravel best practices

After building couple of applications using this template I came across couple of issues and I have tried to address those in the PR to the best of my knowledge.

Using env directly in the service provider is not recommended. All env values should be resolved via config.

Decoupled shopify related business logic from AppServiceProvider, extract the logic into its own service provider ShopifyServiceProvider

Implementing business logic directly into the web.php is not recommended. I have also organized the web.php file by moving the business logic to its own namespace and relevant controllers.

@huzaifaarain
Copy link
Author

I have signed the CLA!

dharanikumar07 pushed a commit to dharanikumar07/survey-app that referenced this pull request Nov 15, 2025
…, config)

- Add `app/Providers/ShopifyServiceProvider.php` to initialize Shopify Context,
  force HTTPS, and register webhooks (incl. privacy topics)
- Slim `app/Providers/AppServiceProvider.php` by removing Shopify-specific logic
- Centralize Shopify envs in `config/shopify.php` (`api_key`, `api_secret`,
  `scopes`, `custom_domains`, `api_version`) and use `config()` at runtime
- Register `App\Providers\ShopifyServiceProvider` in `config/app.php`
- Replace route closures with controllers:
  - `app/Http/Controllers/AuthController.php` for `/api/auth` and `/api/auth/callback`
  - `app/Http/Controllers/WebhookController.php` for `/api/webhooks`
  - `app/Http/Controllers/SpaController.php` as `Route::fallback`
- Update `routes/web.php` to reference new controllers
- Delete unused `app/Http/Controllers/ShopifyOAuthController.php`

BREAKING CHANGE: removed `/api/products` and `/api/products/count` endpoints (by request).

Inspired by Shopify template refactor: Shopify/shopify-app-template-php#524
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.

1 participant