arlmedia [arlmedia]

Hello, and thanks for applying for a CVS account.

May you report the difference between the proposed module, and http://drupal.org/project/dictionary, http://drupal.org/node/43612?

subscribing

Thanks for your reply.

1. See the coding standards (http://drupal.org/coding-standards) to understand how a module should be written. In particular, see how the code should be formatted; how functions, constants, and Drupal variables declared from the module should be named; which Unicode functions a module should be using.

2.   require_once('bdic.module');
   

   require_once() would try to load the module file from the current directory, which the Drupal root directory, not the directory containing the module. There is a Drupal function that should be used to load a module, in cases like this.

   For the same reason, require_once('bdic.admin.inc'); doesn't work too. Also in this case, there is a specific Drupal function to use.
   It's then bad design to split code in two different files, when the second file is always included from the first. In such cases, it is better to merge files, or change which functions are including in the first file, and in the second file.
   To include the module file from bdic.admin.inc is useless; the module file is already loaded, when bdic.admin.inc will be loaded.

3.   $sql = "select * from {role} where name = 'bdic teacher'";
     $res = db_query($sql, array());
     //if it does not exist, then create it
     $arr = db_fetch_array($res);
     if ($arr === FALSE) {
       $params = array('bdic teacher');
       $sql = "insert into {role} (name) values ('%s')";
       db_query($sql, $params);
   

   SQL reserved words must be written in uppercase letters.
   If you are checking if an entry exists, then the code should be like the following:

     if (!db_result(db_query("SELECT 1 from {role} where NAME = 'bdic teacher'")) {
       $params = array('bdic teacher');
       $sql = "INSERT INTO {role} (name) VALUES ('%s')";
       db_query($sql, $params);
   

4.   $items['admin/content/bdic/entry/add'] = array(
       'title' => 'Add word',
       'description' => 'Add one entry and translations to the dictionary.',
       'page callback' => 'bdic_add_lemma',
       'page arguments' => array(),
       'access arguments' => array('administer bdic'),
       'type' =>  MENU_LOCAL_TASK | MENU_VISIBLE_IN_BREADCRUMB,
       'file' => 'bdic.admin.inc',
       'parent' => 'admin/content/bdic'
     );
   
     // show all fields in the table next to a delete, edit link
     // show an add field add the end of the page
     // in the case of deletion show a warning regarding the affected rows
     $items['admin/content/bdic/fields'] = array(
       'title' => 'Fields',
       'description' => 'Edit fields as classifiers for bdic translations.',
       'page callback' => 'bdic_show_fields',
       'page arguments' => array(),
       'access arguments' => array('administer bdic'),
       'type' => MENU_DEFAULT_LOCAL_TASK | MENU_LOCAL_TASK | MENU_VISIBLE_IN_BREADCRUMB,
       'weight' => -9,
     'file' => 'bdic.admin.inc',
     'parent' => 'admin/content/bdic'
     );
     $items['admin/content/bdic/fields/edit/%'] = array(
      'title' => 'Edit the field',
      'page callback' => 'bdic_edit_field',
      'file' => 'bdic.admin.inc',
      'page arguments' => array(5),
      'access arguments' => array('administer bdic'),
      'type' => MENU_CALLBACK | MENU_DEFAULT_LOCAL_TASK,
      'parent' => 'admin/content/bdic/fields'
   
     );
   

   • MENU_DEFAULT_LOCAL_TASK, and MENU_LOCAL_TASK are mutually exclusive, as are MENU_DEFAULT_LOCAL_TASK, and MENU_CALLBACK.
   • MENU_VISIBLE_IN_BREADCRUMB is never used in menu callbacks definition; the fact the constant is not explained makes me think it's internally used by Drupal, but it should not be used by third-party modules.
   • Other menu callbacks are not correctly defined; menu callbacks of type MENU_LOCAL_TASK require a menu callback of type MENU_DEFAULT_LOCAL_TASK.

   To understand better how menu callbacks are defined, see each implementation of hook_menu.

5. /**
    * Implements hook_access
    */
   
    function bdic_access($op, $node, $account) {
     switch ($op) {
       case "create":
         return user_access('administer bdic');
         break;
       case "delete":
         return user_access('administer bdic');
         break;
       case "update":
         return user_access('administer bdic');
         break;
       case "view":
         return user_access('access bdic');
         break;
     }
    }
   

   The code is not using the correct permissions. Permission to update a content type is given to users who has permission to update the content type of that node, not to whom has permission to administer that content type.
   See hook_access(), and forum_access().

6. function bdic_autocomplete($string) {
     header('content-type: text/html; charset: utf-8');
     if (strlen($string)<2) {
       print drupal_to_js(array());
     }
     else {
       $matches = array();
       $result = db_query_range("SELECT distinct word from {bdic_lemma} WHERE Lower(word) LIKE Lower('%s%%')", $string, 0, 20);
       while ($lemma = db_fetch_object($result)) {
         $matches[] = check_plain($lemma->word);
       }
       $result = db_query_range("SELECT distinct translation from {bdic_translations} WHERE Lower(translation) LIKE Lower('%s%%')", $string, 0, 20);
       while ($translation = db_fetch_object($result)) {
         $matches[] = check_plain($translation->translation);
       }
       // sort matches and remove the doubles
       sort($matches, SORT_STRING);
       $matches2 = array();
       foreach ($matches as $m) {
         if (!array_key_exists($m, $matches2)) {
           $matches2[$m] = $m;
         }
       }
   
       drupal_json($matches2);
     }
   }
   

   • Autocomplete callbacks don't set the content type.
   • The coding standards report which Unicode string functions should be used.
   • For queries using DISTINCT, there is a Drupal function (db_distinct_field()).

7.   header('Content-type: application/txt');
     header('Content-Disposition: attachment; filename="my_dictionary.txt"');
   

   Drupal has the function drupal_set_header().

8. Permissions are in the form <ver> <object>, not <noun> <noun>.
9. In all the strings appearing in the user interface you should be using BDIC, as it's an acronym.

10. /**
     * Implements menu_alter hook
     */
    

    The correct comment is

    /**
     * Implements hook_menu_alter().
     */
    

I am closing this application due to lack of replies.

When a fixed version of your module is ready, please re-apply fresh.

Re-closing.

Please read the following links as this is very important information about CVS applications.

• Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for these applications.
• Please read Migrating from CVS Applications to (Git) Full Project Applications and Applying for permission to opt into security advisory coverage on how this affects and benefits you and the application process. In short, every user has now the permissions necessary to create new projects, but they need to apply for opt into security advisory coverage. Without applying, the projects will have a warning on projects that says:
  

  This project is not covered by Drupal’s security advisory policy.