ctf0 / Laravel-Media-Manager

A "Vuejs & Laravel" Media Manager With Tons of Features
MIT License
825 stars 178 forks source link

first changes to work with php 8 and laravel 9 #201

Closed jasperf closed 1 year ago

jasperf commented 2 years ago

Update to work in Laravel 9 and PHP 8.x . Pull request added but testing still needed.

ctf0 commented 2 years ago

u can test the changes on the demo repo

jasperf commented 2 years ago

u can test the changes on the demo repo

What repository is that? And then I guess I would need to change the composer.json to load my branch somehow.. in vcs? But then this repository for new Laravel Media Manager would also need to load other PackageChangeLog.. unless you can merge/update that one more quickly?

ctf0 commented 2 years ago

What repository is that?

i meant https://github.com/ctf0/demos/tree/media-manager

then this repository for new Laravel Media Manager would also need to load other PackageChangeLog

changes for PackageChangeLog are merged, use dev-master for the pkg version

jasperf commented 2 years ago

@ctf0 Sorry for the late response. Been really busy and several other packages are also not ready for Laravel 9. So you mean we need to update our composer.json to dev-master for this package and then composer update package and see if all is well, correct?

ctf0 commented 2 years ago

np, yes.

jasperf commented 2 years ago

Using "ctf0/media-manager": "dev-master" on a clean Laravel 9 installation still results in errors:

 Problem 1
    - ctf0/package-changelog[v1.0.0, ..., v1.x-dev, v2.x-dev] require php ~7.0 -> your php version (8.1.2) does not satisfy that requirement.
    - illuminate/support[v7.0.0, ..., v7.28.4] require php ^7.2.5 -> your php version (8.1.2) does not satisfy that requirement.
    - illuminate/support[v8.0.0, ..., v8.11.2] require php ^7.3 -> your php version (8.1.2) does not satisfy that requirement.
    - Root composer.json requires ctf0/media-manager dev-master -> satisfiable by ctf0/media-manager[dev-master].
    - ctf0/media-manager dev-master requires ctf0/package-changelog * -> satisfiable by ctf0/package-changelog[v1.0.0, ..., v1.x-dev, v2.0.0, ..., v2.x-dev].
    - Conclusion: don't install laravel/framework v9.2.0 (conflict analysis result)
    - ctf0/package-changelog[v2.0.0, ..., v2.0.4] require illuminate/support >=7.0 <9.0 -> satisfiable by illuminate/support[v7.0.0, ..., 7.x-dev, v8.0.0, ..., 8.x-dev].
    - Only one of these can be installed: illuminate/support[dev-master, v5.0.0, ..., 5.8.x-dev, v6.0.0, ..., 6.x-dev, v7.0.0, ..., 7.x-dev, v8.0.0, ..., 8.x-dev, v9.0.0-beta.1, ..., 9.x-dev], laravel/framework[v9.2.0, 9.x-dev]. laravel/framework replaces illuminate/support and thus cannot coexist with it.
    - Root composer.json requires laravel/framework ^9.2 -> satisfiable by laravel/framework[v9.2.0, 9.x-dev].

but it seems dev-master does not have the update to test that we added to pull request yet.

Using

{
    "name": "laravel/laravel",
    "type": "project",
    "description": "The Laravel Framework.",
    "keywords": ["framework", "laravel"],
    "license": "MIT",
    "repositories": [
        {
            "type": "vcs",
            "url": "git@github.com:smart48/Laravel-Media-Manager-1.git"
        }
    ],
    "require": {
        "php": "^8.0.2",
        "guzzlehttp/guzzle": "^7.2",
        "laravel/framework": "^9.2",
        "laravel/sanctum": "^2.14.1",
        "laravel/tinker": "^2.7",
        "ctf0/media-manager": "dev-master"
    },
    "require-dev": {
        "fakerphp/faker": "^1.9.1",
        "laravel/sail": "^1.0.1",
        "mockery/mockery": "^1.4.4",
        "nunomaduro/collision": "^6.1",
        "phpunit/phpunit": "^9.5.10",
        "spatie/laravel-ignition": "^1.0"
    },
    "autoload": {
        "psr-4": {
            "App\\": "app/",
            "Database\\Factories\\": "database/factories/",
            "Database\\Seeders\\": "database/seeders/"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "Tests\\": "tests/"
        }
    },
    "scripts": {
        "post-autoload-dump": [
            "Illuminate\\Foundation\\ComposerScripts::postAutoloadDump",
            "@php artisan package:discover --ansi"
        ],
        "post-update-cmd": [
            "@php artisan vendor:publish --tag=laravel-assets --ansi --force"
        ],
        "post-root-package-install": [
            "@php -r \"file_exists('.env') || copy('.env.example', '.env');\""
        ],
        "post-create-project-cmd": [
            "@php artisan key:generate --ansi"
        ]
    },
    "extra": {
        "laravel": {
            "dont-discover": []
        }
    },
    "config": {
        "optimize-autoloader": true,
        "preferred-install": "dist",
        "sort-packages": true
    },
    "minimum-stability": "dev",
    "prefer-stable": true
}

