Skip to content
Yannick Warnier edited this page Mar 21, 2024 · 28 revisions

Coding conventions

Table of Contents

Default conventions

Chamilo follows PSR-1, [PSR-2] (https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md) and to some measure (to be increased progressively), the Symfony Coding Standards, the Twig Coding Standards.

PSR-2 does not set a guideline for the properties names such as $StudlyCaps, $camelCase, or $under_score. We are gradually using $camelCase (and replacing other styles) since v1.10 of Chamilo, in order to have a better code consistancy, because of the interactions we have with other libraries such as PEAR, Silex, Symfony2 components, Doctrine (to name a few) that use this convention.

The following rules define additional elements, not defined in PSR-2 (which includes PSR-1, which includes PSR-0).

Language

  • Use English for comments and variables/function names.

The code, comments and developer instructions you write should be in English, no matter what your native language is. It's great if manuals are available in many languages, but code with comments and variables in English, French and Spanish is hard to understand. For feature requests and bug reports, English, French and Spanish are accepted, although English is preferred.

Code documentation

  • Document your code using PHPDoc or one-liners
  • Add licensing terms link to all new files

For many reasons, it is important to write good documentation. All page- or block-level code documentation should be generated respecting PHPDoc. Adding comments with ======= or ///////// separators is a deprecated syntax and is not accepted for new comments. Please remove these on sight.

All files in the Chamilo code should always show the following second line (after the PHP opening tag):

/* For licensing terms, see /license.txt */

All code in Chamilo must be written and shared under the GNU/GPLv3+ license or a compatible license (in the exclusive case of including external libraries which already have their own license). No software that has a license incompatible with GNU/GPLv3+ will be accepted in the Chamilo code.

Syntax

require vs include

Please use:

require 'main/inc/global.inc.php';

or

require __DIR__.'/../../main/inc/global.inc.php';

don't use:

require('main/inc/global.inc.php');

include and require are language constructs, not functions. Although they do work with parenthesis, the fact of adding the parenthesis actually makes PHP believe it is a function, and as such go through the additional process of trying to find the function reference.

When you get a chance to safely fix these in some script you are modifying, please do so.

An include in PHP is the same as a require, except that require is stricter, meaning that if you try to include a file that does not exist, include will generate a warning message, while require will generate a fatal error.

Unless you want to try out inclusion of some files, and you actually catch the warning messages so you can deal with them, there's no reason at all to be using include. include obfuscate problems in your code, preventing you from realizing that some of the libraries you are trying to use are not actually there. Sometime later, side-effects might appear because the file you are supposed to be including is not actually included but you don't know it.

Don't use include or include_once, use require or require_once instead!

Short PHP tags

  • Do not use <?

Use the correct, complete php tags: start with and end with ?> (except at the end of a file). Do not use short tags like <?, <?= or ASP-style tags <%, <%= as these rely on php.ini configuration settings short_open_tag and asp_tags. These settings can cause errors when they are off and Chamilo code assumes that they are set to on.

The php.ini comments advise not to use short tags or ASP-style tags:

 NOTE: Using short tags should be avoided when developing applications or
 libraries that are meant for redistribution, or deployment on PHP
 servers which are not under your control, because short tags may not
 be supported on the target server. For portable, redistributable code,
 be sure not to use short tags.

Closing PHP tags

  • Do not using file-level closing PHP tags ?>

As per PSR-2, we ask not to use the closing ?> tag at the end of a script. This is due to the fact that, if there are too many blank lines at the end of a script (more than one), after the closing tag, then your script will trigger the sending of HTTP headers to the user, preventing the system to send the appropriate redirect headers or other headers bound to the session (sending of cookies).

For example,

<?php
// Comment.
$var = 123;
?>

This code will send headers to the user although we are not at the end of the PHP process (there are still more scripts to be loaded in this chain), and consequently trigger the apparition of strange warning message, and the loss of the user's session. If you intentionally omit the closing tag (which in turn could trigger problems for parsing the PHP code through XML, but who does that?), then you ensure nothing will send blank lines to the user's browser, because the blank lines are considered as part of the PHP code and, as such, are ignored.

