runish [rune]

Hi KiamLaLuno

Thx for your reply. My comments to that:

1) I am familiar with the "coding standards" and my current style is not very far from it. Last I ran the code through the coder module, It gave only minor errors. Should it be accepted as Drupal project, I will change it to 100% compliance with Drupal coding style. There is several thousands lines of code, and it will take a _long_ time to make it pass the coder module with 0 errors.

2) To quote from the current crud project:

"This is a utility / support module that simplifies SELECT, INSERT, UPDATE and DELETE operations for tables with a simple primary key (either an auto_increment key or a key defined in the sequences table)"

My module automatically generates:
* A list page of all items in the table (including pagination and ordering)
* An edit form for a single item
* A deletion form for a single item (including support for cascading deletion)
* Support for one-to-many / many-to-one and one-to-one table relationships

Only based on the schema in the .install file and a bit of additional configuration. So I _really_ don't think the two modules have the same purpose. But again I have no problems changing the name.

3) To automatically create forms based on database schemas is not currently a part of Drupal and can not easily be implemented using Drupal core functions. At least not without writing 80+ lines of code for each table you want implemented. Using crud module, it takes about 5 lines of configuration data for a table with no relationships.

Did you read through the code / try to install the modules?

Thanks for your time!

Added a screenshot of what the code looks like and the result you get.

Sorry for spamming. Here is a pdf file of the list,edit and delete view.

Thx a lot for quick reply.

To me Crud operations has always have something to do with forms, but I can see that you can consider CRUD both at the database level and at the User Interface level.

Will recommit this project under another name.

Ahh ok sure. Will refactor my code into a new module name, and upload it to this thread. If just I could figure out a good name :)

Edit: Am thinking about "Auto Admin"

Thx!

Thx for feedback!

1) No you are right. I've already fixed the naming of the functions in crud.inflector.inc and db.inc (now crud.db.inc). In my own setup I have a inflector and a db module, so just copied the files from there and forgot to change the function names. Also fixed all errors when running it through the coder module. These changes will appear in my next upload.

2) Was not aware that backticks are MySQL specific, thanks for pointing that out.

3) I see what you mean, will change it into using placeholders instead of doing the MySQL by hand.

Still thinking about that name... will upload a new and improved version when I think of one.

So here is a new version called: Auto Admin, folder name: "autoadmin".

## Changes ##
* Removed backticks. Might put them back in when mysql or mysqli driver is in use.
* Fixed naming of functions / files.
* Verified code with hardest setting in coder module.

## Not changed: Placeholders ##
I looked into how Drupal does it, and it does something similar to what I am doing: http://api.drupal.org/api/function/_db_query_callback/6, so my solution should not be more unsafe, although less optimized. My main problem is that I really like having the autoadmin_db_where() function that returns a string like "foo='bar' AND foz='baz'" from an array, and it gets more complicated (but certainly doable) to return an array like array("foo='%s' AND foz='%s'", array('bar', 'baz')). But if you really want, I will change it!

All the best
Rune

Hi

Here is an upload of autoadmin.db.inc with placeholders instead of manually escaping the sql. If it is in accordance with drupal guidelines (?) I will continue with the other files, but that is going to take some more work.

Cheers

## autoadmin_db_get_placeholder ##
If you are referring to http://api.drupal.org/api/function/db_type_placeholder/6 this is based on the type of field in the schema, whereas mine is based on the type of the variable. I _should_ use the schema information more in my database functions, but I would like to do that in a later version.

## autoadmin_db_result_has_rows ##
The closest I know of is http://api.drupal.org/api/function/db_result/6, but that moves the pointer of database object. The only other alternative I could find was to first do a "COUNT(*)" mysql call, but that is not so efficient.

Yir, you were right about the placeholders, even though it was a bit of a chore to change, it feels a lot nicer! Thx.

EDIT: Also helped med catch a couple of places where I forgot to escape variables before adding them to the sql.
EDIT2: Meaning that now I don't need to, because db_query() does it for me.

## Changes ##
* all db_query() calls uses placeholders
* removed the autoadmin_db_result_has_rows() function
* minor cleanups

Here is a new version.

## Changes ##
* No one line if sentences
* Non-documentation comments now are capitalized sentences with punctuation
* added @ingroup for form and @file docblocks
* added @see for form docblocks
* named parameters now have space around " = "

Hi again

Have not heard from anybody on this for a while, of reasons which I do not now, but I got a new version ready:

## Changes ##
* Removed all my custom database functions, everything is using db_query(), pager_query(), db_fetch_array() and drupal_write_record() now.
* Revamped code for SQL query generation.
* SQL generated is a lot more human readable
* The code is a lot tighter and easier to understand.
* Schemas with errors are not added to menu.
* Less magic more Drupal.

