thekordy / ticketit

A simple helpdesk tickets system for Laravel 5.1+ which integrates smoothly with Laravel default users and auth system, demo is available at: http://ticketit.kordy.info/tickets
MIT License
875 stars 386 forks source link

Simplifying Package Pre-requests #65

Closed thekordy closed 9 years ago

thekordy commented 9 years ago

We should not ask users at upgrades or fresh installs to do a lot of preparing their application in order to use Ticketit package.

In v0.2.0 we ask users to set up 2 non-default Laravel settings (users, emails), and to install 2 packages (collective forms, laravel datatables). I believe the less pre-requests we ask for the more attractive is the package.

About dependents packages, Composer made it easy for installing dependents packages with no need of user interaction, so the only other thing remains is registering the dependents packages service provider and that can be done via TicketitServiceProvider as follow:

/*
         * Register the service provider for the dependency.
         */
        $this->app->register('Collective\Html\HtmlServiceProvider');
        $this->app->register('yajra\Datatables\DatatablesServiceProvider');
        /*
         * Create aliases for the dependency.
         */
        $loader = \Illuminate\Foundation\AliasLoader::getInstance();
        $loader->alias('Form', 'Collective\Html\FormFacade');
        $loader->alias('Html', 'Collective\Html\FormFacade');
        $loader->alias('Datatables', 'yajra\Datatables\Datatables');
thekordy commented 9 years ago

What about fresh installs? If someone has just installed the package, he will be required to do much steps:

  1. Config pre-requests Laravel settings
  2. Install Ticketit
  3. add Ticketit service provider to app config file 4. add dependents service providers
  4. migrate
  5. Run the seeder

At this point he can go to the default url/tickets and use the new admin panel to adjust all settings, but if he uses another master template name, that requires three more steps:

  1. publish ticketit config file
  2. must adjust the master_template setting
  3. run the seeder again

@FusionDesign @fishmad , Do you have any thoughts about doing these:

  1. Do the migration automatically first time
  2. Ask At the first visit about the master_template name, if it differs from the default name in settings instead of giving error
  3. Run the seeder automatically at first time
thekordy commented 9 years ago

In answer to 1 .. We could do the migration automatically if the system found one is missing:

TicketitServiceProvider.php