<?php
// Comment.
$var = 123; 

Note that PHP itself will automatically ignore one blank line after the closing PHP tag. Just one.

Whitespaces

Adding a blank line between lines of code creates paragraphs of code and helps to visualize it. You don't want to read books that have a thousand pages filled with line after line and no breaks; nobody wants to read code like that either.

Also, using spaces around operators and after comma's can help to make the code more readable. However, most people write their function brackets without a space between the function name and the brackets:

   function something($param1, $param2) 
   {
       if ($param1 and $param2) {
           // Code
       }
   }

When configuring your PHP editor, make sure it replaces all tabulations by 4 blank spaces. In PHPEclipse, this can be done by following the sequence: Window -> Preferences -> PHPEclipse -> PHP -> Appearance -> Display Tab Width: 4 and PHPEclipse -> PHP -> Typing -> Insert spaces for tab. This ensures files are seen the same way under all editors (including terminal-based editors).

For all uses of binary operators (=, +, etc), these should be surrounded by spaces, with the exception of the concatenation operator (.) which must not be surrounded by spaces.

Follow Twig code conventions

If you have to use variables, use lower cased and underscored variable names:

{% set foo = 'foo' %}
{% set foo_bar = 'foo' %}

See http://twig.sensiolabs.org/doc/coding_standards.html

Inline JavaScript and CSS

Classes

  • Use generic-css-class-names, NOT GenericCSSClass-Names

All CSS classes and IDs should be expressed in lowercase letters separated by hyphen (-).

echo '<div class="generic-class other-class-less-generic">'.get_lang('Term').'</div>';

When using a CSS style specific to a tool, use the name of the tool as a prefix.

echo '<div class="tool-name-something tool-name-something-else generic-modifier">'.get_lang('Term').'</div>';

Include in header

JavaScript and CSS must be added in the header. To do this, prepare your CSS or JS as strings, then inject them into the $httpHeadXtra variable:

global $httpHeadXtra;
$css = '<style type="text/css" media="screen">/*<![CDATA[*/
.mybutton {
  border: 1px solid black;
}
/*]]>*/
</style>';
$httpHeadXtra[] = $css;

(same thing for JavaScript, preferably using the JQuery way of loading a function: $().load(function(){...}))

Although inline CSS is not welcome, it is authorized under certain circumstances

Use of brackets { ... }

For classes and methods, put your opening brackets and closing brackets on an empty line, and at the same indentation

function something($a)
{
    for ($i = 0; $i < $a; $i++) {
        echo "$a <br />";
    }
}

This gives a very clear view of where a loop starts and where it finishes. Some code editors even improve this visibility by tracing a vertical line between the opening and the closing bracket.

Please note that control structures should have their opening brackets on the same line.

Don't use single-line conditions without brackets

Do NOT use:

if ($hello) echo 'goodbye';

Use:

if ($hello) {
    echo 'goodbye';
}

Using a condition on a single line is not a good idea, and [PSR-2] (https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md) kind of implicitly forbids it: "Opening braces for control structures MUST go on the same line, and closing braces MUST go on the next line after the body.", so the logic in Chamilo is to always define the body of a condition on a separate line.

To remain flexible and more quickly readable, the code MUST always include brackets, and should not include single-line conditional statements.

This helps delimiting clearly the boundaries of the condition even if the indentation is broken, without requiring the 1-second brain loop to figure out if that's really the single statement included and whether it should be more indented or not.

Use of internal functions

Try to use Chamilo's internal functions (API) as much as possible. There are three main libraries that you will need to review before starting to code with Chamilo: api.lib.php, database.lib.php and security.lib.php

The database library provides a complete abstraction layer that allows you to use all the necessary MySQL functions from Database:: prefixed static methods, to the exception of mysql_query(), that is replaced by api_sql_query(), defined in api.lib.php.

