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

Understanding scopes #57

Open
Nyholm opened this issue Mar 12, 2016 · 6 comments
Open

Understanding scopes #57

Nyholm opened this issue Mar 12, 2016 · 6 comments

Comments

@Nyholm
Copy link
Collaborator

Nyholm commented Mar 12, 2016

I want to discuss planned features of this bundle. The PR (#54) from @juanolon got me thinking of this.

Currently we have 3 scopes. GLOBAL and USER where the latter requires you to provide an entity. We do also have ALL which is some kind of default for the USER scope. (One can discuss if the names are proper #23).

PR #54 introduce a granularity of the USER scope. I understand the need and I agree with it. He wants to make sure class A does not access settings of class B. (Both are naturally in the USER scope). This is a fix for #3.


I can see some possible solution for this. I don't know which one is better. .

Option 1) Making scopes dynamic. Lets use GLOBAL, ALL and DYNAMIC and then extend the SettingsOwnerInterface with a getScope function.

Option 2) Something more like #54 where we define a namespace and make sure the entity in the USER scope is an instance of that namespace.

Option 3) In the configuration specify what entities that are allowed to what sections of the configuration.

Option 4) ??? There must be something else

@rvanlaak
Copy link
Owner

Option 2, which involves the namespaces, also can be seen as a simple solution for grouping settings? Making the scopes dynamic would involve changes to the database structure, so probably option 2 is the easiest solution because it solely involves changing the configuration format.

@Nyholm
Copy link
Collaborator Author

Nyholm commented Mar 12, 2016

Okey, Great.

Correct me if Im wrong, but that will fix #3, right? If so, Im happy to make sure #54 is merged.

@rvanlaak
Copy link
Owner

Correct, it will fix #3 and I think for now #54 is the quick win we should pick.

@juanolon
Copy link

personally, i kind of like the option 1. this way, it's possible to scope settings, on the fly. like for example, an user get new roles, so getScope could return a new namespace. but this involves some other problems, like:

  • what is a scope? the class namespace? just some string?
  • i'm not sure, to what extend, this option would involve DB changes, as the old settings would not be loaded, and new ones are created on the fly. @rvanlaak how exactly did you think of that? maybe some internal behaviour, i'm not aware of..
  • cache?

so option 1 may get more complex.. that left us with option 2 - which is actually the easiest way ^^

@rvanlaak
Copy link
Owner

Most of the times the easiest solution is the way to go, as long as it solves your use cases. So lets try to agree on option 2. Don't like the idea that database changes would be needed 😉

@Nyholm
Copy link
Collaborator Author

Nyholm commented Mar 12, 2016

Agreed! Lets do option 2.
Will it still make sense to have the scope named "USER"? Maybe rename the scope as well?

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

No branches or pull requests

3 participants