EDIT: The information about what this module actually does(Create administration forms automatically) is a bit scattered in this thread. Let me know if you need a rewrite of it!

EDIT2: To the best of my knowledge this version meets the changes suggested by KiamLaLuno.

EDIT3: KiamLaLuno has handled this process until now, but if he is too busy, maybe I could convince someone else to take a look? Sorry for nagging! :)

New version:

## Changes ##
* Added functional tests.

Hi

Thx a lot for feedback! Will address the issues mentioned. I could use some clarification regarding #26.

This works

  $res = db_query("SELECT %s, %s FROM {%s}",
    $pk, $field['relation']['foreign alias'], $field['relation']['foreign schema']
  );

But I agree that it is undrupalish. Would you prefer

  $res = db_query('SELECT ' . $pk . ',' . $field['relation']['foreign alias'] . ' FROM ' . $field['relation']['foreign schema']));

or

  $res = db_query(sprintf("SELECT %s, %s FROM {%s}",
    $pk, $field['relation']['foreign alias'], $field['relation']['foreign schema']
  ));

?

Cheers

Yes. All these three are taken from the schema supplied by the developer in hook_autoadmin(). Will change all places of using placeholders for SELECT and FROM statements into string concatenation, while keeping SQL injection in mind.

New version

## 25 ##
1. Now it is, or I just dont see it.
2. Everything in the userinterface is translateable now.
3. The size of the field was MySQL specific (http://help.scibit.com/Mascon/masconMySQL_Field_Types.html). Now returns default drupal values for other database engines.

## 26 ##
Fixed

## Other changes ##
* Cleanups and refactorings.

Fixed a bug (and added tests) regarding pagination. Please review this version instead.

Hey, thx for feedback!

## 1 ##
Fixed, but seems that it must run a lot more code now to do the same job.

## 2 ##
Fixed, see ## 4 ##

## 3 ##
Fixed bug in autoadmin_add_error(), and added the unit tests testAddErrorOneArgument(), testAddErrorTwoArguments(), testAddErrorThreeArguments() to test that it works.

## 4 ##
Think I finally understand the full extent of the problem now. Before I did not consider the need to make export/extract of .pot files work. Here are two different ways to define autoadmin schemas:

1)

$schema['album'] = array(
    // Dont autogenerate names based on table and field name
    'autoname' => FALSE,
    // Dont translate title, title_plural and description for schema and fields
    'translate' => FALSE,
    'title' => t('Album'),
    'title_plural' => t('Albums'),
    // Must repeat description here, because its against the drupal way
    // to use the t() function in .install files.
    'description' => t('Recorded albums'),
    'path' => 'admin/labels/albums',
    'alias' => 'title',
    'fields' => array(
      'id' => array (
        'title' => t('Id'),
        'title_plural' => t('Ids'),
      ),
      'title' => array(
        'title' => t('Title'),
        'title_plural' => t('Titles'),
      ),
      'description' => array(
        'title' => t('Description'),
        'title_plural' => t('Descriptions'),
      ),
      'weight' => array(
        'title' => t('Weight'),
        'title_plural' => t('Weights'),
      ),
      'label_id' => array(
        'title' => t('Label'),
        'title_plural' => t('Labels'),
      ),
    ),
    'relations' => array(
       'id' => array(
        'type' => 'has_many',
        'foreign table' => 'link_artist_album',
        'foreign key' => 'album_id',
        'foreign alias' => 'album_id',
      ),
      'label_id' => array(
        'type' => 'has_one',
        'foreign table' => 'label',
        'foreign key' => 'id',
        'foreign alias' => 'name',
      ),
    ),
  );

Here all text in the schema are using the t() function and can thus be extracted to a pot file.

2)

$schema['album'] = array(
    // Autogenerate names based on table and field name.
    'autoname' => TRUE,
    // Translate title, title_plural and description for schema and fields.
    'translate' => TRUE,
    'path' => 'admin/labels/albums',
    'alias' => 'title',
    'relations' => array(
       'id' => array(
        'type' => 'has_many',
        'foreign table' => 'link_artist_album',
        'foreign key' => 'album_id',
        'foreign alias' => 'album_id',
      ),
      'label_id' => array(
        'type' => 'has_one',
        'foreign table' => 'label',
        'foreign key' => 'id',
        'foreign alias' => 'name',
      ),
    ),
  );