Don't use mysql_fetch_array(), but use Database::fetch_array() (as of Chamilo 1.8.*)

Due to an uncontrolled past history of coding, some calls to mysql_query() are still passed through the api_sql_query(). Although this is not really a problem in itself, we are making efforts to see this function passed to Database::query() instead, to make sure everything related to the database layer will be handled by the Database class. If you are updating some code in Chamilo, please make sure you change all api_sql_query() calls to Database::query(), to ease the move to an abstraction layer in the future.

api_get_setting('X') === 'true'

Due to a certain level of freedom added to the platform settings in the database, true and false values are written as 'true' and 'false' strings in the database. As a result, api_get_setting('X'), where X has a value of either true or false, will return 'true' or 'false', not TRUE or FALSE.

For this reason, these coding conventions include the fact that, when calling api_get_setting() on a boolean value, you compare it directly to the strings 'true' or 'false', and you do NOT use the if(api_get_setting('X')) form. Explicit, not implicit.

If a variable is available in configuration.php instead of the database, use api_get_configuration_value('X') to get it. This function also supports acess-urls, if you use them, if the setting in configuration.php uses a sub-array with the access-url ID (check the function comments for more details/examples).

Variables naming

New variables must be expressed in $camelCase (starting with lower case and using a capital letter for each new word).

Although you might find a lot of $lower-case-variables, these are remains of an older coding convention. Wherever you are completely sure about the use of a specific variable expressed in lower-case, you are invited to change it to camel case in a separate commit prefixed with the "Minor - Coding conventions" message.

Language variables should all use camel-case and start with an upper-case character. However, these are used literally through the get_lang() function, so there should be no confusion about their usage.

CONSTANTS names MUST be in all-upper-case letters

Date and time management

These coding conventions have to be applied only for new code (e. g. code written after Feb 17, 2010) that uses dates and times:

  • Any date and time stored in the database must use the SQL DATETIME format
  • Before storing any DATETIME in the database, you MUST call the function api_get_utc_datetime($time) with the date and time you want to store in the database. This function will return you a string under the format Y-m-d H:i:s that you can store directly in the database.
  • Before displaying any datetime, you must call the function api_get_local_time($time, $to_timezone = null, $from_timezone = null) with the datetime you want to convert.

To know more about date and time management in Chamilo, click on Date and time management

Database

Entities

Since Chamilo 1.10, we have been using entities more and more. These can be found in the src/chamilo/[Some]Bundle/Entity/ directories. In Chamilo 1.11 and later, you should always use entities unless it makes real sense to use something else.

Using entities generates a series of new requirements in terms of development.

To avoid using the automated naming of indexes by Doctrine, we require any developer to name their indexes. Adding indexes to a table with a name is done the following way:

https://github.com/chamilo/chamilo-lms/blob/7621d9e42e7ca2d7537196bd4fcb21af7b827fb6/src/Chamilo/CourseBundle/Entity/CAnnouncementAttachment.php#L8-L19

However the name of the index has to follow a certain set of rules (not like "course" in this example):

  • start with 'idx_'
  • contain a shortened version of the table name (ex: 'cannattach_')
  • contain a shortened version of the indexed row(s) (ex: 'cid')

For Foreign Keys, Doctrine doesn't support naming at the moment, so we let Doctrine manage the naming. This can cause some differences in the database structure as reflected by php bin/doctrine.php orm:schema-tool:update --dump-sql

Queries format

Since Chamilo 1.11.0, pretty much every type of object can be considered an Entity. As such, their queries should use simple entity getters/setters.

Note:

  • Database::getManager() should not be used in entities or repositories of any kind. Instead, this call should occur before referring to a repository or entity, from the calling script
  • api_get_setting() should not be called from entities. Entities should remain independent of specific library code of Chamilo
  • the QueryBuilder (see below) should not be called from entities. Any requirement for QueryBuilder inside an entity demonstrates a bad definition of the entity. QueryBuilder can be called from Repositories. Ideally, it should only be called from repository objects.

