Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Activator feature #789

Closed
ryssbowh opened this issue Jul 7, 2019 · 13 comments

Comments

@ryssbowh
Copy link
Contributor

commented Jul 7, 2019

Hi there,

In my project I needed to take the activation logic out of the Module class, the main reason for this being that the activation status being in a git tracked file, I find it makes it messy when pushing changes to modules (as composer packages) and pulling them in other projects.
If I'm developing a module, it's obviously enabled, but I might not want it enabled by default when pulling it in another project, maybe I have some logic to be made before etc..

Anyway I've implemented that in my project, with a default FileActivator that stores the activation status in a separate file, but it could be a database activator or anything else.

I'm not doing a pull request as it's quite a change but if that's of any interest I'll do one https://github.com/ryssbowh/laravel-modules/tree/activator/.

Thanks again for this great package !

@nWidart

This comment has been minimized.

Copy link
Owner

commented Jul 7, 2019

You can take a look at the database-modules branch for this too. It's not done yet, but since no-one cares enough contribute I won't finish it.

@nWidart

This comment has been minimized.

Copy link
Owner

commented Jul 7, 2019

I just looked at your branch, I like the idea of this. 👌 It's a good intermediary solution.

@ryssbowh

This comment has been minimized.

Copy link
Contributor Author

commented Jul 7, 2019

Thanks,

I did have a look at the database branch, but didn't feel I had the level to take it on.

Doing a pull request now.

@ryssbowh ryssbowh referenced this issue Jul 7, 2019
@SeoRoman

This comment has been minimized.

Copy link

commented Jul 7, 2019

@nWidart What all does the database-modules system still need?

@nWidart nWidart closed this Sep 2, 2019
@lexdewilligen

This comment has been minimized.

Copy link

commented Oct 3, 2019

@nWidart I've started working on the database implementation using your database branch. I've implemented some methods that were abstract, but I don't know how to go on. Could you take a look at my branch and tell me what needs to be done next?

My head get's a bit dizzy with the activator solution also in place. Should that be removed? I need the database functionality, so I'd like to implement this quickly.

lexdewilligen@0fe036e

@nWidart

This comment has been minimized.

Copy link
Owner

commented Oct 3, 2019

Thank you!

I believe if you need the database implementation for maintaining module statuses, using the activator feature will be your fastest way to get this up and running.

@lexdewilligen

This comment has been minimized.

Copy link

commented Oct 3, 2019

@nWidart Yes, but I don't understand how to implement the activator feature for the database..

@ryssbowh

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2019

Hi,

If you write a new Activator class that implements the ActivatorInterface and reference it in your config :

'activators' => [
    'file' => [
        'class' => FileActivator::class,
        'statuses-file' => storage_path('modules_statuses'),
        'cache-key' => 'activator.installed',
        'cache-lifetime' => 604800,
    ],
    'database' => [
        'class' => DatabaseActivator::class
    ]
],
'activator' => 'database'

That should work

@lexdewilligen

This comment has been minimized.

Copy link

commented Oct 3, 2019

Yes I also got to that part, but more on the specific implementation of the DatabaseActivator.

How should I implement the following for example:

    /**
     * @inheritDoc
     */
    public function setActiveByName(string $name, bool $status): void
    {
        $this->modulesStatuses[$name] = $status;
        $this->writeJson();
        $this->flushCache();
    }

I'd say:

    /**
     * @inheritDoc
     */
    public function setActiveByName(string $name, bool $status): void
    {
        $this->moduleEntity->is_active = $status;
        $this->moduleEntity->save();
    }

Or should that be done differently?

@nWidart

This comment has been minimized.

Copy link
Owner

commented Oct 3, 2019

that looks ok yes. Probably a findByModuleName or something just before.

@lexdewilligen

This comment has been minimized.

Copy link

commented Oct 7, 2019

Okay, I've implemented the DatabaseActivator lexdewilligen@bf0d294, however, there are still a few issues:

  • loadMigrationsFrom does not load the packages modules migration
  • Modules don't get booted/registered

Any clue on why this is the case? How/where are modules booted using the FileActivator?

https://github.com/lexdewilligen/laravel-modules/tree/feature/database-activator

UPDATE

Problem 1 is solved, I had to change loadMigrationsFrom from:

$this->loadMigrationsFrom(__DIR__ . '../database/migrations');

To:

$this->loadMigrationsFrom(__DIR__ . '/../database/migrations');

Problem 2 is solved too

@nWidart

This comment has been minimized.

Copy link
Owner

commented Oct 8, 2019

Hi @lexdewilligen ,
I had a quick look at your branch and it looks good.
I would do a couple of changes but as start this is very promising, good job!

@lexdewilligen

This comment has been minimized.

Copy link

commented Oct 8, 2019

Thank you @nWidart ! I've just got it "working" last night. I'll improve the code and then submit a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.