public function boot()
    {
        // Package Migrations
        $tables = [
            '2015_07_22_115516_create_ticketit_tables',
            '2015_07_22_123254_alter_users_table',
            '2015_09_05_204742_create_jobs_table',
            '2015_09_29_123456_add_completed_at_column_to_ticketit_table',
            '2015_10_08_123457_create_settings_table'
        ];

        // Application active migrations
        $migrations = DB::select('select * from migrations');

        $mig_counter = 0;
        foreach ($migrations as $migration_arr) { // Count active package migrations
            if (in_array($migration_arr->migration, $tables)) {
                $mig_counter += 1;
            }
        }
        if (count($tables) != $mig_counter) { // If a migration is missing, do the migrate
            Artisan::call('migrate', array('--path' => 'vendor/kordy/ticketit/src/Migrations'));
        }
thekordy commented 9 years ago

for the third point, we could run the seeder right after the automatic migration .. something like this:

// Package Migrations
        $tables = [
            '2015_07_22_115516_create_ticketit_tables',
            '2015_07_22_123254_alter_users_table',
            '2015_09_05_204742_create_jobs_table',
            '2015_09_29_123456_add_completed_at_column_to_ticketit_table',
            '2015_10_08_123457_create_settings_table'
        ];

        // Application active migrations
        $migrations = DB::select('select * from migrations');

        $mig_counter = 0;
        $run_seeds = true;
        foreach ($migrations as $migration_arr) { // Count active package migrations
            if (in_array($migration_arr->migration, $tables)) {
                $mig_counter += 1;
            }
        }
        if (count($tables) != $mig_counter) { // If a migration is missing, do the migrate
            Artisan::call('migrate', array('--path' => 'vendor/kordy/ticketit/src/Migrations'));
        }

        // If ticketit_settings empty, run the seeder
        $settings_table = DB::select('select * from ticketit_settings');
        if (empty($settings_table)) {
            Artisan::call('db:seed', array('--class' => 'Kordy\\Ticketit\\Seeds\\SettingsTableSeeder'));
        }
thekordy commented 9 years ago

And we should fix the seeder class for the path of the config file

Seeds/SettingsTableSeeder.php

$config = [];

        if (File::isFile('../config/ticketit.php')) {
            $config = include '../config/ticketit.php';
            File::move('../config/ticketit.php', '../config/ticketit.php.backup');
        }

but that will make the seeding from command line fail to merge the previous user settings from 'config/ticketit.php' !!

thekordy commented 9 years ago

Found out how to resolve the seed problem .. actually two problems:

  1. seems that if we run seed from cli it will use a different path other than if you run it from provider
  2. because i put the automatic seed functionality in the provider boot, then when I run migrations or seed from cli, it will use the provider seed, so it will fail because provider seed is using a different path!!

The solution, because anyway if we run the seed using the provider or using the cli, either way, the provider seed will run, so lets make a check and choose the right path in each situation as follow:

in TicketitServiceProvider.php

// If ticketit_settings empty, run the seeder
        $settings_table = DB::select('select * from ticketit_settings');
        if (empty($settings_table)) {
            $cli_path = 'config/ticketit.php'; // if seeder run from cli, use the cli path
            $provider_path = '../config/ticketit.php'; // if seeder run from provider, use the provider path
            $config_settings = [];
            $settings_file_path = false;
            if (File::isFile($cli_path)) {
                $settings_file_path = $cli_path;
            } elseif (File::isFile($provider_path)) {
                $settings_file_path = $provider_path;
            }
            if ($settings_file_path) {
                $config_settings = include $settings_file_path;
                File::move($settings_file_path, $settings_file_path.'.backup');
            }
            $seeder = new SettingsTableSeeder();
            $seeder->config = $config_settings;
            $seeder->run();
        }

Remove the file operations in Seeds/SettingsTableSeeder.php

class SettingsTableSeeder extends Seeder
{

    public $config = [];

    /**
     * Seed the Plans table
     */
    public function run()
    {

        $defaults = [];

        $defaults = $this->cleanupAndMerge($this->getDefaults(), $this->config);

        foreach ($defaults as $slug => $column) {

            $setting = Setting::bySlug($slug);
thekordy commented 9 years ago

I will open a special PR for this issue for not making conflicts with other active PRs

thekordy commented 9 years ago

Note: You can try installing a fresh ticketit 0.2 using this command: You might like to remove the dependents packages and a fresh database to see the latest effects mentioned in this issue

composer require kordy/ticketit:0.2.x-dev

Then just add the ticketit class to the config/app.php

Then the master_template setting in config/ticketit.php (only if it's different the master.blade.php)

fishmad commented 9 years ago

my master is located in a subdir /layouts so I'll see what happens.

thekordy commented 9 years ago

Do you have any thoughts about doing these:

  1. Do the migration automatically first time
  2. Ask At the first visit about the master_template name, if it differs from the default name in settings instead of giving error
  3. Run the seeder automatically at first time

I did 1 and 3 .. Still 2 requires interaction from the user as follow (only if it's different the resources/views/master.blade.php) :

  1. create ticketit config file at config/ticketit.php
  2. must adjust the master_template setting
  3. migrate:rollback and then run the seeder again
fusiondesign commented 9 years ago

I'm alive again. I gotta hit up a couple customer requests then I'll be doing a fresh install and giving thing a go. Very very nice work @thekordy

fishmad commented 9 years ago

I dropped my database, removed ticketit folders entirely Commented // Kordy\Ticketit\TicketitServiceProvider::class, from config/app.php ran - composer require kordy/ticketit:0.2.x-dev php artisan migrate php artisan db:seed Uncommented Kordy\Ticketit\TicketitServiceProvider::class, ran php artisan migrate --path=vendor/kordy/ticketit/src/Migrations

Everything installed and populated correctly, however I still had go into mysql directly and edit master_template from master to layouts/master.

I also had to open Models/Setting.php and still Uncommented Cache::forget('settings');

I would think the only way to truly capture to master file path is through CLI install process using a custom console command and take input from the user.

Alternatively you could try a custom file search during install to looks for any file with master in the filename and attempt to auto capture the view file path.

fusiondesign commented 9 years ago

Sorry I didn't get time to work on this last night! I've been up all night working on a client app for new features!! After some sleep today I'll get back to it :)

thekordy commented 9 years ago

Something I am working on it right now ;)

ticketit installation ticketit installation3

thekordy commented 9 years ago

Done :) ... Now a fresh install takes only 3 simple steps:

  1. composer require kordy/ticketit:0.2.x-dev (or just dump fresh db and clone the new commit)
  2. Add Kordy\Ticketit\TicketitServiceProvider::class, to config/app.php file
  3. Hit url/tickets-install

This just an opening, any modifications and improvements are very welcomed :+1:

thekordy commented 9 years ago

Sorry I didn't get time to work on this last night! I've been up all night working on a client app for new features!! After some sleep today I'll get back to it :)

@FusionDesign Please take all the time you need, No need for any rush. Only if you've got a free time more than you need and there is something you deeply like to code ;)

fusiondesign commented 9 years ago

I'm blown away!! @thekordy - Pulling things down and syncing now! Can't wait to try it

fusiondesign commented 9 years ago

It worked flawlessly. Excellent work @thekordy . One mention would be future updates to the Seeder: If we change a value or add new values to getDefaults() in SettingsTableSeeder.php -- Do you envision using the installer to run a simple reSeed as well or what do you imagine in this scenario?

I was considering changing line 26 in TicketitServiceProvider from "DB::table('ticketit_settings')->count() != 0" to something like:

... && DB::table('ticketit_settings')->count() != count(Setting::getDefaults())

I'm not sure how we'd like to approach this but I may have a better idea as I parse through all the awesome changes!!

fusiondesign commented 9 years ago

getDefaults() is not static obviously so that code wouldn't work without some changes, just getting some ideas going

thekordy commented 9 years ago

That would be better obviously :+1: ... also I was thinking to make the tables (migrations) check more dynamic, so instead of using a static array, it searches our Migrations directory for migrations files and compares them with installed migrations .. that would be more dynamic.

ticketit_migrations = File::files('../vendor/kordy/ticketit/src/Migrations/');
fusiondesign commented 9 years ago

Ooooohhh fancy! I like these ideas. :D