One of a series of test instances for migrating the Koha Wiki MediaWiki database.
For the current Koha Wiki, visit https://wiki.koha-community.org .Coding Guidelines
Basics
General rule: if you submit code that fixes existing code that violates those guidelines, QA can still be passed. You don't have to fix everything. If you want to fix more than just your fix, to respect current guidelines, you're welcome of course. But in this case, please do it in a 2nd patch, to have reviewers being able to distinguish easily what's related to your bugfix and what is related to your guidelines-violation fixing.
Example: you submit a patch that does not respect perlcritic after applying the patch. The QA team will check for perlcritic after your patch, and if the patch does not add a new perlcritic violation, it's OK.
Patches submitted before the introduction of a new rule may pass QA even if they do not meet the current coding guideline requirements on the discretion of the QA team member (Grandfather clause).
Handling disagreements
If a community member has concerns with a patch that:
- Are specifically about coding style (not stated functionality or regressions in other code)
- Are not covered by an existing coding guideline
then the patch and concern should be brought up on the koha-devel development mailing list and/or the next development IRC meeting so a new coding guideline can be added.
Licence
Each file (scripts & modules) must include the GPLv3 licence statement, typically at the top.
# This file is part of Koha. # # Copyright (C) YEAR YOURNAME-OR-YOUREMPLOYER # # Koha is free software; you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by # the Free Software Foundation; either version 3 of the License, or # (at your option) any later version. # # Koha is distributed in the hope that it will be useful, but # WITHOUT ANY WARRANTY; without even the implied warranty of # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details. # # You should have received a copy of the GNU General Public License # along with Koha; if not, see <http://www.gnu.org/licenses>.
Commit messages
When you submit code to the project (using Git) please write useful commit messages. Detailed guidelines on writing commit messages can be found at the wiki page on Commit messages.
Refactoring code
Don't needlessly refactor code, if it ain't broke, don't fix it! Don't waste time on changing style from someone else's style to yours! If you must refactor, make sure that the commit for the refactoring is completely separate from a bugfix.
Bug Numbers
Contributors to Koha must reference a bug number with every commit. This helps us determine the purpose of the commit and establishes a more searchable and understandable history. If there is no bug open at bugs.koha-community.org that addresses your contribution, please open one and describe the bug or enhancement. Then, include Bug + the bug number at the beginning of your commit message and set the "Status" field to "Needs Signoff". This helps us keep track of patches that have been contributed but not yet applied.
It is also requested that you attach the patch file to the bug report. This allows others not on the patches list to pull in your code, test, and sign-off. You can use git bz for that.
Indentation
Indentation of text-based files (.pl, .pm, .tt, .css, .js, etc.) must be 4 spaces (not tabs, not 2 or 6 or 8 spaces) You can fix tabs by using:
git config --global core.whitespace trailing-space,space-before-tab git config --global apply.whitespace fix
(from https://blog.bigballofwax.co.nz/2011/12/15/fixing-whitespace-when-apply-patches-with-git/)
If you want to reindent large amounts of existing code in connection with a bugfix or enhancement, you must do so in a separate patch from the one with code changes (unless the reindent is required due to the addition or removal of an indentation level; for example, an additional if added around a block).
Editors
Koha's source tree includes an EditorConfig file (.editorconfig). If your editor supports EditorConfig, this file will be picked and your editor will be configured (for Koha's source) so it follows the guidelines.
Most editors support EditorConfig, either natively or by using a plugin. Check your favourite editor's support here.
Specific editor tricks
Naturally, what editor is used is up to you. Some helpful things for the various ones can be listed here:
User Interface
HTML Templates
- Since version 3.4, Koha uses Template::Toolkit
- There is a way to check syntax. prove xt/author/valid-templates.t will catch unclosed [% IF %] and [% FOREACH %] and nesting problems.
- As of version 3.12 Koha uses an HTML5 doctype, and templates should validate accordingly. Templates in previous versions are expected to be valid XHTML (Transitional). For these earlier versions the only exception is some attributes which exist in HTML5 (autocomplete="off").
- HTML5 does not strictly require closing tags, but Koha templates should continue to use XHTML-style syntax tag closing, including self-closing tags.
- Web Developer Toolbar, a Firefox addon, is recommended for testing templates in the staff client. It includes the option to "Validate local HTML" using the W3C's validator.
HTML1: Template Toolkit markup inside HTML
Avoid [% IF %] inside tags because the translation tools try to read the templates as straight HTML and most TT comments inside tags probably confuse/break them. The most common example of an incorrect construction:
<option value="1" [% IF (selected) %] selected="selected" [% END %]>Yes</option>
This must be written this way:
[% IF (selected) %] <option value="1" selected="selected">Yes</option> [% ELSE %] <option value="1">Yes</option> [% END %]
HTML2: HTML inside Perl scripts
Language strings inside .pl or .pm files cannot be translated. Define variables in your script to pass to the templates for output. For example:
[% IF ( variable) %] Message to display [% END %]
HTML3: Displaying dates
If you have a date to display in the interface, just: add
[% USE KohaDates %]
at the beginning of your template and, for each date to be displayed, add
| $KohaDates
For example:
[% MyDate | $KohaDates %]
The MyDate must be provided in ISO format (that's what MySQL provides), and will be displayed as defined by the DateFormat syspref.
This coding guideline has been added in January 2012. You may find code not using it, but for new code you must use it.
HTML4: Upper and lower cases in strings
We have decided to use upper cases only for the first letter of a given sentence/label/tab description/page title/...
So you must write:
Pay fines
and not
Pay Fines
Note this rule will ease a lot translation process, as, for instance, translators will have to translate each of "Pay Fines" and "Pay fines" and "pay fines"
HTML6: A hash key with the same name as a TT virtual method
In TT, when calling a hash value, you use its key, like "borrower.name". But TT also proposes virtual methods with the same syntax, like "results.size".
The problem appears when a hash key has the same name as a TT virtual method. If hash "data" contains a key named "size":
- "data.size" will return the number of keys in the hash
- use "data.item('size')" to get the value of the key named size
The list of TT Virtual Methods.
This solution was found to correct Bug 11357. Solution came from here.
HTML7: Use system preference TT plugin
When adding a system preference check, use the Koha.Preference() method, provided by the Koha TT plugin, in the TT template. This should be used instead of passing a variable to a TT template from a Perl script using the C4::Context->preference() method.
Usage example:
[% USE Koha %] ... [%- IF Koha.Preference( 'OpacAuthorities' ) -%] code here [%- END -%]
HTML8: Use Asset TT plugin for linking JavaScript and CSS files
Usage example:
[% USE Asset %]
# create <link> tag
[% Asset.css("css/buttons.dataTables.min.css") | $raw %]
# create <script> tag
[% Asset.js("lib/jquery/plugins/dataTables.buttons.min.js") | $raw %]
HTML9: Filter all template variables
In order to prevent XSS vulnerabilities all the variables displayed in the templates must be escaped using an appropriate filter. See Bug 13618 for more information.
Not all of them are concerned by the vulnerability but to be on the safe side we are going to escape them all.
Usually you will want to use the filters provided by TT: html, uri, url, etc. And sometimes we will want to display the variable as it is, i.e. you will want to make the browser interpret the content (HTML or JavaScript), in that case you will explicitly use the raw filter, that does nothing.
Examples:
- Display a variable coming from the controller, you do not really know if it is safe or not:
[% your_variable | html %]
if your_variable contains HTML tags, they will be displayed, escaped, in the browser. So if it is contains
"<script>alert('hola');</script>"
, the html escaped version will be
"&lt;script&gt;alert('hola');&lt;script&gt;"
, which will be displayed
"<script>alert('hola');</script>"
- Display a variable that does contain HTML tags, but you want them to be interpreted by the browser. A typical use case is the system preferences (eg. StaffLoginInstructions, IntranetCirculationHomeHTML, OPACMySummaryNote, etc.)
[% Koha.Preference('OPACMySummaryNote') | $raw %]
if your variable contains html tags, they will be interpreted. So if it is contains
"<strong>something important</strong>"
, the raw version will be
"<strong>something important</strong>"
, which will be displayed "
something important
"
- Display a variable in a URL query string, using the uri filter
<a href="/a/link/to/detail.pl?param=[% param | uri %]">click me</a>
You may need to use the url filter in cases where the variable contains the URL:
- Display a variable in a URL using the url filter
<a href="[% homepage_url | url %]">Return to home</a>
- Display a safe variable
[% safe_variable | $raw %]
If and only if you are sure that the variable is safe (for instance they are integers from the DB), you can use the raw filter. It can be done later to speed up the processing, in case we are displaying hundreds/thousands of integers, or already escaped variables, in a loop.
This is not recommend if you are not certain of what you are doing!
(Note that you will need to add [% USE raw %] to the start of your template if you do want to use the raw filter plugin.)
If it is still not clear you can try and apply this patch: [1]
Click on the different link it generates and understand which ones are the correct ones.
HTML10: Set autocomplete attribute on all password fields
In a form which requesting the user's password for authentication an <input> with type "password" should have an autocomplete attribute set to "off." This helps prevent browsers from remembering passwords in form history. It might look like this:
<input type="password" name="password" id="password" size="20" autocomplete="off" />
If the page is asking the user to define a new password, the autocomplete attribute can be set to "new-password":
New password: <input type="password" name="new_password" id="new_password" size="20" autocomplete="new-password" /> Confirm new password: <input type="password" name="confirm_password" id="confirm_password" size="20" autocomplete="new-password" />
Depending on browser support, this value for the autocomplete attribute can tell the browser not to auto-fill a saved password and to suggest a new password for the user.
Notices
NOTICE1: Default Notices should now be writing using template toolkit
The notices system has supported the use of template toolkit for some time, all developments that add new notices or update existing notices should now use Template Toolkit.
Accessibility
ACC1: All OPAC pages require a single 'block' with the '.maincontent' class
Every OPAC page should contain a single .maincontent block signifying to accessibility helpers where the primary content of the page is located.. This allows for the 'Skip to main content' link to work.
<div class="maincontent">
</div>
ACC2: Input type "number" should be avoided
Using `<input type="number">` has a number of accessibility and localisation issues. Decimal delimiters and number incrementing in browsers is inconsistent and screen readers treat the input type with various levels of inconsistency and incompatability.
The current recommendation is to use:
<input type="text" inputmode="numeric">
This allows for a degree of separation between how the user enters data (“input mode”), what the browser expects the user input to contain (type equals number), and potentially how it tries to validate it.
More information can be found at: gov.uk
- Note:* A `pattern` attribute can optionally be added to limit the numeric input to just integers `pattern="\d*"` or `pattern="[0-9]*"`.
<input type="text" inputmode="numeric" pattern="\d*">
ACC3: Title elements should contain unique information information first
https://www.w3.org/WAI/tips/writing/ states that title elements should contain unique information first. This aids accessibility for all as browser titles become much more relevant and useful for navigation.
Example:
<title>Advanced editor shortcuts › Administration › Koha</title>
JavaScript
JS1: Embedding JavaScript blocks in templates
Whenever possible JavaScript should be placed in a separate file. When this is not possible (for instance, when template processing is required) JavaScript may be embedded directly in the template. Follow this structure:
<script>
...JavaScript here
</script>
Note that "//<![CDATA[" comments are no longer required now that templates use an HTML5 doctype. Existing comments should be removed as templates are updated. As of Bug 20053 the "type" attribute is no longer added to the <script> tag.
JavaScript blocks should be placed at the bottom of the template. See Coding Guidelines#JS12:_Include_javascript_at_the_end_of_template.
JS2: Enabling translation of language strings in embedded JavaScript
If your JavaScript includes strings which will be output to the user, it must be possible for them to be parsed by Koha's translation tool. Each language string must be wrapped in a special function, _(). Wherever would quote a string, "Hello world!", you must enclose it in this function: _("Hello world!"). For example:
if (f.company.value == "") {
alert(_("You must specify a name for this vendor."));
return false;
}
A more complex example:
This code is not correct:
param1 +="<\/optgroup><option value=\"newlist\">[ New List ]<\/option>"
This code is correct:
param1 +="<\/optgroup><option value=\"newlist\">[ "+_("New List")+" ]<\/option>"
This must be done any time you want linked JavaScript to output language strings to the user. Failing to do so will result in untranslated interface elements. Exception: #JS5:_Enabling_translation_of_language_strings_in_linked_JavaScript
JS3: Avoid strings which mix text and markup
Text mustn't contain HTML tags. For example, this code is not correct:
var displaytext = _("The Numbering Pattern will look like this: <br \/><ul class=\"numpattern_preview\">");
The correct form is:
var displaytext = _("The Numbering Pattern will look like this: ") + "<ul class=\"numpattern_preview\">";
JS4: Use the format() method to join variables with translatable strings
Koha's JavaScript allows you to join variables with translatable strings using a format() method:
$('#merge_status').text(_("Closed on %s").format(invoice.closedate));
During the translation process translators can restructure the sentence and move the "%s" wherever it belongs in their language. More than one placeholder can be passed:
var alert_message = _("You have chosen to move all unreceived orders from '%s' to '%s'.").format(budget_from, budget_to);
The translation tool will only parse JavaScript which is embedded in templates. Text in linked JavaScript files cannot be translated.
JS5: Enabling translation of language strings in linked JavaScript
Since Bug 21156 - Internationalization: plural forms, context, and more for JS files you can have translated strings directly defined in the .js files. Example:
__("your string to translate")
__nx("There is one item", "There are {count} items", 3, {count: 3}));
The list of the available function are in koha-tmpl/intranet-tmpl/js/i18n.js, they are the twins of the method from Koha/I18n.pm.
More examples can be found in the POD of Koha/Template/Plugin/I18N.pm.
JS6: Trailing comma in a reference list
Firefox will tolerate a trailing comma in an array literal, but Internet Explorer does not.
This is wrong:
KOHA.MyLib = {
GetUgc: function(id) {
// foo
},
ugcCallBack: function(booksInfo) {
// foo
},
};
Replacing ''},'' line with ''}'' this is correct:
KOHA.MyLib = {
GetUgc: function(id) {
// foo
},
ugcCallBack: function(booksInfo) {
// foo
}
};
JS7: Don't use <body onload=...>
<body onload> is not the best way to add load events to a page. Generally the event should be added to the jQuery $document(ready)() function.
$(document).ready(function(){
// onload function here
});
Doing so lets jQuery handle all the onload events and helps prevent conflicts. jQuery can intelligently parse and process multiple document.ready() blocks, so a linked JavaScript file can contain one and an embedded <script> block on the same page can include one too.
JS8: Follow guidelines set by ESLint
ESLint offers an online tool for checking JavaScript, as well as several options for incorporating into your workflow via many tools. Developers are encouraged to use this tool to catch errors in their JavaScript. Errors found in existing JavaScript in Koha should be corrected as part of Bug 23833 - JavaScript should conform to coding guidelines recommended by ESLint.
JS9: Avoid the use of event attributes like "onclick" to attach events
In order to separate behavior from presentation, the use of "onclick" and other event attributes should be avoided.
From the Mozilla Developer Network:
"Warning: These attributes should be avoided. This makes the markup bigger and less readable. Concerns of content/structure and behavior are not well-separated, making a bug harder to find. Furthermore, usage of event attributes almost always causes scripts to expose global functions on the Window object, polluting the global namespace."
https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Event_attributes
Instead, event handlers should be created in scripts linked to or embedded in the page. For example, instead of using "onclick:"
<input type="button" onclick="location.href = '/page/redirect/link'" value="Redirect" />;
Define an event handler in a <script> block:
<script>
$(document).ready(function(){
$("#redirect").on("click",function(e){
e.preventDefault();
location.href = '/page/redirect/link';
});
})
</script>
And add an ID to the element, or a class if you want to apply it to several elements:
<input type="button" id="redirect" value="Redirect" >
JS10: Form Validation
JavaScript-based form validation should be done using the jQuery form validation plugin unless the form validation will not work with its limitations. We are not limited to rules built into the plugin. Custom validation rules can also be defined.
Enabling form validation by form class name
Validation of required fields can be enabled in a form by adding the class "validated" to the form tag. This enables automatic validation of required fields based on markup in the form.
Required fields should have a "required" class to trigger form validation and a "required" attribute as a non-JS fallback:
<label for="firstname" class="required">First name:</label>
<input type="text" name="firstname" id="firstname" class="required" required="required" />
<span class="required">Required</span>
This will appear like this:
If you try to submit the form without filling in the field the validation plugin will add a default error:
Enabling form validation via JavaScript
If you require more complex validation rules, these can be added via JavaScript. A good example is in the validation rules for adding or editing a patron category:
$("#category_form").validate({
rules: {
categorycode: {
required: true,
letters_numbers: true
},
description: "required",
enrolmentperiod: {
required: function(element){
return $("#enrolmentperioddate").val() === "";
},
digits: true,
enrollment_period: true,
min: 1
},
enrolmentperioddate: {
required: function(element){
return $("#enrolmentperiod").val() === "";
},
enrollment_period: true
},
dateofbirthrequired: {
digits: true
},
enrolmentfee: {
number: true
},
category_type: {
required: true
}
},
messages: {
enrolmentperiod: {
required: __("Please choose an enrollment period in months OR by date.")
},
enrolmentperioddate: {
required: __("Please choose an enrollment period in months OR by date.")
}
}
});
In this example category_form is the ID of the <form> tag. In the list of validation rules, names like categorycode description refer to the ID of form fields.
The line,
description: "required",
...shows a shortcut syntax for defining a required field. The extended syntax allows more options:
categorycode: {
required: true,
letters_numbers: true
},
JS11: Using jQueryUI widgets
The jQueryUI JavaScript file included with Koha contains several modules providing different features. The jQueryUI file was created using their Build Your Download tool. Only the modules required for Koha are included.
jQueryUI modules currently in use in master:
- UI Core
- Core
- Widget
- Mouse
- Position
- Interactions
- All
- Widgets
- Autocomplete (staff client only)
- Tabs
- Datepicker
- Slider (staff client only)
New jQueryUI downloads should be built using the smoothness theme.
As new features are added to Koha which require additional jQueryUI modules, the jQueryUI JavaScript file should be regenerated by selected the existing modules and only the new required module. Unused modules should not be included.
jQueryUI CSS Customizations
jQuery widgets in Koha are styled to match Koha's other interface elements. CSS customizations to jQueryUI widgets should be put in the global CSS file for the affected interface: opac.css for the OPAC, staff-global.css for the staff client. The CSS file downloaded with jQueryUI should not be modified.
jQueryUI tabs
The jQueryUI tabs widget enables JavaScript-driven tabs in many places in the OPAC and staff client. No page-specific include is required. The jQueryUI library is globally included.
The HTML structure:
<div id="some_cutom_id" class="toptabs">
<ul>
<li><a href="#tab1">Tab 1</a></li>
<li><a href="#tab2">Tab 2</a></li>
</ul>
<div id="tab1"> Contents of tab 1 </div>
<div id="tab2"> Contents of tab 2 </div>
</div>
Note that each anchor in the unordered list corresponds to the id of the
The JavaScript initialization of the tabs:
$(document).ready(function(){
$("#some_custom_id").tabs();
});
jQueryUI Datepicker
The jQueryUI datepicker widget is part of the globally-included jQueryUI library but it requires some special initialization on each template where it is to be included. calendar.inc should be included immediately after doc-head-close.inc:
[% INCLUDE 'doc-head-close.inc' %] [% INCLUDE 'calendar.inc' %]
calendar.inc includes some reusable configuration settings and translatable strings to be used by the calendar.
When you want a form field to trigger a basic calendar popup simple add a class to the input tag:
<input type="text" size="10" id="suspend_until" name="suspend_until" class="datepicker" />
If a form field requires custom configuration the datepicker instance can be initialized in a script block:
$(document).ready(function(){
$("#newduedate").datepicker({ minDate: 1 }); // require that renewal date is after today
});
In this case you would not add the datepicker class to the input tag.
Part of the global configuration available via calendar.inc is some default behavior for linking two inputs so that the date in the first input doesn't fall after the date in second. If your template contains only one pair of linked fields you can add a special class to each one: datepickerfrom and datepickerto.
<ol>
<li>
<label for="from">From:</label>
<input type="text" id="from" name="dateduefrom" size="10" value="[% dateduefrom %]" class="datepickerfrom" />
</li>
<li>
<label for="to">To:</label>
<input type="text" id="to" name="datedueto" size="10" value="[% datedueto %]" class="datepickerto" />
</li>
</ol>
In order for this to work via the default configuration settings the id of the first input must be "from."
If you have more than one pair of inputs which need to be linked in this way you must explicitly include the configuration for each in the JavaScript on that page. Here is an example from reports/acquisition_stats.tt:
$(document).ready(function() {
// http://jqueryui.com/demos/datepicker/#date-range
var dates = $( "#from, #to" ).datepicker({
changeMonth: true,
numberOfMonths: 1,
onSelect: function( selectedDate ) {
var option = this.id == "from" ? "minDate" : "maxDate",
instance = $( this ).data( "datepicker" );
date = $.datepicker.parseDate(
instance.settings.dateFormat ||
$.datepicker._defaults.dateFormat,
selectedDate, instance.settings );
dates.not( this ).datepicker( "option", option, date );
}
});
var datesRO = $( "#fromRO, #toRO" ).datepicker({
changeMonth: true,
numberOfMonths: 1,
onSelect: function( selectedDate ) {
var option = this.id == "fromRO" ? "minDate" : "maxDate",
instance = $( this ).data( "datepicker" );
date = $.datepicker.parseDate(
instance.settings.dateFormat ||
$.datepicker._defaults.dateFormat,
selectedDate, instance.settings );
datesRO.not( this ).datepicker( "option", option, date );
}
});
});
See http://jqueryui.com/demos/datepicker/ for more information.
JS12: Include javascript at the end of template
All scripts should be at the end of template, just before:
[% INCLUDE 'intranet-bottom.inc' %]
At the begin of template set the 'footerjs' variable like this:
[% SET footerjs = 1 %]
This is important for handling system-wide included javascript correctly in header and footer.
The included javascript should look like this (example taken from tools/overduerules.tt):
[% MACRO jsinclude BLOCK %]
[% Asset.js("js/tools-menu.js") %]
<script>
var tab_map = { "1" : _("First"), "2" : _("Second"), "3" : _("Third")};
$(document).ready(function() {
$('#selectlibrary').find("input:submit").hide();
$('#branch').change(function() {
$('#selectlibrary').submit();
});
$("li>a.tab").each( function(){
var id = $(this).attr("data-number");
$(this).html(tab_map[id]);
});
$('#rulestabs').tabs();
});
</script>
[% END %]
So the page related javascript is defined as macro and after that correctly handled by js_includes.inc, for more information see bug 17418 and bug 17858
JS13: Fetching resources
[To be continued]
Bug 36084 is retrieving the work done in Vue to nicely fetch resources from endpoints in JavaScript.
See `koha-tmpl/intranet-tmpl/prog/js/fetch/api-client.js`.
An example could be:
1const client = APIClient.circulation;
2return client.checkins.create(params).then(
3 success => {
4 alert("everything went well!");
5 },
6 error => {
7 console.warn("Something wrong happened: %s".format(error));
8 }
9)
Terminology
TERM1: The agreed set of library terms must be used
See the dedicated page: Terminology.
TERM2: Gender-Neutral Pronouns
When referring to a person who could be of any gender, you should use the words they/them/their or avoid using pronouns altogether. This goes for code comments, text in templates, and strings in tests. For example, here's a string from a patrons test updated to be gender neutral.
Before:
is( $total, $enrolmentfee_K + $enrolmentfee_J, "A child growing and becoming a juvenile, he should pay " . ( $enrolmentfee_K + $enrolmentfee_J ) );
After:
is( $total, $enrolmentfee_K + $enrolmentfee_J, "A child growing and becoming a juvenile, they should pay " . ( $enrolmentfee_K + $enrolmentfee_J ) );
Gender neutral terms are preferable for a few reasons. They're more welcoming, showing that Koha expects users and contributors to be of any gender. They're also more accurate. Inappropriately using a particular gender can cause confusion, leading someone to believe that code operates differently based on the value of borrowers.sex, for instance.
TERM3: Inclusive Language
Use inclusive language that promotes a community where everyone feels they are represented. Avoid terms that express or imply negative ideas that are racist, colonialist, or otherwise biased towards groups of people. Refer to the terminology list for terms to use/avoid.
Example: Instead of using the term blacklist, use deny list which more accurately describes its function.
Database
Database Design
SQL1: FK violations in the installer
Installer SQL should not cause FOREIGN KEY violations
SQL2: SQL92 keywords
Don't use SQL92 keywords as field names
SQL3: ANSI quotes
Use SET SQL_MODE=’ANSI_QUOTES’
SQL4: Date defaults
Design for date field with NULL rather than ‘0000-00-00’
SQL5: Quoting integers
Don't use quotes around integer values:
default '0'
becomes
default 0
SQL6: Backticks
Don't use ` in table or column names (this is a mySQLism)
SQL7: Primary keys
If you add a table to Koha, the table must have a primary key, and the primary key name must be tablename_id (color_id for example)
SQL
SQL8: SQL code in .pl scripts
There must be no SQL in CGI .pl scripts (command-line scripts may include special-purpose SQL). For historical reasons there is SQL in admin/script, that's considered as a mistake. SQL is also tolerated for reports. Other than those 2 cases, all SQL must be in the C4/xxxx.pm or Koha/xxxx.pm (xxxx being the module your SQL refers to)
SQL9: SELECT
Instead of writing:
$sth=$dbh->prepare("Select borrowers.*,categories.category_type from borrowers left join categories on borrowers.categorycode=categories.categorycode where cardnumber=?");
write:
my $query = '
SELECT borrowers.*, categories.category_type
FROM borrowers
LEFT JOIN categories ON borrowers.categorycode = categories.categorycode
WHERE cardnumber = ?
';
$sth = $dbh->prepare($query);
What are the rules from this example:
- SQL reserved words must be in capital letters
- the query string must start by a "go to line", thus the query starts at the beginning of an editor line
- the query string always ends with a line containing only "';"
- 2 spaces indent for "FROM", "WHERE", "ORDER BY", "GROUP BY", "LIMIT" blocks.
- 2 more spaces for "JOINS" (in the "FROM" block) and "AND" (in the "WHERE" block)
How to write "WHERE xxx IN (23, 45, 67)"?
SQL10: Placeholders
In order to prevent SQL injection attacks, you must use placeholders (question marks) and pass the variables into the "execute" method for WHERE clauses and all other points in the SQL that pass user input directly to database queries.
my $query = '
SELECT itemnumber,
biblionumber
FROM items
WHERE biblionumber IN
(' . join(',', ('?') x @biblionumbers) . ')';
my $sth = $dbh->prepare($query);
$sth->execute(@biblionumbers);
Adding tables and fields
SQL11: All fields added to the database must be documented in kohastructure.sql
This is so that Schema Spy, and dbicdump, will provide a description of the fields. It is very easy to do. Simply add a comment at the end of each CREATE TABLE/field definition line:
DROP TABLE IF EXISTS letter;
CREATE TABLE letter (
module varchar(20) NOT NULL default '' COMMENT 'Koha module that triggers this notice or slip',
code varchar(20) NOT NULL default '' COMMENT 'unique identifier for this notice or slip',
branchcode varchar(10) default NULL COMMENT 'the branch this notice or slip is used at (branches.branchcode)',
name varchar(100) NOT NULL default '' COMMENT 'plain text name for this notice or slip',
is_html tinyint(1) default 0 COMMENT 'does this notice or slip use HTML (1 for yes, 0 for no)',
title varchar(200) NOT NULL default '' COMMENT 'subject line of the notice',
content text COMMMENT 'body text for the notice or slip',
PRIMARY KEY (module, code, branchcode)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COMMENT="table for all notice templates in Koha";
SQL12: Booleans
In order to standardize the way we handle boolean columns, and to provide a good path for serializing boolean values into JSON objects, this rules must be followed.
DB structure
Boolean fields have to be defined in kohastructure.sql as TINYINT(1) for consistency. For example:
CREATE TABLE `letter` (
...
is_html TINYINT(1) default 0 NOT NULL, -- does this notice or slip use HTML (1 for yes, 0 for no)
...
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
DBIC schema files
Booleans have to be annotated as booleans in the schema files. This annotation will be picked by Koha::Object::TO_JSON to properly handle booleans in the serialization process.
Following the above example, this is how it would be done for the letter table, in the Koha/Schema/Result/Letter.pm file:
# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-05-09 21:01:19
# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:WZxxDvP7M/Ns/e9cFseeNA
__PACKAGE__->add_columns(
'+is_html' => { is_boolean => 1 },
...
);
Perl
Here is a guide to using Koha Objects
Here is a guide to using the Koha Logger
PERL1: Perltidy
The code must be tidy using perltidy and the .perltidyrc file present in the root of the Koha directory. You must run perltidy on any new perl files added. When you add changes to existing files you should consider setting your code editor to perltidy the changed code.
See also Perltidy.
PERL2: Modern::Perl
- All scripts and modules should use Modern::Perl to enable the strict and warnings pragmas, i.e.
use Modern::Perl;
PERL3: Fix warnings
Error and warning messages produced by strict and warnings should be quelled by fixing the code, not by tacking on a no warnings directive.
PERL4: Perlcritic
All Perl submitted must validate perlcritic (level 5, the lowest one)
PERL5: Disagreements
In case of a disagreement on coding style, the perlstyle man page will decide for us
- For hash refs, use
$myhash->{mykey}, not$$myhash{mykey}
PERL6: Indentation
This rule has been superseded by Coding Guidelines#Indentation
PERL7: Definitions
The same vocabulary should be used both internally in source code ( Module names, variable names, etc ) and externally ( javascript, templates, etc ).
Canonical Koha terminology is listed on the Terminology page.
If you are developing a new feature that requires new terminology, please locate and use the matching term defined by ODLIS. If you cannot find an appropriate term via ODLIS, please bring the question to the Koha community via the Koha Developers Mailing List or the next Koha Developers IRC Meeting.
PERL8: Undefined arguments at the end of function calls
If you call a sub and don't use some arguments at the end of the sub, it's OK not to pass undef, i.e. blabla( $var1, $var2, undef, undef ) can be written as blabla( $var1, $var2 )
PERL9: Subroutine naming conventions
Koha namespace [current]
- subroutines in the Koha namespace should be snake case.
- module names should be camel case.
C4 namespace [outdated]
All sub names must contain a verb & a name. The verb describing what the sub does and the name the object it works on.
Use capital for verb initial & name initial.
For example, AddBiblio will add a biblio in the database.
common verbs :
- Add to add (NOT "New" or "Create")
- Mod to modify (NOT "Update")
- Del to delete (NOT "Remove")
- Get to read something (NOT "Read" or "Find")
If the sub works on a single object, it's singular. If it works on more than 1, it's plural. For example:
- sub GetBranch => will return a single branch
- sub GetBranches => will return information about more than 1 branch
Internal functions should begin with an underscore and contain only lower-case characters with fully-spelled out names.
For example, _koha_delete_biblio will delete a biblio record from the koha biblio table.
PERL10: Verboten subroutine parameters
- some subs have a $dbh handler as 1st parameter : remove it & use C4::Context->dbh instead
- some subs have $env as 1st parameters : remove it & use C4::Context->userenv->{variable} instead
PERL12: VERSION
- [Deprecated as of start of 3.12 release cycle] Each module should have a $VERSION variable to preserve progressive levels of compatibility with dependent code.
PERL13: POD
POD comments should include as least the following headings:
=head1 NAME
The name of the module and a very brief description of the type of functions contained in the module.
=head1 SYNOPSIS
This may give an example of how to access the module from a script, but it may also contain any general discussion of the purpose or behavior of the module's functions, and discussion of changes made to the module.
=head1 DESCRIPTION
If not included in the SYNOPSIS, a general description of the module's purpose should appear here.
=head1 FUNCTIONS
Descriptions of each function in the module, beginning with an example of how the function is called and continuing with a description of the behavior of the function.
=head1 AUTHOR
This will generally be the "Koha Development Team," but may also be an individual developer who has made significant contributions to the module. Give a contact e-mail address.
=cut
For two examples, see the POD comments in the Search.pm module (perldoc Search.pm) and the Biblio.pm module.
In addition, every subroutine must include POD as in the following:
=head2 SearchSuggestion
my @array = SearchSuggestion($user, $author, $title, $publishercode, $status, $suggestedbyme)
Searches for a suggestion that matches the supplied parameters. If you want a parameter to be ignored, set it to C<undef>.
Returns:
=over 4
=item C<@array> the suggestions found, as an array of hashes. The hashes contain the fields from the suggestions table.</nowiki>
=back
Note the status is stored twice:
* in the status field
* as parameter ( for example ASKED => 1, or REJECTED => 1) . This is for template & translation purposes.
=cut
sub SearchSuggestion {
my ($user,$author,$title,$publishercode,$status,$suggestedbyme) = @_;
...
return \@array;
}
When updating a module you are required to add POD for added and changed subroutines or ensure existing POD is accurate, it is encouraged to add POD to all subroutines as possible
PERL14: Exports
Exports from routines in C4:: and Koha:: should be kept to an absolute minimum, and Good-Practices should be followed. Ideally there should be no routines at all exported from Koha:: though this ideal may not be attainable.
PERL15: Object-oriented code and the Koha:: namespace
Whenever it makes sense, code added to the Koha:: namespace should be object-oriented. However, code that is naturally procedural should not be shoehorned into the OO style. Modules in the Koha:: namespace should not reference the C4:: namespace, with the exception of C4::Context.
Koha::Object(s)
Tables can be turned into Objects trivially with Koha Objects
Table relationship accessors should be written to allow 'prefetch'
For example one should use:
sub guarantor {
my $self = shift;
my $guarantor_rs = $self->_result->guarantorid;
return unless $guarantor_rs;
return Koha::Patron->_new_from_dbic($gaurantor_rs);
}
One should not use:
sub guarantor {
my $self = shift;
return Koha::Patrons->find( $self->guarantorid() );
}
Koha::Objects can be extended with new methods. Koha::Object(s) is the preferred way to create object-oriented ("OO") modules in Koha at this time.
Another examples:
There is a foreign key (and ON DELETE CASCADE constraint), and there is no need to test the existence:
sub biblio {
my ( $self ) = @_;
my $biblio_rs = $self->_result->biblio;
return Koha::Biblio->_new_from_dbic( $biblio_rs );
}
See also Koha::Item->current_holds.
Class::Accessor
Class::Accessor is a great way to provide access to member variables:
use base qw(Class::Accessor);
__PACKAGE__->mk_accessors(qw( member1 member2 ));
Those two lines at the top of the package are sufficient to create read/write accessors to the member variables $self->{'member1'} and $self->{'member2'} that can be accessed as follows:
warn $object->member1;
$object->member2('newvalue');
A useful idiom for ->new()
A useful idiom for the ->new() routine in object-oriented classes that do not need to process the arguments passed in as a hashref but merely need to save them for future processing:
sub new {
my ($class, $args) = @_;
$args = {} unless defined $args;
return bless ($args, $class);
}
An object with that initialization routine can be created with:
my $obj = Koha::Object->new({ 'member1' => 'value1', 'member2' => 'value2' });
PERL16: Hashrefs should be used as arguments
Rather than passing hashes, which is inefficient, or using lots of positional parameters, subroutines that require multiple arguments should use hashrefs for passing arguments, like so:
sub myroutine {
my ($args) = @_;
return $args->{'index'} . ' => ' . $args->{'value'};
}
print myroutine({ 'index' => 'kw', 'value' => 'smith' });
An example of BAD practice would be either of the following:
sub myroutine {
my (%args) = @_;
return $args{'index'} . ' => ' . $args{'value'};
}
print myroutine('index' => 'kw', 'value' => 'smith');
or
sub myroutine {
my ($index, $value) = @_;
return $index . ' => ' . $value;
}
print myroutine('kw', 'smith');
PERL17: Unit tests are required (updated Apr 26, 2017)
Unit tests must be provided for *ALL* new routines in modules, as well as for changes to existing routines (perhaps excluding very trivial edits). The scripts in t are database independent and should also pass if the database is completely empty or the SQL service is stopped. The scripts in t/db_dependent obviously need a Koha database, but should not rely on [optional] sample data as branches, currencies, patron categories, etc. As demonstrated in many existing scripts, TestBuilder can help you to create your own data.
Statement coverage should be as close to 100% as possible. In order to check test coverage, you will need to install Devel::Cover and run the following commands:
perl -MDevel::Cover ${MYTEST}
cover
This will generate a report in cover_db/coverage.html which you can review for coverage issues.
For more about unit tests and continuous integration, see Unit Tests and Continuous Integration.
PERL20: Koha namespace
Modules in the Koha namespace sould be object oriented when possible, using Koha::Object(s) as a preferred base.
PERL20.1 Use Koha::Object methods if exists
If a Koha::Object already exists, use it instead of other methods of table CRUD.
PERL21: CRUD operations
CGI scripts that handle CRUD (Create, Read, Update, Delete) operations should all be handled in the same script when possible. Example: admin/cities.pl
PERL22: Plack friendly coding
Global state variables in modules and CGI scripts are forbidden, their use elsewhere is discouraged. (see https://perl.apache.org/docs/general/perl_reference/perl_reference.html#my____Scoped_Variable_in_Nested_Subroutines for context)
PERL23: Single argument for methods and functions
Methods and functions should take a single argument: either a database ID, arrayref, or hashref with named arguments.
Simply put, if you submit a patch that is intended to improve performance, include so pre and post patch benchmarks to show the effect.
PERL25: Read only variables
Use the constant pragma. In the past Readonly was used, but it was phased out with bug 14066 and bug 16588.
PERL26: Koha::Exceptions (Approved on Apr 26, 2017)
Instead of die or croak when meeting unfavorable conditions in our code, we should raise exceptions via Koha::Exceptions.
Do not hastily add new submodules in Koha/Exceptions. First try to use existing or more generalized ones, like MissingParameter, WrongParameter, etc. When raising the exception in your code, you can pass a more detailed description to throw() or even pass additional fields (see Exception::Class).
Examples of its use can be found in e.g. Koha/Patron/Modification.pm or git grep "Koha::Exceptions". Also look at the try catch blocks (with Try::Tiny) in the subtest Exceptions of t/db_dependent/Koha/Objects.t. Here is another example:
# From Koha::Exceptions
use Exception::Class (
[...]
'Koha::Exceptions::WrongParameter' => {
isa => 'Koha::Exceptions::Exception',
description => 'One or more parameters are wrong',
},
[...]
);
# From Koha::DateUtils and sub output_pref
use Koha::Exceptions;
[...]
Koha::Exceptions::WrongParameter->throw( 'output_pref should not be called with both dt and str parameter' ) if $dt and $str;
# From t/DateUtils.t
use Try::Tiny;
[...]
try {
output_pref({ 'str' => $testdate_iso, dt => $dt, dateformat => 'iso', dateonly => 1 });
ok( 0, 'output_pref should carp if str and dt parameters are passed together' );
} catch {
is( ref($_), 'Koha::Exceptions::WrongParameter', 'output_pref should throw an exception if str and dt parameters are passed together' );
};
PERL27: Rule about returning undef from Tomas Cohen Arazi [NEW PROPOSAL dev meeting 2/2020]
PERL28: General object-oriented code guidelines [PROPOSAL FOR CHANGE dev meeting 2/2020; to be moved from PERL15]
PERL29: Direct Object Notation
Modern best practice is to always use direct object notation.
Use:
my $cgi = CGI->new;
Don't use:
my $cgi = new CGI;
Reasoning: https://www.effectiveperlprogramming.com/2020/06/turn-off-indirect-object-notation/
PERL30: Pass references when adding a new plugin hook
When implementing a new hook for Koha plugins using Koha::Plugin->call( 'method_name', $arguments ) one must ensure that $arguments are passed as a reference to ensure each plugin that implements the hook method is fed the most up to date data (which may have been altered by a previous plugin called in the execution chain).
Use:
my $new_barcode = $newdata{'cardnumber'};
Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode );
Not:
my $new_barcode = Koha::Plugins->call( 'patron_barcode_transform', $barcode );
Reasoning: If we pass a simple value, each plugin will receive just that original value and not the updated return value from the previous plugin
Security
CSRF protection
With 24.05 Koha will be fully protected against CSRF attacks. Bug 34478 (and other dependent bugs) introduced a global protection with some patterns to follow.
The main idea is to require a CSRF token whenever a stateful form is submitted (POST, PUT, DELETE, PATCH).
To enforce this requirement we have a new Plack::Middleware module (`Koha::MiddleWare::CSRF`) that will respond with a 403 if a form is submitted using one of the stateful methods without embedding a valid CSRF token. The token must be in a 'csrf_token' CGI parameter, or in a X-CSRF-TOKEN HTTP header.
Stateless requests (GET, HEAD, OPTIONS, TRACE) must not have a 'op' parameter starting with 'cud-'.
Using Javascript the token can be retrieved from a meta tag that is part of any pages: `$('meta[name="csrf-token"]').attr('content')`
There are also 2 tests to catch regressions:
- find-missing-csrf.t
All POST forms must include a CSRF token using the include file 'csrf-token.inc'
- find-missing-op-in-forms.t
All POST forms must include a 'op' parameter (starting with cud-, see bug )
Each time you switch a git branch from before/after this change, you need to copy. Within ktd you will need to `cp /kohadevbox/koha/debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi`.
Changes required for other developers or consumers
Plugins
Some plugins will need to be adjusted, and this change needs to be widely advertised.
The middleware will affect the plugins as well. The changes made in bug 34478 needs to be applied to plugins.
svc scripts
The scripts from the svc directories now require a CSRF token OR reject non-POST requests.
svc/authentication
See its own bug, bug 36094.
REST API
See bug 34451.
Current work still in progress
https://annuel.framapad.org/p/koha_34478_remaining
See bug 34478 and follow-up reports.
REST API is not covered yet, see bug 34451.
Scripts
CMD1
All new scripts that are intended to be run at the command line (cronjobs, maintenance) should use the Koha::Script base class.
use Koha::Script
use Koha::Script -cron;
This sets up some basic environment for your script behind the scenes so logging actions that may be called behind the scenes are recorded consistently.
API
The API has it's own suplimentary guidlines available at Coding Guidelines - API
Documentation
DOC1: Cookies
Documenting the use of cookies in Koha for the purpose of abiding to European GDPR and other data privacy regulations is required. If a new cookie is added, an old cookie is removed or updated, the documentation on Use of Cookies needs to be updated accordingly.
Deprecations
DEPR1: non-XSLT search results and bib display templates deprecated
As of 2014-07-02, use of the XSLT search results and bib details display templates for the staff interface and public catalog is now the preferred option. Consequently, the non-XSLT option is now deprecated and it will not be maintained.
Template files used by the non-XSLT bib and results displays need not be touched in future staff interface and public catalog enhancements; it is necessary only to update XSLT.
DEPR2: The GRS-1 Zebra indexing mode is deprecated
As of 23 July 2014, the DOM Zebra indexing mode is now the standard supported method for Koha. The GRS-1 indexing mode is deprecated for all record types and MARC flavours.
New Koha patches that update index definitions must change the DOM indexing definitions. If a patch is intended to be backported to a maintenance release, changes to GRS-1 index definitions should be kept in a separate patch.
DEPR3: C4 deprecated for new modules
New modules should be added to the Koha namespace as C4 is deprecated.
- Getting involved | Development workflow | Bug Reporting Guidelines | RFCs | Plugins | Plugin hooks
- Version Control Using Git | git bz | Commit messages | Sign off on patches | QA Test Tools | How to QA | Debugging in VIM
- Coding Guidelines | Koha Objects | Rest Api HowTo | Coding Guidelines - API | Unit Tests | Continuous Integration | Interface patterns | Database updates | Adding a syspref | Bootstrap and LESS
- Debian Packages | Building Debian Packages | Easy Package Building | Commands
- External: Dashboard | Bugzilla | Schema | perldoc | REST API | Jenkins