This commit is a good example of how things should be cleaned up if they do not match the rules above.

Default entity fetching

Getting the items through entities will take the following form:

use Chamilo\UserBundle\Entity\User;
use Chamilo\UserBundle\Repository\UserRepository;

// Initialize the entity manager (only once per script)
$entityManager = Database::getManager();

// Generate a repository object of the type you need, using a @var header so 
// that your code editor can easily know what we're talking about and can 
// auto-offer the proper code completion (in this case all the methods 
// available for this object).
/** @var UserRepository $repository */
$repository = $entityManager->getRepository('ChamiloUserBundle:User');

// Find one specific item in this repository (by its primary key as defined 
// in the entity, by default). Again, the @var line helps your IDE give you
// auto-complete
/** @var User $user */
$user = $repository->find(56);

Likewise, if you'd like to check a list of all elements in the repository, you can simply do something like this:

/** @var UserRepository $repository */
$repository = $entityManager->getRepository('ChamiloUserBundle:User');
$users->findAll();
/** @var User $user */
foreach ($users as $user) {
    echo $user->getId().PHP_EOL;
}

Note that this particular example is really a bad idea on portals with a lot of users, as Doctrine will build a complex structure for every user, ending up in a very large amount of data being queried very slowly.

Getting related items/entities

Normally, entities define all the relationships they have with others. They do that through th entity definition. For example, in src/Chamilo/UserBundle/Entity/User.php, you will find this:

    /**
     * @ORM\OneToMany(targetEntity="Chamilo\CoreBundle\Entity\SkillRelUser", mappedBy="user", cascade={"persist"})
     */
    protected $achievedSkills;

This actually defines the exact JOIN that Doctrine has to make to get all elements from the SkillRelUser table. And then a bit further in the entity, you'll find:

    /**
     * Get achievedSkills.
     *
     * @return ArrayCollection
     */
    public function getAchievedSkills()
    {
        return $this->achievedSkills;
    }

So if you want the SkillRelUser table elements for the user you fetched in the section above, you could just continue the code snippet of the previous section with:

// $user = $repository->find(56);

$skillRelUserList = $user->getAchievedSkills();

// and then you can browse through each skill like this:
/** @var SkillRelUser $skillRelUser */
foreach ($skillRelUserList as $skillRelUser) {
    echo $skillRelUser->getSkill()->getName();
}

Of course, in this case, getSkill() and getName() are defined in the matching entities.

