robersonfaria / laravel-database-schedule

Manage your Laravel Task Scheduling in a friendly interface and save schedules to the database.
GNU General Public License v3.0
322 stars 54 forks source link

Introduce the optional ability to specify jobs in groups #41

Closed thyseus closed 3 years ago

thyseus commented 3 years ago

When dealing with a lot of jobs (we will have 100+) it is useful to order the jobs by given groups. By default this feature is turned off.

robersonfaria commented 3 years ago

@thyseus Ok, I think it's an interesting feature, you could add a filter by group in the index screen and also put in the README a little explanation of how it works, a comment in the config file would also be interesting.

thyseus commented 3 years ago

@robersonfaria ok, will do so, expect it to arrive tomorrow.

https://github.com/robersonfaria/laravel-database-schedule/pull/42 will be merged into this MR so you just have to click on "merge" once reviewed :)

thyseus commented 3 years ago

@robersonfaria me and @lordofthebrain did some work:

grafik

thanks a lot in advance for your exhausting code review :)

robersonfaria commented 3 years ago

@thyseus and @lordofthebrain thank you very much for the contributions.

Later today I will merge a branch setting up automated tests in the package, I have written a few initial tests, but if you have availability to write more tests I would appreciate it.

The package is growing a lot and getting difficult to test whenever there is a new pull request, I believe that automated tests will help a lot.

thyseus commented 3 years ago

@robersonfaria, @lordofthebrain automated tests would be very great. I can´t wait to see how you have implemented them ! Do you know https://github.com/orchestral/testbench ? I have never used it but would like to do so in the future.

A new major release at packagist would also be very cool.

robersonfaria commented 3 years ago

@thyseus Yes, the tests are already in version 1.1.5, I used this package to configure it, I hadn't used it yet, it took a bit of work but it's working. I've already put in some basic tests, I still need to understand their coverage. I plan to get some more time in the next few days to write more tests.

About the major release, you say release a 2.0.0 because of testing? I see it's not a change that impacts the component's usage, so I went with version 1.1.* but we can change.

thyseus commented 3 years ago

@robersonfaria, @lordofthebrain That is great!

We have quickly added Feature tests that test the UI of the package like:

<?php

namespace Tests\Feature;

use App\Models\Customer;
use Tests\TestCase;

/**
 * @internal
 * @coversNothing
 */
class ScheduleTest extends TestCase
{
        public function testScheduleNotReachable()
        {
                $response = $this->followingRedirects()->get('/schedule');
                $response->assertStatus(403);
        }

        public function testScheduleIndex()
        {
                $customer = Customer::factory()->create();
                $user = $customer->users()->first();

                $user->assign('admin-schedule');

                $this->actingAs($user);

                $response = $this->followingRedirects()->get('/schedule');
                $response->assertStatus(200);
                $response->assertSee('Europe/Berlin');
        }

        public function testScheduleCreate()
        {
                $customer = Customer::factory()->create();
                $user = $customer->users()->first();

                $user->assign('admin-schedule');

                $this->actingAs($user);

                $response = $this->followingRedirects()->get('/schedule/create');
                $response->assertStatus(200);
                $response->assertSee('Create new schedule');
        }
}

We placed this Feature test into OUR project tests. Not sure how this is possible with testbench.

How about a 1.2.0 release since we gained a lot of non-breaking features in the recent time ?

robersonfaria commented 3 years ago

I believe that in the package these tests will not work because we don't have user control and permissions, and I believe that you won't be able to access a route in the test either. But it would really be quite interesting to get to test the screens, I'll study about.

About the release, I'll create 1.2.0