Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

DboPostgres & FormHelper improvements #41

Closed
wants to merge 4 commits into from

5 participants

@remidewitte

This is a very small improvement to support additional settings on connection.
Very useful to set the datestyle to DMY for example.

The second commit is also a small improvement so that any options defined in the model are picked up by the FormHelper. Helpful for enumerated values for example.

@AD7six
Collaborator

doesn't look very small

changes to view/helpers/form.php - why?
changing the comment from # to --, why?

is SET ... TO ... syntax understood by all db drivers?

@remidewitte

Well, if you click on the diff https://github.com/cakephp/cakephp/pull/41/files it is small imho... I am just a bit new to git, this is why it is 4 commits.

changes to view/helpers/form.php - why?
=> any options defined in the model are picked up by the FormHelper. Helpful for enumerated values for example.

changing the comment from # to --, why?
=> # does not work with Postgresql whereas -- is SQL standard

is SET ... TO ... syntax understood by all db drivers?
=> SET ... TO ... is a postgresql syntax. Mysql uses SET ... = ... . That's why it is only a modification on DboPostgres.

Thanks.

@AD7six
Collaborator

amusing you think anyone would comment without looking at the diff.

Tip: if you put 2 or more logical changes in a pull request - if any one of your changes cannot be integarted its more work/The pull request doesn't get actioned. Also if you push to the same branch before the pull request is closed your changes are going to get added to the same pull request - therefore it's a good idea to create a branch specifrically for each pull request you want to make.

I perhaps misled you with my questions, why change something in schema to be sql standard, and change the postgres dbo driver so that it has something which is sql standard (SET x =), but implemented in a postgres specifric way? Why give the postgres driver some functionality which other dbos /could/ have but deliberately do not?

As there are no test case or documentation changes, I don't know how you are intending the form helper changes to work - but putting something in the helper class which relies on a specific dbo driver doesn't sound like something which belongs in the core, nor does unwarrented/hidden coupling between the model and view layer.

@remidewitte

Thanks for the tip about the branches. I initially wanted to create 2 pull requests and realized it was added to the same one.

About the SET, I don't really know whether it would be useful to other databases. But if some people thinks it is, I would be happy to do so... For many European countries, dates are input and displayed 'd/m/Y', so instead having to convert all the time from and to the ISO 'Y-m-d' we just need to issue a simple 'SET datestyle TO sql, dmy' in Postgres.

About the form.php, I suspected it would be a bit more controversial because of no test case. Let me just emphasize that it is not related to dbo. I use it with an 'Enumerable' behavior which is completely generic. The only bit missing to get some magic is the change in the form.php.

In a model class, I define a $enums property like this :
public $enums = array(
'field1' => array(
1 => 'Yes',
0 => 'No'
);
);

class EnumerableBehavior extends ModelBehavior {

/*

  • Add 'options' to schema to be picked by FormHelper::input() */ function setup(&$Model, $settings) { if(!isset($Model->enums)){ foreach ($Model->enums as $field => $values){ if(isset($Model->virtualFields[$field])){ $Model->_schema[$field]['type'] = 'string'; } $Model->_schema[$field]['options'] = $values; } } }

}

@d1rk

I like the enums.

@ADmad
Collaborator

Model::$_schema is a protected var by convention, hence modifying it from a ModelBehavior class is technically incorrect.

@westernmagic westernmagic referenced this pull request from a commit
@paulirish paulirish dd_belatedPNG++ to new version for extra bugfixes. (keep in mind that…
… it and all other JS is unminified and they should clearly be minified when going out into production.) fixes #41
078e291
@markstory
Owner

Closing as this no longer applies cleanly and the changes are missing tests.

@markstory markstory closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  cake/console/libs/schema.php
@@ -217,7 +217,7 @@ function dump() {
}
}
$db =& ConnectionManager::getDataSource($this->Schema->connection);
- $contents = "#" . $Schema->name . " sql generated on: " . date('Y-m-d H:i:s') . " : " . time() . "\n\n";
+ $contents = "-- " . $Schema->name . " sql generated on: " . date('Y-m-d H:i:s') . " : " . time() . "\n\n";
$contents .= $db->dropSchema($Schema) . "\n\n". $db->createSchema($Schema);
if ($write) {
View
6 cake/libs/model/datasources/dbo/dbo_postgres.php
@@ -129,6 +129,12 @@ function connect() {
if (!empty($config['encoding'])) {
$this->setEncoding($config['encoding']);
}
+ // Support any SET ... TO ... (http://www.postgresql.org/docs/9.0/static/sql-set.html)
+ if (!empty($config['settings'])) {
+ foreach ($config['settings'] as $name => $value) {
+ $this->_execute("SET " . $name . " TO " . $value);
+ }
+ }
return $this->connected;
}
View
9 cake/libs/view/helpers/form.php
@@ -730,8 +730,13 @@ function input($fieldName, $options = array()) {
$options['type'] = 'password';
} elseif (isset($this->fieldset[$modelKey]['fields'][$fieldKey])) {
$fieldDef = $this->fieldset[$modelKey]['fields'][$fieldKey];
- $type = $fieldDef['type'];
- $primaryKey = $this->fieldset[$modelKey]['key'];
+ if(isset($fieldDef['options'])){
+ $options['type'] = 'select';
+ $options['options'] = $fieldDef['options'];
+ } else {
+ $type = $fieldDef['type'];
+ $primaryKey = $this->fieldset[$modelKey]['key'];
+ }
}
if (isset($type)) {
Something went wrong with that request. Please try again.