phpvms
phpvms copied to clipboard
Fix Cron to Set Flights Visible to True Only When Owned By The Core
This PR resolves an issue where the nightly cron sets the visibility of flights created and owned by modules. This functionality is not exactly desired, as module developers would need to setup a cron event to set the flights back to their desired settings.
@BossOfGames I forgot what we decided about this
I forgot too, to be quite honest. I made a modification to my repo on this a while ago for my VA. Maybe a behavior like this could be enabled/disabled via config/setting/env file?
On Wed, Mar 6, 2024 at 1:53 AM Nabeel S. @.***> wrote:
@BossOfGames https://github.com/BossOfGames I forgot what we decided about this
— Reply to this email directly, view it on GitHub https://github.com/nabeelio/phpvms/pull/1770#issuecomment-1979214175, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAV7WHARBG3UJYJGWMRXXTDYWXZ67AVCNFSM6AAAAABC4GNF42VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZZGIYTIMJXGU . You are receiving this because you were mentioned.Message ID: @.***>
Why not
$flights = Flight::where('active', 1)->whereNull('owner_type')->get();
This will eliminate already inactive flights and the ones owned by a 3rd party module in the beginning.
Current implementation will still handle the visibility of 3rd party flights even this PR gets merged if there are days provided for the "owned" flight.
This will allow module developers to handle their flights separately and improve resource usage by skipping inactive flights at query level.
If a setting or config variable is required, we can simply change the query instead of function coding. Also this must be documented properly so addon developers can check it before applying/implementing their own scripts.
Why not
$flights = Flight::where('active', 1)->whereNull('owner_type')->get();
This will eliminate already inactive flights and the ones owned by a 3rd party module in the beginning.
Current implementation will still handle the visibility of 3rd party flights even this PR gets merged if there are days provided for the "owned" flight.
This will allow module developers to handle their flights separately and improve resource usage by skipping inactive flights at query level.
If a setting or config variable is required, we can simply change the query instead of function coding. Also this must be documented properly so addon developers can check it before applying/implementing their own scripts.
I agree with this better. I have updated the PR to reflect this suggestion.
If you want CI/CD to pass you should change this line:
https://github.com/nabeelio/phpvms/blob/53ab19a47d6c5a48635f6bff412d94e1e4bcfd0f/app/Database/factories/FlightFactory.php#L58
and set
owner_type
tonull
because in fact v7 sets it tonull
and notFlight::class
when we create a Flight
Didn't realize that code was there. I have corrected that, as well as fixed the removal of setting visible to true by default.