This is a lot easier to write but extract of .pot files does not work. I plan to make a pot extracter. To translate this the user must manually search for each phrase in the translation interface, or use the Translation Client module (See attached screenshot showing a danish translation).

## Whats else is new ##
* Added languages folders in both autoadmin and autoadmin_example with extracted pot files.
* Cleanups and refactorings.

Thanks a lot!
Rune

Hi Again

1)
I'm sorry but I don't understand what you mean about the messages. All error messages are translatable and gives hints to the developer when he has made a mistake in the schema definition.

2)
Drupal 6 does not support extraction of non literal strings, so I've figured that instead of creating yet another .pot extractor I could just write the strings as php code to a dummy file. This is not pretty but is the only way to make translation works seamlessly with the .pot extractor tools. See autoadmin_dynamic_t() and autoadmin_schema_write_strings_as_code(). This only runs during flushing of cache, and makes everything work as it should.

New version attached.
Cheers
Rune

Added language files for autoadmin. Please review this version instead.

EDIT: disregard, you replied with #37 while I wrote this.

1)
Since $foreign_schema['title_plural'] already is a translated string, to use this...

$links[] = array(
      'title' => t('Edit !title_plural', array('!title_plural' => $foreign_schema['title_plural'])),
);

...would add an unnecessary translation burden. You could argue that in some languages it might be more correct with the reverse ordering of words, but I think it is a good trade off.

2)
Message is never empty because the variable $msg is never empty. It is concatenated to the optional [SCHEMA: foo] [FIELD: bar]. But there is always a message.

3)
I have double checked the 84 uses of t() and autoadmin_dynamic_t() and they are all a conscious choice. The places I am not using placeholders are places where it would add an unnecessary translation burden.

Thx for feedback!

LOL...this is cool:)

I'm by no means a linguist, did not even know what OSV meant before two minutes ago. Found this at http://jurisdynamics.blogspot.com/2008/02/themes-and-rhemes-and-xsv-smil....

OVS and OSV languages are "fantastically rare...found "maybe eight or ten languages with OVS order in simple transitive clauses . . . and maybe four with OSV.

So the current version does not support "Yoda" mode, as he apparently speaks mostly OSV;)

But I still strongly feel that with the extra amount of translation strings this would give (maybe about 20 for a standard size schema), it is not worth it.

But you are the boss and if you insist I will add something like a 'advanced translation' => TRUE setting, that translates with placeholders. But that seems very much to me to be a case of YAGNI. I would like to release it like it is now, and first change it if it becomes a problem. There are several other features/enhancements that in my eyes would enhance the module more.

Thx!
R

added support for Central Morocco Tamazight, and "Yoda Mode" :)

Generating php code in autoadmin.dynamic_strings.inc solves the problem of strings not being extracted with .pot tools.

This is how I solved translation of dynamically generated strings, so it works with drupal core modules:

Please consider the following code:

$foo['bar'] = 'Some text.';
var_dump(t($foo['bar']));

This is translatable. The user can browse to "admin/build/translate/search", search for "Some text.", translate it and it works fine. However it generates two problems:

1) Errors shown on "admin/build/translate/export" page. This I solved with the autoadmin_dynamic_t() function that wont trigger these errors.

2) "Some text." is not extracted to the .pot file generated at "admin/build/translate/export". This I solved with generating dummy php code and writing it to autoadmin.dynamic_strings.inc. See the autoadmin_schema_write_strings_as_code() function.

1) + 2) autoadmin.dynamic_strings.inc is regenerated by the autoadmin_schema_write_strings_as_code() function every time the cache is cleared, so it contains _all_ the strings from _all_ modules implementing autoadmin.

About using dummy file

Hey found this here: http://api.drupal.org/api/function/t

In some cases, modules may include strings in code that can't use t() calls. For example, a module may use an external PHP application that produces strings that are loaded into variables in Drupal for output. In these cases, module authors may include a dummy file that passes the relevant strings through t(). This approach will allow the strings to be extracted.
...
Having passed strings through t() in a dummy function, it is then okay to pass variables through t().

About the module having write access to its own folder

I checked out where t() functions are extracted from, and they have to be in one of the following folders:

• modules
• sites/all/modules
• sites/all/themes
• themes

I have neither found nor searched of precedence for a module writing to its own folder, but it is the only option to get the translation to work! Please bear in mind that if the developer is not happy with this he can simply add:

$schema['album'] = array(
    // Dont translate title, title_plural and description for schema and fields
    'translate' => FALSE,
    // ...
);

And no writing to disk will occur!!!

I think I've exhausted all the options on this, and landed on the best and most transparent solution.
Cheers
Rune