You could even extend this one step further and get specific comments on skills (so joining the tables user, skill_rel_user, skill and skill_rel_comment:

foreach ($skillRelUserList as $skillRelUser) {
    echo $skillRelUser->getSkill()->getName();
    $comments = $skillRelUser->getComments();
    /** @var SkillrelComment $comment */
    foreach ($comments as $comment) {
        // ... echo ...
    }
}

If you are worried about performance, know that the default mechanism in Doctrine is to be "lazy". That is, it will not fetch the related resources unless called. An additional setting (under implementation in Chamilo) is called "EXTRA LAZY" and still reduces the probability of auto-fetching. Also know that Doctrine comes with many possibilities of caching that balance the extra weight some entity-based queries can take.

Fetching in specific order

In many cases, getting a list of entities in the default order (by primary key) will not be an option. You want to be able to order your results with specific parameters. Do that with criteria:

// Instantiate your repository here
/** @var User $user */
$user = $repository->find(56);
$criteria = \Doctrine\Common\Collections\Criteria::create();
$skillRelUserList = $user->getAchievedSkills();

$criteria->orderBy(['id' => 'DESC']);
$criteria->setFirstResult(1);
$criteria->setMaxResults(5);

$skillRelUserList->matching($criteria);

/** @var SkillRelUser $skillRelUser */
foreach ($skillRelUserList as $skillRelUser) {
    // ...
}

Criteria can be extended with "where" conditions, like this:

$criteria->where(Criteria::expr()->contains('argumentation', 'text'))
     ->andWhere(Criteria::expr()->eq('argumentation', '1'))
     ->orWhere(Criteria::expr()->....('...','...'));

Using queryBuilder for complex queries

In some other cases, your queries will be much more complicated, making it impractical to go through normal entities, repositories and criteria.

However, you cannot use direct SQL queries, because these are often not portable between database environments.

Using the query builder is the solution to that:

$repository = $entityManager->getRepository('ChamiloCoreBundle:Course');
$queryBuilder = $repository->createQueryBuilder('c');
$courses = $queryBuilder
    ->select('c')
    ->innerJoin('ChamiloCoreBundle:CourseRelUser', 'cu', Join::WITH, 'c.id = cu.course_id')
    ->innerJoin('ChamiloUserBundle:User', 'u', Join::WITH, 'cu.user_id = u.id')
    ->where('c.code = :code')
    ->andWhere('u.firstname = :firstname')
    ->setParameter('code', 'ABC')
    ->setParameter('firstname', 'Yannick')
    ->getQuery()
    ->getResult()
;
/** @var Course $course */
foreach ($courses as $course) {
    // ...
}

Using var_dump() with entities

Although you might be tempted to print everything you got in your entity or repository object "just to make sure" you got everything you need, please remember that Doctrine objects can take large amounts of memory and that printing them with var_dump() (or print_r()) will actually trigger the fetching of all the related items that have not been fetched because they were not required.

As such, when using var_dump() on some large entity, you might very often exhaust all the available memory for the execution of your PHP script and end up with a "Memory limit exceeded" error.

Try to only var_dump() one item at a time, and if possible not an entity related to many other entities (like users).

Deprecated query formats

Queries formatted like this:

    $sql = "UPDATE $tbl_admin_languages SET original_name='{$_POST['txt_name']}'
            WHERE id='{$_POST['edit_id']}'";
    $result = Database::query($sql);

or like this:

        $resultData = Database::select(
            'COUNT(1) AS count',
            Database::get_main_table(TABLE_MAIN_USER),
            [
                'where' => ['id = ?' => intval($userId)],
            ],
            'first'
        );

...are all transition forms of new queries that are getting deprecated progressively. Please help us move these to the correct forms described above.

Repositories

Defining repositories (referenced in the entities section above) must be done in a directory parallel to Entities:

  • /src/Chamilo/UserBundle/Entity
  • /src/Chamilo/UserBundle/Repository

At some point in Chamilo, we used another strategy (putting the Repository directory inside the Entity directory), but this is a deprecated style that we are changing. See https://symfony.com/doc/3.4/doctrine/repository.html

Use of backticks

Backticks ( ` ) are a very bad MySQL-only habit. Don't use backticks at all. This will improve your respect of the SQL standard and make you find meaningful names for your tables and your fields, that will avoid them being misunderstood for a MySQL reserved keyword. When copy-pasting some code from phpMyAdmin, make sure you remove all backticks.

time(), not NOW()

  • Use Chamilo's date functions, do NOT use MySQL's NOW() (ever)

time() is a PHP function that generates a UNIX timestamp (the number of seconds from 1/1/1970 00:00:00 until now). NOW() is a MySQL function that generates a timestamp to the required format to fill the field it is put in.

Combining the two functions can lead to time matching problems, because time() depends on Apache and NOW() depends on MySQL server. These two programs can have different times (they can be on different servers or simply have a different configuration).

Because the process in Chamilo is based on PHP interpretation, time() must be used instead of NOW(), even when simply filling database logs fields. This will ensure that time-based values are all based on the same time source, and will make reliable statistical queries possible.

Database: INT, not INT (11)

  • Do NOT use the useless MySQL syntax INT (11). Only use INT.

INT (11) is a markup created by phpMyAdmin in the automated table creation query generation, but is actually useless in the sense of PHP + MySQL. To be truly portable, declare INT fields as INT, not INT (11).

This is a quote from http://dev.mysql.com/doc/refman/5.0/en/numeric-types.html:

The display width does not constrain the range of values that can be stored in the column, nor the number of digits that are displayed for values having a width exceeding that specified for the column. For example, a column specified as SMALLINT(3) has the usual SMALLINT range of -32768 to 32767, and values outside the range allowed by three characters are displayed using more than three characters.

Database: INT UNSIGNED for primary and foreign keys

When declaring a primary or foreign key, please use the "UNSIGNED" qualifier. Although few of the current fields have been declared as unsigned, there is no use for the negative side of integers.

Unsigned integers, in MySQL, go from 0 to 4,294,967,295 (which is a considerably safe primary key allowance, given that MySQL starts to have problems dealing with new records over the 1,000,000,000 records without optimization)

Database: Foreign keys implicit declaration

When declaring foreign keys in the database, the relation is considered implicit, and should be using the name "id" in the table where it acts as a primary key, and "[origintable]_id" in the table where it is used as a foreign key. The element type must be the same.

Database: SQL reserved keywords

In the different implementations of SQL, some words happen to be reserved for specific use, like uid for internal IDs in Oracle or "time" in PostgreSQL. Although Chamilo 1 doesn't support anything else than MySQL, we are definitely working on having support for other database management systems. This means that we have to avoid using reserved keywords, even if they are reserved only in another database system. If you have doubts, you can check the words on this page: http://www.petefreitag.com/tools/sql_reserved_words_checker/?word=time

An easy trick to avoid using reserved keywords by mistake is to simply put the name of the table (or any short version of it) as a prefix to the field.

An essential part of not using reserved keywords is to avoid using the special MySQL character "backtick" () which doesn't work in any other database management system, and is specially made to allow you to use reserved keywords in MySQL. We don't want that, so we don't want to see any in a SQL query. Backticks are generally added because you made a copy-paste from phpMyAdmin. Avoid that. Care about your queries!

Security

We do our best effort to make Chamilo the most secure LMS around. This is kind of acknowledged by the number of CVEs and the time to fix. To maintain a high standard of security in Chamilo, we need all developers to be on track as to what is needed to maintain it that high. The following sections try o give you a short but important overview of what we expect from you.

Security: Filter SQL close to the database

It is mandatory to always make sure the data is filtered properly before being put in the database.

With PHP 7.0 and 7.1, the principle of "Type declaration" was added to PHP. Chamilo 1.11.14 and above depend on a minimum PHP version of 7.2, so Type declaration, which is a superior filtering mechanism (more strict than most others) should be used whenever possible in the development of Chamilo 1.*.

By using type filtering at the function/method declaration level, we delegate filtering at the script level and block any non-matching variable from going any further (the function/method will just not execute if the type doesn't match). For new functions/methods, please always use Type declaration.

In other cases, or where Type declaration is not practical, please always filter your data inside the function which is calling the database query. This way, you ensure your database is safe, no matter where the variables came from.

For strings, use

$filteredString = Database::escape_string($unfilteredString);

For integer values (or other numerical values) use the corresponding converter/type caster:

$filteredInt = (int) $unfilteredInt;
$filteredFloat = (float) $unfilteredFloat;

This is very important, as using the string filter on integers might open the way for an SQL injection (refs #7440)

In Chamilo 1.11.x and up, 3 methods commonly used will auto-filter the contents of the queries, provided you use them correctly:

  • SomeEntity->merge() and SomeEntity->flush() will make sure that the data getting into the database is filtered SQL-wise, because they use entities, and entities contain a definition of the data type that is expected in the database, so it can do the proper filter. So no need to filter data in your entities themselves.
  • DatabaseManager->SomeResource->find() will automatically filter the ID you give to find(). Once again, this is because we are using entities behind that, so Doctrine (our Object Relational Mapping framework) knows what type of data it should be and what filtering to apply
  • Database::insert() uses an SQL statement prepare() method, which also escapes the whole query before executing it, providing you passed all the parameters as an array, as expected

You can find more information on Doctrine security here: https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/reference/security.html

Security: Filter user input that has a chance to get on screen

Type declaration is a required practice wherever possible (wherever we use functions/methods). But this section's content is still accurate, even with type declaration enabled, as some strings might match the string type but still include hacks (as strings).

XSS attacks are always based on using JavaScript to the user. What is less known is that a user can be directed to your site with a URL that includes JavaScript. In this case, the JavaScript can be executed with increased access due to the user's session. Although the effects are often damaging the user's experience rather than the website's safety, it is best to avoid the problem as much as possible.

A Security class is provided inside Chamilo 1.8.x which allows for filtering of several kinds of attacks. We ask you to make use of Security::remove_XSS() on any content that might contain JavaScript to show to the user. This includes messages generated from third party sources.

For example:

echo $_POST['username'];

should be escaped like so:

echo Security::remove_XSS($_POST['username']);

Displayable MUST be filtered just before displaying it, because we don't know exactly where we will show it, so maybe escaping JS and HTML code doesn't make sense because we are going to write the data to a file that will never execute in a browser. So the database might actually contain data that is tainted with XSS hacks. No worries. The important thing is to make sure that when it is printed for a browser to view, XSS is properly escaped.

In Chamilo 2.0 and subsequent versions, Twig (the templating system) has the ability to auto-filter XSS. However, because we consider we should not depend on this mechanism to have secure filtering, we expect this feature to be turned off and the variables to be escaped before sending to the TPL.

Security: CSRF

Cross-Site Request Forgery is also a mechanism by which JavaScript is abusing the user's browser. It is generally triggered from another website to benefit from the "open-session" effect that a user generates by staying "connected" to several sites at any one time. For example, a hackers site might use the user's session to connect to Chamilo and post spam content inside a course's description.

To avoid this, always make sure your forms are protected by a mechanism of controlled submission. The Security class can help you do that, particularly Security::get_token() and Security::check_token().

Translations management

Language terms

Note: We will be moving to gettext for the release of Chamilo 2.0, which should happen in 2018. We will migrate all current translations to it at this point, so don't worry about loosing efforts translating: nothing will be lost.

At the moment, the translation system is located at https://translate.chamilo.org. Strings must be added and translated there, NOT sent through PRs to the code. We then regularly "pull" the translation changes into Chamilo.

Once Chamilo 2.0 is in active development, we will move to another platform at https://translation.chamilo.org, but the translation workflow will then be different from the one below.

In general terms, language terms have to follow these rules in 1.11.x:

  • always start with an uppercase letter [A-Z]: e.g. $Alert;
  • use uppercase letters (not underscores) to mark different words: e.g. $AlertIncomingAttack;
  • when requiring a complex, variable string, use "%s" to enable a substitution, and mention it in the variable title with an X, Y or Z. For example: $HelloXWelcomeToPortalYWhereYouHaveZTrial = 'Hello %s, Welcome to the %s portal, where you have a %s days trial available to you'; ...indicated that you will have 3 variable values that will be inserted inside the string. This form is preferred to partial strings concatenation, as not all languages use the same syntax.

A language term should always be as expressive as possible. Don't try to join several strings into one. Something like: get_lang('Dear ').$username.get_lang('WelcomeToPortal').get_lang('WeHopeYouHaveFun').get_lang('BestRegards').$teamname; will not work out in languages with different grammatical rules like Japanese, Arabic, Quechua and many more.

Instead, use a context-full string like this "Dear %s, welcome to this portal. We hope you'll have fun learning through the courses made available to you. Best regards, %s".

The %s will then be replaced in the call to get_lang(): sprintf(get_lang('GeneralWelcomeToPortal'),$username, $teamname);

This is because, in other languages, the order of the sentences may not be exactly the same. You might make an introduction as "Yannick San E" (as compared with "Dear Mr Yannick"), instead of what could be possible with a single get_lang('Dear'), which would force you to use "E San Yannick", which doesn't mean anything in Japanese (it would be the equivalent of "Yannick Mr Dear" in English)

Language terms in Twig (templates)

When using Twig, instead of preparing the translated variable inside PHP and for Twig, you should use the Twig filter "get_lang" and give the appropriate variable (without the $ sign), like this:

  <div class="salutation">{{ "Hello" | get_lang }}</div>

This will automatically translate the message to the current language.

If, as suggested above, you use a composed language variable (using %s signs), then you can call the get_lang filter as well, but you need an extra step (and you need the replacement of "%s" to have been assigned to a Twig variable:

  <div class="salutation">{{ "HelloX" | get_lang | format(show_user_info.user_info.complete_name) }}</div>

Multiple replacements can be obtained through the use of multiple parameters separated by commas in the format() call.

Error management

Debugging messages

All debugging messages should be kept locally, unless they represent a long-term debugging session (spanning several days). The error_log('debug message'); method is preferred to the echo 'debug message'; method because it doesn't break the user display, even if forgotten in the code. To check the results of error_log(), check your Apache's error log. Under Ubuntu, you can have a live view of the debug console using the following command pattern:

sudo tail -f /var/log/apache2/yoursite-error.log

Error obfuscation

When using the @ symbol to obfuscate errors, make sure you check $php_errormsg (only available if php.ini track_errors setting is on) to deal with the error. In this specific case, adding a call to error_log() is an excellent idea, so that the error message is not "lost".

See [http://www.php.net/manual/en/reserved.variables.phperrormsg.php the php documentation page] for more info about this variable.

For example:

  $res = @rmdir('/var/www/chamilo/archive/ABC');
  if ($res === false) {
    error_log(__FILE__.' line '.__LINE__.': '.(ini_get('track_errors')!=false?$php_errormsg:'error not recorded because track_errors is off in your php.ini'));
  }

Fatal, Warning and Notice -level error messages

We assume that any code you add to Chamilo will not generate Fatal nor Warning errors. You can try that by putting your portal in "test mode" in the configuration page. Notice-level error messages are accepted when submitting new code, but they are intended to disappear when the code is properly reviewed.

To show notice-level warnings, it is necessary to change the code of main/inc/global.inc.php to substitute E_ALL & ~E_NOTICE by E_ALL.

Git and Contribution process

Git

Git and inclusion of external libs: commit original first

Although most external libraries are included through Composer now, this documentation still serves for those libraries unknown to Composer.

When integrating an external library, plugin or any piece of code maintained by a third party, make sure the first integration commit is the integration of the original version of the external code. Only commit the adaptation to the Chamilo code after that first "original" commit. This way, it will be much easier, later on, to detect the customization that has been made to integrate the external library.

A natural consequence of that (but that might not be obvious for everybody) is that you should always send one separate commit to include a new library. It should not be sent together with a Chamilo code change in one group commit.

Please avoid sending the code examples or unit tests that generally come with these libraries. They will take space and possibly introduce security flaws in the Chamilo code.

Starting in 1.10.0, Chamilo LMS uses Composer and Bower for the inclusion of PHP libraries and JS libraries, respectively. If the library you are including is available through either Bower or Composer, you are required to use it (and update composer.json) as a source rather than inserting it into Git directly.

Other Git concerns

A commit should be something "atomic", that you can send as one piece and that you can remove in one piece without affecting the rest too much. If fixing a small bug, a commit could be a few lines of changes in one or several files.

If, during the bugfix, you find code that is not developed in respect with these coding conventions, your code style changes must go into a separate commit, preferrably starting with the keyword "Minor - " so reviewers know this commit doesn't affect the code logic (and cannot affect the code logic).

Submitting code style changes

When sending changes in coding style (for example increasing the indentation for a whole function, a whole class or a whole file, please make sure you send a commit with a "Minor: code style" message separately, so comparing the code between an initial file and another version with the modified code is not too difficult. As an example, a mixed code commit can be seen here (very difficult to distinguish the actual code changes).

Clone this wiki locally