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

Create plugin settings on database #14

Closed
Multiconecta opened this issue Jan 30, 2019 · 13 comments
Closed

Create plugin settings on database #14

Multiconecta opened this issue Jan 30, 2019 · 13 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@Multiconecta
Copy link
Contributor

Dismembering enhancement issue #4.

This issue will address the need of plugin configuration form and database table for any settings of plugin. Then, it will be possible to include new enhancements with specific settings to enable, assuring the initial normal behavior if no settings are enabled.

As will be needed at least one setting, the project should have an "Enable timer in tasks", default enabled, that will just deactivate all hooks (and the plugin) if set to Disable.

@OscarBeiro OscarBeiro added the enhancement New feature or request label Jan 30, 2019
@OscarBeiro OscarBeiro added this to the 1.1.1 milestone Jan 30, 2019
@OscarBeiro
Copy link
Contributor

Great!
I think it would be better to remove this setting when we have a useful one, to avoid having a useless plugin if it is correctly installed and activated.

@Multiconecta
Copy link
Contributor Author

Perfect. I'm on it. I'll let the config fork commented out after successful tests, so no settings will appear.

@Multiconecta
Copy link
Contributor Author

Multiconecta commented Feb 18, 2019

I think I have it. But I would like to know how would be better to have those settings:

  1. in a tab inside Setup - General (the same as Behaviours Plugin, for example)
  2. in its own settings page (the same as Additional Fields Plugin, for example)

The settings page/tab would be accessed by clicking Setup - Plugins - ActualTime (as all plugins are). We can also add a menu item (probably in menu Assistance), case we choose option 2 above. Additional Plugins, for instance, adds its settings menu in Setup menu. I think this would not make too much sense if plugin's settings in a tab inside general setup (option 1 above).

My opinion: as there aren't many settings planned, and it will be usually "one time configure" settings, I would choose option 1, with no menu item. But I would like your opinions, @OscarBeiro , @xacobofg

@OscarBeiro
Copy link
Contributor

As you wish.
It's OK option 1.

@Multiconecta
Copy link
Contributor Author

The settings are ready. I just don't know if it shouldn't be pulled to a test branch, so you can see it working. Then, we just modify it to not work until an actual setting is necessary. Today, there is only the "Enable timer on tasks", that would disable all hooks (all plugin work) if not on. If you prefer, I can do the PR now (there is a "Enable timer on task" option in general setup, but disabling it save the option in the database and just do nothing).

@OscarBeiro
Copy link
Contributor

Good idea.
Please, go ahead

@Multiconecta
Copy link
Contributor Author

Oscar, I did not understand. Do you want me to create a pull request to master branch, with no action, or will you create a new branch, so I can create a pull request to that branch, for testing purposes?

@OscarBeiro
Copy link
Contributor

Sorry, about the misunderstanding.
We had a development branch, but it was left behind a few commits.
A new development branch has been created, please create your PR to it.
Regards,

@Multiconecta
Copy link
Contributor Author

Created PR #19 on branch development, for testing purposes. Only settings avaiable: enable or disable timer.

For Glpi to update the database, the plugin version was increase to 1.1.1. That way, updating plugin directory will show the upgrade option on Setup - Plugins page, needed to get the database change. No timer's data for the tasks will be lost.

@OscarBeiro
Copy link
Contributor

OK. We had a warning before upgrade, but we couldn't reproduce.
So , please, move on.
Obrigado

@Multiconecta
Copy link
Contributor Author

Didn't get it? Clicking Upgrade in Setup-Plugins, you've got a warning? I didn't have any of these.

Just "git checkout development"
Glpi: Setup - Plugins, Upgrade, Enable
After that, you can reach the settings form clicking on "ActualTime" in the plugins page. Or directly in Setup-General, tab "ActualTime".

@OscarBeiro
Copy link
Contributor

Don't worry. It's merged because It's working.
I told you we couldn't reproduce.

So should we close this issue, or do you want to keep working on it?

@Multiconecta
Copy link
Contributor Author

We can close it. So we use the development branch for all the changes that will use the settings (that will probably come on 1.2.0). That way, no one using the plugin now need to upgrade and will do so only on the 1.2.0 release, but we can develop the features that need the settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants