Coding Guidelines - Koha Wiki

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

"&amp;lt;script&amp;gt;alert('hola');&amp;lt;script&amp;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 &rsaquo; Administration &rsaquo; 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 embedded in .tt files 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 embedded 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 support for the Autocomplete widget. The jQueryUI file was created using their Build Your Download tool. Only the modules required for Koha are included. Use of jQueryUI in Koha is deprecated. No new jQueryUI dependencies should be added.

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: staff-global.css for the staff client. The CSS file downloaded with jQueryUI should not be modified.

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)

JS14: Prettier JavaScript

All new JS files should be kept tidy using Prettier. Adding /* keep tidy */ to the top of the file will enforce this via QA scripts and git hooks. Older JS files should also strive to become tidier and eventually all end up with the keep tidy header line too.

yarn run prettier --trailing-comma es5 --arrow-parens avoid --write path/file.js

QA scripts are checking the following without needing the /* keep tidy */ comment:

.vue files
files in t/cypress/integration (all .ts files)
files in koha-tmpl/intranet-tmpl/prog/js/vue (.ts and .js files)

JS15: JSDoc

All new JavaScript files must include comments in JSDoc format. At a minimum, document all classes and functions to:

Provide Inline Documentation: JSDoc acts as living documentation within your code, making it easier to understand for both yourself and other developers.
Enable Tooling: JSDoc annotations allow code editors (like VSCode and Neovim) to provide:
- Inline Type Hints: Displaying argument and return types directly in your code.
- Intelligent Code Completion: Suggesting function names, arguments, and types.
- Safer Refactoring: Catching type-related errors during code changes.

Specific Requirements:

Argument and Return Types: Always document argument types using @param {type} name and return types using @returns {type}.
Type Inference: Leverage your IDE's capabilities for type inference. In VSCode, typing /** above a function will often automatically generate JSDoc with inferred types.

Concise Descriptions: When the purpose of a function or class is self-evident, focus on clear type annotations. Add descriptive comments for more complex logic.

For functions:

 1/**
 2 * Calculates the area of a rectangle.
 3 *
 4 * @param {number} width - The width of the rectangle.
 5 * @param {number} height - The height of the rectangle.
 6 * @returns {number} The calculated area.
 7 */
 8function calculateArea(width, height) {
 9    return width * height;
10}

For classes:

 1/**
 2 * Represents a person.
 3 */
 4class Person {
 5    /**
 6     * Creates a new Person instance.
 7     *
 8     * @param {string} name - The person's name.
 9     * @param {number} age - The person's age.
10     */
11    constructor(name, age) {
12        this.name = name;
13        this.age = age;
14    }
15
16    /**
17     * Returns a greeting message.
18     *
19     * @returns {string} The greeting message.
20     */
21    greet() {
22        return `Hello, my name is ${this.name} and I am ${this.age} years old.`;
23    }
24}

For Vue:

 1/**
 2 * A component that displays a greeting message.
 3 */
 4export default {
 5    props: {
 6        /**
 7         * The person's name.
 8         * 
 9         * @type {String}
10         */
11        name: {
12            type: String,
13            required: true,
14        },
15    },
16
17    data() {
18        return {
19            /**
20             * @type {Number}
21             */
22            age: 30,
23        }
24    },
25
26    methods: {
27        /**
28         * Returns a greeting message.
29         * 
30         * @return {String}
31         */
32        greet() {
33            return `Hello, ${this.name}! You are ${this.age} years old.`
34        }
35    }
36};

JS16: JavaScript form submission via links

When using a link to submit a form you should use the form-submit.js function.

 [% Asset.js("js/form-submit.js") | $raw %]

This allows you to specify several options using data elements in the link, e.g.:

<a class="btn btn-default btn-xs submit-form-link" href="#" data-adjustment_id="[% adjustment.adjustment_id | html %]" data-invoiceid="[% invoiceid | html %]" data-action="invoice.pl" data-method="post" data-op="cud-del_adj" data-confirmation-msg="[% t('Are you sure you want to delete this adjustment?') %]">

data-method should specify 'post' or 'get' - and csrf token will be automatically included for 'post'.
data-action will specify the action element.
data-confirmation-msg will display the supplied string in a confirm box.

All other data elements will be submitted as form inputs.

See bug 36246 for initial commit.

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.

Action logs

ACTN1: New additions to actions logs should use the JSON Diff format

Bug 25159 introduced a standardised diff format that should be used going forward for recording data changes in the action logs.

sub store {

    if ( $self->in_storage ) {
        my $original = $self->get_from_storage;

        . . .

        logaction( 'X', 'MODIFY', $self->reserve_id, $self, undef, $original ) if C4::Context->preference('XLog');
    }

    return $self;
}

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 },
  ...
);