I oddly enough get the same warnings/errors:

Problem 1
    - ctf0/package-changelog[v1.0.0, ..., v1.x-dev, v2.x-dev] require php ~7.0 -> your php version (8.1.2) does not satisfy that requirement.
    - illuminate/support[v7.0.0, ..., v7.28.4] require php ^7.2.5 -> your php version (8.1.2) does not satisfy that requirement.
    - illuminate/support[v8.0.0, ..., v8.11.2] require php ^7.3 -> your php version (8.1.2) does not satisfy that requirement.
    - Root composer.json requires ctf0/media-manager dev-master -> satisfiable by ctf0/media-manager[dev-master].
    - ctf0/media-manager dev-master requires ctf0/package-changelog * -> satisfiable by ctf0/package-changelog[v1.0.0, ..., v1.x-dev, v2.0.0, ..., v2.x-dev].
    - Conclusion: don't install laravel/framework v9.2.0 (conflict analysis result)
    - ctf0/package-changelog[v2.0.0, ..., v2.0.4] require illuminate/support >=7.0 <9.0 -> satisfiable by illuminate/support[v7.0.0, ..., 7.x-dev, v8.0.0, ..., 8.x-dev].
    - Only one of these can be installed: illuminate/support[dev-master, v5.0.0, ..., 5.8.x-dev, v6.0.0, ..., 6.x-dev, v7.0.0, ..., 7.x-dev, v8.0.0, ..., 8.x-dev, v9.0.0-beta.1, ..., 9.x-dev], laravel/framework[v9.2.0, 9.x-dev]. laravel/framework replaces illuminate/support and thus cannot coexist with it.
    - Root composer.json requires laravel/framework ^9.2 -> satisfiable by laravel/framework[v9.2.0, 9.x-dev].
jasperf commented 2 years ago

With changelog removed in fork / pull request it installed fine using:

{
    "name": "laravel/laravel",
    "type": "project",
    "description": "The Laravel Framework.",
    "keywords": ["framework", "laravel"],
    "license": "MIT",
    "repositories": [
        {
            "type": "vcs",
            "url": "git@github.com:smart48/Laravel-Media-Manager-1.git"
        }
    ],
    "require": {
        "php": "^8.0.2",
        "guzzlehttp/guzzle": "^7.2",
        "laravel/framework": "^9.2",
        "laravel/sanctum": "^2.14.1",
        "laravel/tinker": "^2.7",
        "ctf0/media-manager": "dev-master"
    },
    "require-dev": {
        "fakerphp/faker": "^1.9.1",
        "laravel/sail": "^1.0.1",
        "mockery/mockery": "^1.4.4",
        "nunomaduro/collision": "^6.1",
        "phpunit/phpunit": "^9.5.10",
        "spatie/laravel-ignition": "^1.0"
    },
    "autoload": {
        "psr-4": {
            "App\\": "app/",
            "Database\\Factories\\": "database/factories/",
            "Database\\Seeders\\": "database/seeders/"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "Tests\\": "tests/"
        }
    },
    "scripts": {
        "post-autoload-dump": [
            "Illuminate\\Foundation\\ComposerScripts::postAutoloadDump",
            "@php artisan package:discover --ansi"
        ],
        "post-update-cmd": [
            "@php artisan vendor:publish --tag=laravel-assets --ansi --force"
        ],
        "post-root-package-install": [
            "@php -r \"file_exists('.env') || copy('.env.example', '.env');\""
        ],
        "post-create-project-cmd": [
            "@php artisan key:generate --ansi"
        ]
    },
    "extra": {
        "laravel": {
            "dont-discover": []
        }
    },
    "config": {
        "optimize-autoloader": true,
        "preferred-install": "dist",
        "sort-packages": true
    },
    "minimum-stability": "dev",
    "prefer-stable": true
}

