[4.x] Add command to bring the tenants up and down from maintenance and remove deprecated exception

[stein-j]

This PR changes the maintenance mode to comply to the changes made in Laravel 8.

New

• Adds two CLI commands to the package: tenancy:up and tenancy:down with the same parameters as the default Laravel command except for the option render because it is not possible to render a page before Laravel is booted for a specific tenant.

• Adds the function bringUpFromMaintenance in the trait MaintenanceMode.

Changes

A tenant in maintenance mode now throws an HttpException exception instead of the deprecated MaintenanceModeException exception.

Closes #533.

[stancl]

Hey. This looks good, thanks for the contribution! I think I had some maintenance mode related tasks written in my backlog for v4, so I'll probably merge this when working on the feature. Should be in January I think.

[codecov-commenter]

Codecov Report

Merging #761 (7875fbd) into 3.x (20e1fa1) will decrease coverage by 1.39%. The diff coverage is 0.00%.

@@             Coverage Diff              @@
##                3.x     #761      +/-   ##
============================================
- Coverage     87.02%   85.62%   -1.40%     
- Complexity      372      375       +3     
============================================
  Files           103      105       +2     
  Lines          1102     1120      +18     
============================================
  Hits            959      959              
- Misses          143      161      +18     

Impacted Files	Coverage Δ
src/Commands/Down.php	0.00% <0.00%> (ø)
src/Commands/Up.php	0.00% <0.00%> (ø)
src/Database/Concerns/MaintenanceMode.php	75.00% <0.00%> (-25.00%)	:arrow_down:
src/TenancyServiceProvider.php	96.66% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 20e1fa1...7875fbd. Read the comment docs.

[stancl]

Closing #757 and will finish that here. We could probably add --secret but I'm not sure if there's a point in having a different secret than the central app.

[stein-j]

Laravel now simply throws a HttpException exception. https://github.com/laravel/framework/blob/8f3b411969ffdd733a0ba54098e08f8ca7c1369e/src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php#L78

If you give me a few days, I can update the code and comply to the new maintenance mode used by Laravel and then update the commands so that we can use the same parameters as Laravel. I think all parameters can be recreated except for render because this allows to return a response even before Laravel is initialised (which means, before the package is also initialised).

Note: Naturally this would allow #757

[stancl]

Sure, thanks a lot for the work on this! FWIW if there's a signature change, you can use the version in Laravel 9. And probably change the target to the master branch.

[stein-j]

I'm glade you mentioned Laravel 9 because coincidently two PRs (1, 2) have been accepted during those last few days and it changed the maintenance system.

L9 will now have a configurable driver for the maintenance storage system (which will determine how and where the data is stored), at the time of writing this comment there are two drivers/managers file: Which is the old way to put the application to maintenance ; cache : Which will store the data inside the cache.

The reason they added drivers, is because when using a load balancer and you set the application into maintenance mode it would only work on the application the command has been typed on... so a cache driver was needed. But this package doesn't need such implementation because even if you would use a load balancer, there should be only one database connection per tenant for all applications (at least in most use cases).

So I think to keep things simple, this PR will keep its own feature and keep storing the data in the database. It might be important to mention in the documentation that the maintenance mode doesn't uses the driver from the config file.

Maybe in another PR we can try to create our custom driver, tenant compatible.

[stancl]

this PR will keep its own feature and keep storing the data in the database

That is in the data column in our case, right?

[stein-j]

Yes

[stancl]

Just a note: the tests will fail because master has some failing tests due to dependencies that need new syntax. But we can look at this feature's tests in isolation

[stein-j]

@stancl the PR is done and all the tests have been added. As you said there are conflict versions with composer, so the tests fails. Do you want me to look into it ? Or should we wait that the master to be updated ?

[stancl]

Probably wait until master is updated 👍🏻

When we add L9 support, I'll merge 3.x into master

[stein-j]

Hello @stancl,

I just realized that the changes from this PR are not compatible with Laravel 6 & 7. I've put this PR back to draft, ping me when you start working on the V4 and I'll see what can be done at that point.

[stancl]

@lukinovec I think we should finish this PR before the pending tenants since it's smaller and it interacts with the trait adding a --tenants option. That way we'll make the conflict resolution simpler.

Also, the tests are showing as failing, but I think they'll run differently when the CI is triggered again since I simplified the CI matrix in the master branch. So if you could resolve the conflicts, that would trigger a CI build and we'll see how the feature works now.

[stancl]

Only php-cs-fixer is failing, tests seem to be passing, so CI-wise this PR is ready now.

I'll look at the code now and see if this is ready to be merged.

[stancl]

@stein-j Is anything unimplemented yet or is the PR fully ready? Is the code made to work with Laravel 9 (the changes you made to the way tenants are put into maintenance mode)?

[stein-j]

@stancl I want to go over it again, it's been a while and a file was changed in another PR since (https://github.com/archtechx/tenancy/pull/802/files#diff-40487242039e9842dc87ba96285282e42d0cc5b9a7dbeac6f115c4ee2bb8db59). Let me get back to you in a few hours.

[stein-j]

@stancl what will be the minimum requirements for Laravel and PHP for V4 ?

[stancl]

Just that the syntax is updated for Laravel 9. I think you already did this since the PR is recent. But I'm just checking in case anything is missing.

[stein-j]

Ok, it's all good.

[stancl]

FYI I merged master into this so you should delete the 3.x branch after we merge this since it isn't compatible with our 3.x (in case you plan on making PRs to 3.x in the future)

[stancl]

Thanks for your work on this PR!