SQL13: Modifying columns with existing foreign key constraints

When modifying an existing column definition in a way that changes the data type or changes the column from nullable to NOT NULL, any foreign key constraints on that column must be temporarily removed.

This is necessary because, in production environments running MySQL with strict_mode disabled, MySQL forbids column modifications that might lead to the violation of a foreign key constraint, even if the data does not actually produce any such violations. (see https://bugs.mysql.com/bug.php?id=93838#c487718)

Example (from bug 31673):

# Remove FOREIGN KEY CONSTRAINT
if ( foreign_key_exists( 'reserves', 'reserves_ibfk_4' ) ) {
    $dbh->do(q{
        ALTER TABLE reserves DROP FOREIGN KEY reserves_ibfk_4;
    });
}

# Set the NOT NULL configuration
$dbh->do(q{
    ALTER TABLE reserves
    MODIFY COLUMN `branchcode` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'foreign key from the branches table defining which branch the patron wishes to pick this hold up at'
});

# Replace the constraint
$dbh->do(q{
    ALTER TABLE reserves ADD CONSTRAINT reserves_ibfk_4 FOREIGN KEY (branchcode) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE;
});

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.

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)

.perlcriticrc contains our configuration.

To manually check for more policy violations at the expense of more noise:

perlcritic -severity 1 foo/bar.pl  # pick severity level between 1 and 5

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:

Module

=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.

Subroutine

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.
 
=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 );
}

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');

sub myroutine {
  my ($index, $value) = @_;
  return $index . ' => ' . $value;
}
 
print myroutine('kw', 'smith');

PERL17: Unit tests are required

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:

prove --exec 'perl -MDevel::Cover -Ilib' t/MyTest.t t/MyTestDir
cover # generate report

This will generate a report in cover_db/coverage.html which you can review for coverage issues.

You can run tests separately and the results will be merged in the DB. Don't forget to update the report by running `cover` after the last test.

Delete the coverage DB:

# After you finished your session.
# Or if you need to double check that Devel::Cover correctly dropped old coverage data when you changed some files between runs.
rm -rf cover_db/

Measuring coverage when interacting with the running Koha instance

Useful for:

API tests (REST, OAI, etc)
automated UI tests
manual tests!!! Yes it's possible to follow a test plan an see it's coverage 🤯
getting the full coverage of whole test suite (plan many hours of run time)

Example workflow: OAI harvester tests that use the Koha OAI server

sudo killall apache2  # Quick and dirty way to free the 8081 and 8080 port.
# Start the intranet or OPAC or both depending on what your tests interact with.
# This the leveraging the possibility to run Koha as a Mojolicious application.
perl -MDevel::Cover bin/intranet daemon -l http://*:8081
perl -MDevel::Cover bin/opac daemon -l http://*:8080

# here we want to get the coverage of both the unit/integrations tests of the
# OAI server and the tests of the harvester that connect the OAI server via the
# OPAC. Devel::Cover can collect coverage for both the test runner and the
# server the tests are hitting.
prove --exec 'perl -MDevel::Cover -Ilib' t/db_dependent/OAI t/db_dependent/Koha/OAIHarvester.t
# Go back to the terminal where you started the intranet or the OPAC (two
# terminals if both).
# Then do ctrl-c. It seems nothing happens but after maybe a minute, you will get
# output from Devel::Cover and get back to the shell.

# Only now, you can generate the report
cover

restart_all  # quick and dirty way to restart apache and get back initial state

You can also just start the OPAC and the intranet and do manual testing and get the coverage data for the action you did.

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.

PERL24: Performance and cache related patches should include benchmarks of some sort to prove their effects

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

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

PERL31: Prefer "use" than "require"

use is the preferred method of importing Perl modules. You should always use use when possible.

All use statements should appear at the top of the file.

require is allowed when use cannot be used or when it causes problems. Allowed uses of require include:

Loading modules whose name is known only at runtime (for instance, plugins)
Fixing circular dependency problems (aka "Inconsistent hierarchy during C3 merge")

If you think you need to use require for something that is not in the list above, please contact a QA team member first to confirm it's OK

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')

For templates the token can be retrieved by including the following line:

<form action="/cgi-bin/koha/action.pl" method="post">
    # include this if the form action is POST, PUT, DELETE, PATCH
    [% INCLUDE 'csrf-token.inc' %]
    # ...
</form>

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

API1: /svc api's are deprecated

Koha formally adopted REST some time ago and the original /svc api's originally intended for internal use only should not be further enhanced or new endpoints under this namespace added.

API2: REST API's should follow our API guidelines

REST API's have their own guidelines that should be followed listed on - 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.

Developer handbook

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