locally using a clean Laravel 9 setup:

composer update
Loading composer repositories with package information
Updating dependencies                                 
Lock file operations: 3 installs, 0 updates, 0 removals
  - Locking ctf0/media-manager (dev-master e82b29e)
  - Locking maennchen/zipstream-php (2.1.0)
  - Locking myclabs/php-enum (1.8.3)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 3 installs, 0 updates, 0 removals
  - Downloading ctf0/media-manager (dev-master e82b29e)
  - Installing myclabs/php-enum (1.8.3): Extracting archive
  - Installing maennchen/zipstream-php (2.1.0): Extracting archive
  - Installing ctf0/media-manager (dev-master e82b29e): Extracting archive
Generating optimized autoload files
> Illuminate\Foundation\ComposerScripts::postAutoloadDump
> @php artisan package:discover --ansi
Discovered Package: ctf0/media-manager
Discovered Package: laravel/sail
Discovered Package: laravel/sanctum
Discovered Package: laravel/tinker
Discovered Package: nesbot/carbon
Discovered Package: nunomaduro/collision
Discovered Package: spatie/laravel-ignition
Package manifest generated successfully.
81 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
> @php artisan vendor:publish --tag=laravel-assets --ansi --force
No publishable resources for tag [laravel-assets].
ctf0 commented 2 years ago

the package-changelog had an issue in the composer json file, its now released correctly on packagist.

for the media manager, u were correct, i havent commited the new changes yet, so using a different git repo is the way to go until all is good.

jasperf commented 2 years ago

the package-changelog had an issue in the composer json file, its now released correctly on packagist.

I restored now and yes, it does work now inside package

for the media manager, u were correct, i havent commited the new changes yet, so using a different git repo is the way to go until all is good.

Okay, well, all tests well without PHP errors now. Just need to see how I could load a modal in the package to test that a bit. .. or at least load some media.. ideas @ctf0 ?

NB No PHP unit tests nor Vue Jest tests so no testing there to be had

ctf0 commented 2 years ago

yeah i didnt make any tests for the package unfortunately.

as for the modal or the usage in general, all should work as b4 except i think we will have an issue with this part

https://github.com/ctf0/Laravel-Media-Manager/blob/831caac8e585b25fa127ac2093e8ff1d516916b7/src/App/Controllers/MediaController.php#L69

this needs to be updated with the new filesystem

jasperf commented 2 years ago

@ctf0 Why an issue with that? Is that Flysystem 3 related or?

ctf0 commented 2 years ago

check https://flysystem.thephpleague.com/docs/upgrade-from-1.x/

Update : Apparently the update will need more work than just updating the packages versions, i will need to go through all the changes in the link and upgrade the manager with it, however any help is much much appreciated 🙏

jasperf commented 2 years ago

@ctf0 I see (checked Flystem upgrade list some). Will see what I can do this week(end) or next week. Do want this package to upgraded and keep on going strong. Appreciate all the work you have done thus far.

ctf0 commented 2 years ago

@jasperf thank you so much for all your help ❤️

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

jasperf commented 2 years ago

Was reading about listContents and checking Laravel Media Manager code some more again. And realized storageDisk is a separate storageDisk from Storage::disk('') used by Laravel. Been too long since I looked into all this... Just a way to hook into the filesystem I suppose.

But I also wondered about $this->storageDisk->addPlugin(new ListWith()); What was addPlugin here? Then I realized that was also old Flysystem code https://flysystem.thephpleague.com/v1/docs/advanced/provided-plugins/ and plugins together with listWith have been removed https://flysystem.thephpleague.com/docs/upgrade-from-1.x/ .. Well, listWith was/is a plugin for the old version.

No replacement was suggested for this plugin however. Only a mention that is was performing poorly. So we may need a Laravel method to grab all the file data for which you added checks for in trait GetContent. But we better start with the MediaController.php itself first. And for that I am still thinking as well.

For loading in file data we need something new like

// https://flysystem.thephpleague.com/docs/usage/directory-listings/
// perhaps better path check followed by:
$allFiles = $filesystem->listContents('/some/path')

as for mimetype detection in https://github.com/ctf0/Laravel-Media-Manager/blob/master/src/App/Controllers/Modules/GetContent.php before listWith usage we can use https://flysystem.thephpleague.com/docs/advanced/mime-type-detection/