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.

Sign up for GitHub

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

WIP: Laravel upgrade 5.8 #760

Closed
wants to merge 10 commits into from
+211 −70
Closed

WIP: Laravel upgrade 5.8 #760

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1cee927
Composer bumped
dakshhmehta Aug 2, 2020
28c2aea
Test refactor for testbench 3.8 compatibility
dakshhmehta Aug 2, 2020
f1c1083
Workshop module test refactor for testbench 3.8 compatibility
dakshhmehta Aug 2, 2020
79fe076
User module test refactored for testbench 3.8 compatibility
dakshhmehta Aug 2, 2020
c582ce2
Translation module test refactored for testbench 3.8 compatibility
dakshhmehta Aug 2, 2020
ec9dc36
Tag module test refactored to testbench 3.8 compatibility
dakshhmehta Aug 2, 2020
170d76e
Page module test refactored for testbench 3.8 compatibility
dakshhmehta Aug 2, 2020
6ee8766
Setting/Menu test refactored for testbench 3.8 compatibility. But bot…
dakshhmehta Aug 2, 2020
d4ed865
Media test refactored for testbench 3.8 compatibility. Failing lots o…
dakshhmehta Aug 2, 2020
cfbbc20
Reverted composer.json and phpunit to its appropriate code
dakshhmehta Aug 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/node_modules
/public/hot
/public/storage
/public/modules/*
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, again when we publish module, it just copied file from the module assets to public, we dont need I guess. Take a final call and I will remove it if there is intention to copy assets from modules to public.

/storage/*.key
/vendor
Homestead.json
Expand Down
9 changes: 9 additions & 0 deletions Modules/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Activity/
Copy link
Member

Choose a reason for hiding this comment

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

Please don't commit this file.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I will remove this and modified compose.json before committing finally because these are our internal companies module. I will adjust it.

Copy link
Member

Choose a reason for hiding this comment

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

yeah no worries, I guess as much 😄
For your ease, it might be easier to have a separate asgardcms installation only for the contributions.
That way you don't risk mixing both together by accident.

Contact/
CourierTracking/
Faq/
Location/
Rarv/
StationaryPrint/
Task/
Transaction/
4 changes: 2 additions & 2 deletions Modules/Core/Tests/Asset/AsgardAssetManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ class AsgardAssetManagerTest extends BaseTestCase
*/
private $assetManager;

public function setUp()
protected function setUp():void
{
parent::__construct();
parent::setUp();
$this->assetManager = new AsgardAssetManager();
}

Expand Down
6 changes: 3 additions & 3 deletions Modules/Core/Tests/Asset/AsgardAssetPipelineTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ class AsgardAssetPipelineTest extends BaseTestCase
/**
*
*/
public function setUp()
protected function setUp():void
{
parent::__construct();
$this->refreshApplication();
parent::setUp();

$this->assetPipeline = new AsgardAssetPipeline($this->app['Modules\Core\Foundation\Asset\Manager\AssetManager']);
$this->assetManager = $this->app['Modules\Core\Foundation\Asset\Manager\AssetManager'];
}
Expand Down
6 changes: 3 additions & 3 deletions Modules/Core/Tests/Asset/AssetFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ class AssetFactoryTest extends BaseTestCase
*/
private $assetFactory;

public function setUp()
protected function setUp():void
{
parent::__construct();
$this->refreshApplication();
parent::setUp();

$this->assetFactory = app(AssetTypeFactory::class);
}

