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.

Sign up for GitHub

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

Jump to bottom

Add the ability to define alternative data source in plugin's bootstrap.php #116

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

Add the ability to define alternative data source in plugin's bootstrap.php #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Config/bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?php
if(!Configure::check('AuditLog.data_source')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I personally like configuring my plugins through Configure, this introduces a second way of configuration besides the established one, as documented here:

https://github.com/robwilkerson/CakePHP-Audit-Log-Plugin#configuration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also means the configuration will be written for every request.

Configure::write('AuditLog.data_source', 'default');
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed trailing new line

10 changes: 10 additions & 0 deletions Model/Behavior/AuditableBehavior.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public function setup(Model $Model, $settings = array()) {
}

if (!isset($this->settings[$Model->alias])) {
$this->settings[$Model->alias]['data_source'] = Configure::read('AuditLog.data_source');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use the normal and established way of configuration for this plugin, we can use a fall back to default.

I would introduce a property named $defaultDataSource set to default.

$this->settings[$Model->alias]['ignore'] = array('created', 'updated', 'modified');
$this->settings[$Model->alias]['habtm'] = array();
if (!isset($settings['habtm'])) {
Expand Down Expand Up @@ -173,6 +174,11 @@ public function afterSave(Model $Model, $created, $options = array()) {
array('hasMany' => array('AuditLog.Audit'))
);

if(isset($this->settings[$Model->alias]['data_source'])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style issue if (

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be extracted into a separate protected method. Would increase readability, reduce method length and complexity and would enable one to overwrite that new method, if needed.

$Model->Audit->useDbConfig = $this->settings[$Model->alias]['data_source'];
$Model->Audit->AuditDelta->useDbConfig = $this->settings[$Model->alias]['data_source'];
}

// If a currentUser() method exists in the model class (or, of
// course, in a superclass) the call that method to pull all user
// data. Assume than an ID field exists.
Expand Down Expand Up @@ -318,6 +324,10 @@ public function afterDelete(Model $Model) {
);

$this->Audit = ClassRegistry::init('AuditLog.Audit');
if(isset($this->settings[$Model->alias]['data_source'])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next few lines are practically a copy of the new code above. Another reason for extacting it into a generalized method.

$this->Audit->useDbConfig = $this->settings[$Model->alias]['data_source'];
$this->Audit->AuditDelta->useDbConfig = $this->settings[$Model->alias]['data_source'];
}
$this->Audit->create();
$this->Audit->save($data);

Expand Down