No, you are completely right. All strings from all the modules are written to sites/all/modules/autoadmin/autoadmin.potx.inc. So one file needs to be writable!

Writing to autoadmin.potx.inc can be disabled with the 'translate' => FALSE setting, and you have to define your module as the example in (#34.4.1).

The processing and pluralization of database field names to strings makes the job much easier for the developer! If I have a database field called "company" it is a fair assumption that I want the label in the form to be called "Company", and want to use the pluralized string "companies" other places. If I can't write to autoadmin.potx.inc I cant supply this feature. If you have another solution to this catch-22 i'd love to hear it :)

Cheers
Rune

I occurred to me that CCK must face the same problem with translating field names inputted by the user. So I decided to check out how CCK handles the problems and it doesnt really. CCK is floating with code like:

'#title' => check_plain(t($field['widget']['label'])),
array('%label' => t($group['label']))),
content_filter_xss(t($group['settings']['form']['description'])),
return t($field_types[$field['type']]['formatters'][$this->options['format']]['label']);
// ...
// ...
// ...

So this code is bound to trigger warnings when extracting pot files.

Drupal is not really build for doing this kind of thing, and at least some kind of workaround must be applied.

Did also check out the Views module. Here is what I found:

$options[$type] = t($info->name);
$options[$type] = check_plain(t($info->name));
return check_plain(t($output));
return t($this->options['perm']);
$storage[$key] = t($value);
$storage[$option] = t($definition['default']);
$title = t(ucwords($type));
return $this->render_link(t(check_plain($value)), $values);

Asking the modules for all the strings to translate allready works! It is just a LOT more annoying to write.

Sorry for repeating, but the difference is shown in the two examples below. In the first one _all_ strings are written. In the second one, autoadmin will decide what the strings should be. This decisionmaking is based on the names of the database names and database fields. The result is exactly the same.

1)

function example_autoadmin() {
$schema['album'] = array(
    // Dont autogenerate names based on table and field name
    'autoname' => FALSE,
    // Dont translate title, title_plural and description for schema and fields
    'translate' => FALSE,
    'title' => t('Album'),
    'title_plural' => t('Albums'),
    // Must repeat description here, because its against the drupal way
    // to use the t() function in .install files.
    'description' => t('Recorded albums'),
    'path' => 'admin/labels/albums',
    'alias' => 'title',
    'fields' => array(
      'id' => array (
        'title' => t('Id'),
        'title_plural' => t('Ids'),
      ),
      'title' => array(
        'title' => t('Title'),
        'title_plural' => t('Titles'),
      ),
      'description' => array(
        'title' => t('Description'),
        'title_plural' => t('Descriptions'),
      ),
      'weight' => array(
        'title' => t('Weight'),
        'title_plural' => t('Weights'),
      ),
      'label_id' => array(
        'title' => t('Label'),
        'title_plural' => t('Labels'),
      ),
    ),
    'relations' => array(
       'id' => array(
        'type' => 'has_many',
        'foreign table' => 'link_artist_album',
        'foreign key' => 'album_id',
        'foreign alias' => 'album_id',
      ),
      'label_id' => array(
        'type' => 'has_one',
        'foreign table' => 'label',
        'foreign key' => 'id',
        'foreign alias' => 'name',
      ),
    ),
  );

2)

$schema['album'] = array(
    // Autogenerate names based on table and field name.
    'autoname' => TRUE,
    // Translate title, title_plural and description for schema and fields.
    'translate' => TRUE,
    'path' => 'admin/labels/albums',
    'alias' => 'title',
    'relations' => array(
       'id' => array(
        'type' => 'has_many',
        'foreign table' => 'link_artist_album',
        'foreign key' => 'album_id',
        'foreign alias' => 'album_id',
      ),
      'label_id' => array(
        'type' => 'has_one',
        'foreign table' => 'label',
        'foreign key' => 'id',
        'foreign alias' => 'name',
      ),
    ),
  );

The only drawback is that autoadmin needs to be able to write to one single file. I'm all for purity of code but I also like nice features and ease of use. In this case I think it is ok to write to the file.

NB: I have found several other modules already in Drupal that are writing to their own module directories. If you want I can give you the specifics.

Thx
Rune

I agree it would not work on localize.drupal.org, because the strings are collected inside autoadmin. Removed all functionality that relates to dynamic string generation.

Cheers
Rune

I still miss my cool dynamic string generation (might move it to a generator script), but its more drupalish this way and its garantied to work 100% with translation. So I concur.

Its been some ride, thx a lot for all your help and comments!!! Autoadmin wouldn't be as good without them! Sorry if I've been stubborn along the way!

Take care
Rune