Expand Down
2 changes: 1 addition & 1 deletion Modules/Core/Tests/BaseTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ abstract class BaseTestCase extends TestCase
{
protected $app;

public function setUp()
protected function setUp():void
{
parent::setUp();
$this->refreshApplication();
Expand Down
2 changes: 1 addition & 1 deletion Modules/Core/Tests/TestBrowserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

abstract class TestBrowserTest extends BaseTestCase
{
public function setUp()
protected function setUp():void
{
parent::setUp();

Expand Down
2 changes: 1 addition & 1 deletion Modules/Core/Tests/Theme/ThemeManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class ThemeManagerTest extends BaseTestCase
/**
*
*/
public function setUp()
protected function setUp():void
{
parent::setUp();

Expand Down
2 changes: 1 addition & 1 deletion Modules/Core/Tests/Theme/ThemeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class ThemeTest extends BaseTestCase
*/
protected $theme;

public function setUp()
protected function setUp():void
{
parent::setUp();

Expand Down
10 changes: 5 additions & 5 deletions Modules/Core/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@
"require": {
"php": "^7.1.3",
"composer/installers": "~1.0",
"nwidart/laravel-modules": "^4.0",
"laravelcollective/html": "5.7.*",
"nwidart/laravel-modules": "^5.0",
"laravelcollective/html": "5.8.*",
"laracasts/presenter": "~0.2",
"dimsav/laravel-translatable": "~9.0",
"mcamara/laravel-localization": "~1.3",
"dimsav/laravel-translatable": "~10.0",
"mcamara/laravel-localization": "~1.5",
"floatingpoint/stylist": "~0.5",
"maatwebsite/laravel-sidebar": "~2.1",
"yajra/laravel-datatables-oracle": "~8.0",
"yajra/laravel-datatables-oracle": "~9.0",
"tightenco/ziggy": "~0.4"
},
"require-dev": {
Expand Down
6 changes: 3 additions & 3 deletions Modules/Menu/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@
"require": {
"php": "^7.1.3",
"composer/installers": "~1.0",
"nwidart/laravel-menus": "^3.0",
"nwidart/laravel-menus": "^4.0",
"typicms/nestablecollection": "1.1.12"
},
"require-dev": {
"phpunit/phpunit": "~7.0",
"orchestra/testbench": "3.6.*",
"phpunit/phpunit": "~7.5",
"orchestra/testbench": "3.8.*",
"fzaninotto/faker": "1.4.0",
"doctrine/dbal": "~2.5",
"idavoll/page-module": "4.0.x-dev"
Expand Down
2 changes: 1 addition & 1 deletion Modules/Workshop/Tests/BaseTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

abstract class BaseTestCase extends TestCase
{
public function setUp()
protected function setUp():void
{
parent::setUp();
}
Expand Down
4 changes: 2 additions & 2 deletions Modules/Workshop/Tests/ModuleScaffoldTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ModuleScaffoldTest extends BaseTestCase
*/
protected $testModuleSanitizedName;

public function setUp()
protected function setUp():void
{
$this->testModuleName = 'Testing_The-TestModule';
$this->testModuleSanitizedName = 'TestingTheTestModule';
Expand Down Expand Up @@ -104,7 +104,7 @@ private function scaffoldModule($type, $entities, $valueObjects)
->scaffold();
}

public function tearDown()
protected function tearDown():void
{
if (file_exists(__DIR__ . '/../Modules/')) {
self::delTree(__DIR__ . '/../Modules/');
Expand Down
4 changes: 2 additions & 2 deletions Modules/Workshop/Tests/ThemeScaffoldTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ThemeScaffoldTest extends BaseTestCase
*/
protected $testThemePath;

public function setUp()
protected function setUp():void
{
parent::setUp();
$this->finder = $this->app['files'];
Expand All @@ -45,7 +45,7 @@ private function generateFrontendTheme()
$this->scaffold->setName($this->testThemeName)->forType('frontend')->setVendor('asgardcms')->generate();
}

public function tearDown()
protected function tearDown():void
{
$this->finder->deleteDirectory($this->testThemePath);
$this->finder->deleteDirectory(base_path("Themes"));
Expand Down
24 changes: 18 additions & 6 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,21 @@
],
"license": "MIT",
"type": "project",
"repositories": [
{
"type": "vcs",
"url": "https://github.com/dakshhmehta/Menu.git"
},
{
"type": "vcs",
"url": "https://github.com/dakshhmehta/Core.git"
}
],
"require": {
"php": "^7.1.3",
"fideloper/proxy": "^4.0",
"laravel/framework": "5.7.*",
"laravel/framework": "5.8.*",
"laravel/tinker": "^1.0",
"nwidart/laravel-modules": "^4.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be here for auto-discovery to work. Otherwise, you have to manually load the service provider and facade for Laravel Modules in the app config.

"nwidart/laravel-modules": "^5.0",

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I guess I removed it unintentionally.

Copy link
Author

Choose a reason for hiding this comment

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

I have reverted to 4.0, 5.0 seems to be incompatible with other module or am I missing anything?

"cartalyst/sentinel": "^2.0",
"idavoll/adminlte-theme": "4.0.x-dev",
"idavoll/core-module": "4.0.x-dev",
Expand All @@ -38,11 +47,14 @@
"league/commonmark": "^0.18.3",
"league/flysystem-aws-s3-v3": "^1.0",
"mockery/mockery": "^1.0",
"nunomaduro/collision": "^2.0",
"orchestra/testbench": "3.7.*",
"phpunit/phpunit": "^7.0",
"nunomaduro/collision": "^3.0",
"orchestra/testbench": "3.8.*",
"phpunit/phpunit": "^7.5",
"predis/predis": "^1.1",
"roave/security-advisories": "dev-master"
"roave/security-advisories": "dev-master",

"tormjens/eventy": "^0.6",
"spatie/laravel-activitylog": "3.9.1"
},
"autoload": {
"classmap": [
Expand Down
4 changes: 2 additions & 2 deletions phpunit.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<testsuite name="Core">
<directory>./Modules/Core/Tests/</directory>
</testsuite>
<testsuite name="Dashboard">
<!-- <testsuite name="Dashboard">
Copy link
Member

Choose a reason for hiding this comment

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

Why are these commented?

Copy link
Author

Choose a reason for hiding this comment

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

I am refactoring each of them and uncommenting manually to make it compatible for 3.8 testbench. Mostly setUp and tearDown are protected and return void strictly.

Copy link
Author

Choose a reason for hiding this comment

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

All are done and file is reverted to its original form

<directory>./Modules/Dashboard/Tests/</directory>
</testsuite>
<testsuite name="Media">
Expand All @@ -35,7 +35,7 @@
</testsuite>
<testsuite name="User">
<directory>./Modules/User/Tests/</directory>
</testsuite>
</testsuite> -->
<testsuite name="Workshop">
<directory>./Modules/Workshop/Tests/</directory>
</testsuite>
Expand Down