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

Optionally pass the database configuration parameters on Rbac constructo... #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sergiodlopes
Copy link

Possibility to programmatically pass on the database configuration.

Just pass the parameters into the constructor, falls back to default configuration variables (same as those in database.config file).

This change was made with regard of compatibility with existing version.

@jburns131
Copy link
Collaborator

Thank you very much!!!

Please let me review these changes and how they relate to current PR's and discussion and I'll get back to you shortly :-)

@abiusx
Copy link
Contributor

abiusx commented Apr 25, 2015

@jburns131 bump

@sillyh4ck3r
Copy link

Please, also, add ability to change role/user/permision 'id' column names.

For instance, 'role_id', instead of default 'RoleID'.

Thanks.

@abiusx
Copy link
Contributor

abiusx commented Oct 4, 2015

what good will that do? a use case scenario please. It is possible and existed in the initial code base but was removed because of bloat.

On Oct 4, 2015, at 12:03 PM, sillyh4ck3r [email protected] wrote:

Please, also, add ability tp change role/user/permision 'id' column names.

For instance, 'role_id', instead of default 'RoleID'.

Thanks.


Reply to this email directly or view it on GitHub #63 (comment).

@sillyh4ck3r
Copy link

I had some troubles integrating rbac with php-activerecord in a case like this:

class User extends ActiveRecord\Model { static $table_name = 'users'; static $has_many = array ( array ('roles', 'through' => 'userroles',), array ('userroles', 'class_name' => 'UserRole',), ); }

If I defined 'foreign_key' => 'UserID' on for 'roles', the SQL would generate
with an INNER JOIN that uses ON rbac_roles.id = rbac_userroles.role_id
WHERE UserID=?

I needed that role_id to be RoleID.

To get around this, I changed the column names in the respective tables and
edited the rbac.php to reflect those changes. With this workaround, I don't
need to specify 'foreign_key' in my model attributes.

Also, I have added foreign key constraints on the relations tables. (Not
relevant to my original request, though, it helps a bit.)

AbiusX wrote:

what good will that do? a use case scenario please. It is possible and existed in the initial code base but was removed because of bloat.

On Oct 4, 2015, at 12:03 PM, sillyh4ck3r [email protected] wrote:

Please, also, add ability tp change role/user/permision 'id' column names.

For instance, 'role_id', instead of default 'RoleID'.

Thanks.


Reply to this email directly or view it on GitHub #63 (comment).


Reply to this email directly or view it on GitHub:
#63 (comment)

@abiusx
Copy link
Contributor

abiusx commented Oct 4, 2015

I think that’s a good enough solution.
To dynamically fetch column names in every call, makes a lot of bloat and performance overhead.

On Oct 4, 2015, at 3:26 PM, sillyh4ck3r [email protected] wrote:

I had some troubles integrating rbac with php-activerecord in a case like this:

class User extends ActiveRecord\Model { static $table_name = 'users'; static $has_many = array ( array ('roles', 'through' => 'userroles',), array ('userroles', 'class_name' => 'UserRole',), ); }

If I defined 'foreign_key' => 'UserID' on for 'roles', the SQL would generate
with an INNER JOIN that uses ON rbac_roles.id = rbac_userroles.role_id
WHERE UserID=?

I needed that role_id to be RoleID.

To get around this, I changed the column names in the respective tables and
edited the rbac.php to reflect those changes. With this workaround, I don't
need to specify 'foreign_key' in my model attributes.

Also, I have added foreign key constraints on the relations tables. (Not
relevant to my original request, though, it helps a bit.)

AbiusX wrote:

what good will that do? a use case scenario please. It is possible and existed in the initial code base but was removed because of bloat.

On Oct 4, 2015, at 12:03 PM, sillyh4ck3r [email protected] wrote:

Please, also, add ability tp change role/user/permision 'id' column names.

For instance, 'role_id', instead of default 'RoleID'.

Thanks.


Reply to this email directly or view it on GitHub #63 (comment).


Reply to this email directly or view it on GitHub:
#63 (comment)


Reply to this email directly or view it on GitHub #63 (comment).

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

Successfully merging this pull request may close these issues.

4 participants