Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Clarified unit test requirements for deprecated methods

Deprecation is the process by which we refactor and clean up old code, getting rid of functions, classes, methods, files and resources that are no longer used.

We have deprecation guidelines because even if we clean up all uses of something we cannot just remove it, as we're an open source project and we market ourselves as such to those who use our software. It's important to assume that someone out there is using every single function, class, method, file, and resource in their own plugins and customisations.

The guidelines are to ensure that there is consistency in how we clean up code, and to ensure that those who build upon our system are given fair opportunity to upgrade their code before everything breaks. Only once something has been deprecated for period of time can it be removed from existence.

The following code structures and resources need to be properly deprecated before being removed:

  • Functions
  • Methods
  • Arguments of a function or method
  • Changes to method access
  • Files

Deprecation timeline

By default the removal of functionality will be considered for the next major release after the major release for which it was marked as deprecated. For example, a function marked as deprecated for the Totara 17 release will be removed for the Totara 18 release.

There are a few exceptions to this, most notably the deprecation policy only needs to be followed for code structures and resources that are part of the public API. If the method or property is private, or if it is marked with the PHPDoc @internal tag, or the PHPDoc comment clearly identifies it as private API, then any change can be made at any time. 

The upgrade.txt file

As of Totara 10 we have started recording all technical changes and deprecations within upgrade.txt files. Each component and plugin can have its own upgrade.txt file. The file should be located in the plugin's root folder (same location as the version.php file).

Entries to the upgrade.txt file are made at the same time as the code is changed. This means that during review and testing you should be looking for technical changes that should be noted and ensuring they are indeed noted in the upgrade.txt in that very same patch. 

In Totara 9 and below we record these details in the Changelog details field on the JIRA issue. In Totara 10 the Changelog details field is used only to provide details on the actual change, not what has changed technically.

The following is an example of the upgrade.txt file:

Code Block
This file describes API changes in Totara Connect Server,
information provided here is intended for developers.

=== 11 ===
 
* create_something() the first argument is no longer used. Functionality has not changed.
* some_class::some_method() has been deprecated, please call some_class::some_other_method() instead.
 
=== 10 ===

* create_function() added second argument $bar
* some_class has been renamed to \totara\core\some_class, please update all uses.
 

If you are making a technical change then please ensure you record a summary of that change in this file. You can put what ever technical notes you want into it, but please make sure you follow a consistent format. If the area you are making the change in does not already have an upgrade.txt file then please create one and add your notes as part of your patch. 

The debugging call

Wherever executable PHP code is deprecated, we add a call to debugging() with the so that we can confirm that the deprecated code is no longer being executed by unit and behat tests. These notices also appear in logs so that partners and third-party developers can tell when they have customisations that are going to stop working in a future version of Totara.

Include the debugging() call before any other logic in the deprecated function or method.

Code Block
languagephp
    /**
     * Description here.
     *
     * @deprecated since Totara 19.0
     * @param stdClass $context the context
     * 
     * @return bool
     */
    public function do_something($context) {
      debugging(__METHOD__ . ' has been deprecated; please use new_method() instead', DEBUG_DEVELOPER);

      return (bool) $this->new_method($context);
    }


Changing function or method arguments

The signature of a function or method must remain constant through its entire existence, existing arguments can never be removed or reused for something else, any new information required must be passed via a brand new argument added to the end of the arguments list.

It is imperative that the above is always observed, never remove an argument and never reuse an argument for something different.
Doing the above makes life incredibly hard for third party contributors and can lead to impossible to maintain code in the future. In a worst case scenario reusing an argument can lead to data loss issues and even security issues, especially for those who write plugins and customisations.

The following sections describe how to make the more common argument changes to a function or method, but please note there are many different changes that are required less frequently.
The key rule that all of these changes must observe is that any existing calls to the function must continue to work after your change. Backwards compatibility is of the utmost importance. 

Adding a new argument

When adding an argument there are a couple of key things you must ensure are done:

  1. The new argument must be added as the last argument.
    Nothing goes in the middle, always at the end. Adding to the middle is a cardinal sin! 
  2. It must have a default value.
    This is required because any existing calls to the function will not be passing this argument. 
    The default value must lead to the correct default behaviour.
  3. The version it was added to should be noted in the PHPdocs.

The following illustrates the best practice when adding an argument to a function or method.
Pay close attention to the PHPdoc syntax changes as well. 


Expand
titleClick to expand the example...

Original:

Code Block
/**
 * This function creates something.
 *
 * @param string $foo a summary of this variable.
 */
function create_something($foo) {
    // ...
}

With the added argument:

Code Block
/**
 * A one line summary
 *
 * A full description of the function or method and any details you want to share.
 *
 * @param string $foo a summary of this variable.
 * @param string $bar summary of optional second argument. 
 *     Details about the argument.
 *     Since Totara 10.
 */
function create_something($foo, $bar = null) {
    // ...
}



In upgrade.txt:

Code Block
* create_function() added second argument $bar

Notes:

  • The purpose of a function should never change. If the purpose of the function is changing then you should deprecate the old function and create a new one.
    Make the old function call the new one if that is possible.
  • If you need to add an argument that must be provided then you should not be adding an argument at all. You should be writing a new function/method. 

Removing an argument

Function arguments must never be removed.
On occasion you will encounter a situation where an argument is no longer required. Despite its place in the definition, it must never be removed, even if it is the last argument.
If you do find your changes render an argument unused then you should do the following:

  • Rename the variable to $unused or similar.
  • Make sure it has a default value of a simple type for example null or false
  • Amend the PHPdocs to reflect that this variable is unused.
  • If appropriate add a debugging call if the unused variable does not match its default value.


Expand
titleClick to expand the example...

The original function:

Code Block
/**
 * A one line summary
 *
 * A full description of the function or method and any details you want to share.
 *
 * @param string $foo a summary of this argument.
 * @param string $bar a summary of this argument.
 */
function create_something($foo, $bar) {
    // ...
}

With the removed argument:

Code Block
/**
 * A one line summary
 *
 * A full description of the function or method and any details you want to share.
 *
 * @param string $unused This argument is no longer used as of Totara 10.
 * @param string $bar a summary of this argument.
 */
function create_something($unused = null, $bar) {
    if ($unused !== null) {
        debugging('The create_something() second argument is no longer used, please review your code', DEBUG_DEVELOPER);
    }
    // ...
}



In upgrade.txt:

Code Block
* create_something() the first argument is no longer used. Functionality has not changed.

Notes:

  • The debugging call should only be present if functionality has changed. As functionality should be changed only in rare situations this debugging notice should be equally rare.
  • The purpose of a function should never change. If the purpose of the function is changing then you should deprecate the old function and create a new one.
    Make the old function call the new one if that is possible.


Changing scope; public, protected, private

On occasion you will need to change the scope of a method or property.
The direction that you are changing it determines what needs to be done.

Info

Absolutely all changes to scope MUST be mentioned in an upgrade.txt file

If you are making widening the scope, changing it to from private to protected, or protected to public, then you can make this change in code and simply note the scope change in the upgrade.txt as below:

Code Block
* some_class::some_method() is now publicly accessible. // private or protected to public.
* some_class::$some_property was changed from private to protected(). // private to protected.

If you are restricting the scope of a method or property then you also must ensure it is made publicly accessible for a period of time no less than 1 major release.

Changing a method from public to protected

On occasion you may find yourself converting a public method to either protected or private.
Typically this done to clean up an API.


Expand
titleClick to expand the example...

The original code:

Code Block
class foo {
    public function bar($a, $b = null) {
        // ...
    }
}

Converted to a protected method:

Code Block
class foo {
    public function __call($method, $arguments) {
        if ($call === 'bar') {
            // This was deprecated in Totara 10 and public access to this method will be revoked in the future.
            // Please call xxx instead.
			debugging('foo::bar() is now a protected method, please call xxx instead.', DEBUG_DEVELOPER);
            return call_user_func_array($method, $arguments);
        }
        throw new coding_exception('Call to an undefined method', $method);
    }
    protected function bar($a, $b = null) {
        // ...
    }
}


In upgrade.txt:

Code Block
* foo::bar() is now a protected method, previously it was public, please call xxx instead.

Notes:

  • __call should throw a debugging notice to inform the caller of the deprecated function as well how any public calls should be upgraded to.
  • On stable branches the function must continue to function exactly as it did previously. This is also preferable on the development branch, although some leniency can be applied in extreme edge cases.

Converting a public property to a private property

On occasion you may find yourself converting a public property to either protected or private.
Typically this done to clean up an API.


Expand
titleClick to expand the example...

The original code:

Code Block
class foo {
    public $bar;
}

Converted to a private property:

Code Block
class foo {
    private $bar;
 
    public function __get($property) {
        if ($property === 'bar') {
            // This was deprecated in Totara 10 and public access to this property will be revoked in the future.
            // Please call xxx instead.
			debugging('foo::$bar is now a private property, please call xxx instead.', DEBUG_DEVELOPER);
            return $this->{$bar};
        }
        throw new coding_exception('Request for an undefined property', $property);
    }
 
    public function __set($property, $value) {
        if ($property === 'bar') {
            // This was deprecated in Totara 10 and public access to this property will be revoked in the future.
            // Please call xxx instead.
			debugging('foo::$bar is now a private property please do not directly set $bar any more', DEBUG_DEVELOPER);
            return $this->{$bar} = $value;
        }
        throw new coding_exception('Call to an undefined property', [$property, $value]);
    }
}


In upgrade.txt:

Code Block
* foo::$bar is now a private property, previously it was public, you should never have been setting this.

Notes:

  • __get should throw a debugging notice to inform the caller of the deprecated property as well as what they should do instead.
  • __set should:
    • throw a debugging notice and inform the user of the alternative (if there is one)
    • continue to allow setting of $bar unless it is imperative that it does not, in which case it should throw a coding exception instead.


Removing a function, method or class

Its important functions, methods and classes are never immediately removed from code.
Instead they should be deprecated for at least 1 major release before being removed.
Where possible they should be kept in a functioning state during the deprecation period, to ensure any third party calls to them continue to work.
Situations where it is not possible to keep the old function, method, or class working should be extremely rare.
In these situations it is advisable to throw a coding exception, ensure that upgrade.txt is super helpful and ensure that the "Changelog details"  mentions there are API changes that are not backwards compatible.

In upgrade.txt:

Code Block
* some_function() has been deprecated, please call some_other_function() instead.
* some_class::some_method() has been deprecated, please call some_class::some_other_method() instead.
* some_class has been deprecated, please use some_other_class instead.


Removing a unit test related to a deprecated method or class

Because deprecated methods are still in the

unit test

The methods and classes are never immediately removed, they have to be removed with the code only.

Instead each method must product and can be used by third-party developers, they still need to be maintained. Therefore they need to be covered by tests. 

If all logic is removed from a deprecated method or class, so that it exists as a stub that is calling new replacement method(s) that are covered by tests, then the covering tests may be able to be removed as redundant. 

If not, then each method in the testcase which calls a deprecated method must be modified with the static::assertDebuggingCalled(...); and/or static::assertDebuggingCalledCount(...) calls;:

Code Block
languagephp
titleModifying the unit test
/**
...
* @deprecated since Totara 19.0 See mod_perform_webapi_resolver_mutation_update_activity_basic_settings instead.
*/
class mod_perform_webapi_resolver_mutation_update_activity_test extends testcase {
    ...
    // Update each test_...() method with static::assertDebuggingCalled(...)
    ...
    public function test_update_success(): void {
        [, $args] = $this->create_activity();
        /** @var activity $activity */
        ['activity' => $activity] = $this->resolve_graphql_mutation(self::MUTATION, $args);
        static::assertDebuggingCalled(
            "The mutation 'mod_perform_update_activity' had been deprecated, please use mod_perform_update_activity_basic_settings instead",
            DEBUG_DEVELOPER
        );
        // Return values should be updated
        $this->assert_base_update_result($args, $activity);
    }
    ...
}

Note that Behat features should never be trigger debugging() calls, because a properly-deprecated method is not used anywhere in our product. It would only be used by third-party customisations.

Removing a library file

Files are a little different to functions, methods and classes.

It is important that the structures present in a file are handled as per this guidelines.
The file should not be removed immediately, otherwise any code that tries to include it will encounter an error.
Instead it should be deprecated in a similar way to other structures and removed after it has been deprecated for at least one major release.


Expand
titleClick to expand the example...

Contents of the deprecated file:

Code Block
/* 
 * Standard doc block can remain here
 */
 
defined('MOODLE_INTERNAL') || die;
 
debugging('path/to/file.php has been deprecated, please remove all includes.', DEBUG_DEVELOPER);

// Remove all other code from the file.



In upgrade.txt:

Code Block
* path/to/file.php has been deprecated and should no longer be included anywhere.


Removing an entry file

Files are a little different to functions, methods and classes.

It is important that the structures present in a file are handled as per this guidelines.
The file should not be removed immediately, otherwise any code that tries to include it will encounter an error.
Instead it should be deprecated in a similar way to other structures and removed after it has been deprecated for at least one major release.


Expand
titleClick to expand the example...

Contents of the deprecated file:

Code Block
/* 
 * Standard doc block can remain here
 */
 
require('config.php');

// This file has been deprecated and is no longer used.



In upgrade.txt:

Code Block
* path/to/file.php has been deprecated and should no longer be linked to.


Renaming a class

Needs more detail - should mention the db/renamedclasses.php file and provide an example of how this is done.

In upgrade.txt:

Code Block
* some_class has been renamed to \totara\core\some_class, please update all uses.


Renaming a function

Remember a function should never be removed, as such you need to ensure the old function still works but is marked as deprecated.


Expand
titleClick to expand the example...

The original code:

Code Block
function my_func($foo, $bar) {
    global $DB;
    $obj = $DB->get_record('table', ['foo' => $foo, 'bar' => $bar], '*', MUST_EXIST);
    return $obj;
}

Converted to a private property:

Code Block
/**
 * This function has been deprecated please call core_totara_my_func() instead.
 * @deprecated since Totara 10
 */
function my_func($foo, $bar = 'default') {
    debugging('my_func() has been renamed to core_totara_my_func()', DEBUG_DEVELOPER);
    return core_totara_my_func($foo, $bar);
}
 
/**
 * Renamed from my_func()
 * @since Totara 10
 */
function core_totara_my_func($foo, $bar = 'default') {
    global $DB;
    $obj = $DB->get_record('table', ['foo' => $foo, 'bar' => $bar], '*', MUST_EXIST);
    return $obj;
}


In upgrade.txt:

Code Block
* some_function() has been renamed to totara_core_some_function(), please update all calls.

Notes:

Renaming a method

Remember a method should never be removed, as such you need to ensure the old method still works but is marked as deprecated.

Expand
titleClick to expand the example...

The original code:

Code Block
class table {
    public function has_cheers() {
        global $DB;
        return $DB->count_records('table', ['id' => $this->id]);
    }
}

Converted to a private property:

Code Block
class table {
    /**
     * This function has been deprecated please call core_totara_my_func() instead.
     * @deprecated since Totara 10
     */
    public function has_cheers() {
        debugging('table::has_cheers() has been renamed to table::has_chairs()', DEBUG_DEVELOPER);
        return $this->has_chairs();
    }
     
    /**
     * Renamed from has_cheers()
     * @since Totara 10
     */
    public function has_chairs() {
        global $DB;
        return $DB->count_records('table', ['id' => $this->id]);
    }
}


In upgrade.txt:

Code Block
* some_class::some_method() has been renamed to some_class::some_other_method(), please update all calls.


Moving a file

This should never be done, create the new file and deprecate the old file as per the "Remove file" guideline, the old file should show the debugging notice before including the new file.


Moving a function, or class to a new file

Put the function or class in the new location, and then include the new file from the old file.

This should not be done straight up, you should rename the function or class at the same time.


Changing a renderer

Renderer changes should be kept to a minimum, remember we should only be fixing bugs in stable branches, UI changes should be rare. 
Please ensure the UX team have been involved in all UI changes.

In upgrade.txt:

Code Block
Renderer changes:
* core_renderer::some_method has been modified to ensure the header is consistently printed.
* totara_core\renderer:awesome_table has been modified to meet the design spec on TL-XXXX.

In the above, the first is an example of a bug fix.
The second is an example of changing a template per a design spec. 


Changing a template

Template changes should be kept to a minimum, remember we should only be fixing bugs in stable branches, UI changes should be rare. 
Please ensure the UX team have been involved in all UI changes.

In upgrade.txt:

Code Block
Template changes:
* path/to/my_template.feature has been modified to ensure the header is consistently printed.
* path/to/other_template.feature has been modified to meet the design spec on TL-XXXX.

In the above, the first is an example of a bug fix.
The second is an example of changing a template per a design spec. 


Changing the context object used by a template

We have to be extremely cautious when changing context objects, they are a part of the public API.
As such when changing a context object we must ensure the previous object properties are kept and deprecated for a release before being removed. 

In upgrade.txt:

Code Block
Template changes:
* path/to/my_template.template context object has changed, the property ->foo->cheers has been renamed to ->foo->chairs.
* path/to/my_template.template context object has changed, the property ->foo has been deprecated please use ->table instead.


Changing a GraphQL attribute type

GraphQL attributes should not be changed. Instead create a new attribute with a new name and mark the old attribute as deprecated with a comment explaining if there's a replacement.

In schema.graphqls

Code Block
type totara_plugin_item_type {
  id: core_id!
  name: String! @deprecated(reason: "Use `new_name` instead.")
  new_name(format: core_format!): String!
}

In webapi/resolver/type/item.php

Code Block
public static function resolve(string $field, $source, array $args, execution_context $ec) {
	...
	if ($field === 'name') {
        debugging('"name" is deprecated, use "new_name" instead.', DEBUG_DEVELOPER);
    }
}

In upgrade.txt:

Code Block
* GraphQL attribute item.name has been deprecated, please update all calls to item.new_name.


Changing an existing GraphQL persisted query

Deprecation of GraphQL attributes may result in the need to modify persisted queries in the codebase in order to stop using the deprecated fields. It is acceptable to modify an existing persisted query (rather than creating a new version) with the following caveats:

  1. The change occurs in a new major release (not a point release).
  2. The change to an existing persisted query is well documented, in the upgrade.txt for the component, changelogs, and extended release notes.

Theme developers should read the major release changelogs and release notes when updating their themes to support new major versions, and fix the use of any core persisted queries that have changed.

Changing TUI framework files and components

TUI framework code can be classified into two types:

  1. TUI Core (the TUI framework itself)
  2. TUI Plugins (functional implementations that likely depend on TUI Core)

TUI Core

TUI Core is considered a public API, and should follow appropriately detailed steps as noted above for back-end deprecation. This is because TUI Plugins have many dependencies upon TUI Core, and TUI Core also has internal dependencies, e.g. Uniform and its collection of Form components and utilities.

Whole files should remain in place and be noted as deprecated in upgrade.txt and changelogs, and can be removed after being deprecated for at least one major release.

In *.js utility files

Exported methods should be marked with a @deprecated comment, and can be removed after being deprecated for at least one major release.

Code Block
/**
 * This function has been deprecated, please call myNewFunc() instead.
 * @deprecated since Totara 13
 */

In *.vue files

Similar to *.js files, marking methods, props and data structures as @deprecated and updating upgrade.txt and changelogs appropriately is also required for TUI Core components. This includes breaking changes to <template /> , <script /> , <style /> and <lang-strings />  code blocks within a Single-File Component (SFC). 

TUI Plugins

To avoid cross-plugin dependencies, TUI Plugins should ideally not depend on each other, and should only be used within the context in which they need to exist when the Plugin is developed. For example, client/component/totara_engage/src/components/sidepanel/NavigationPanel.vue should not be reused within client/component/mod_perform/src/pages/Activities.vue (because the common functionality should be abstracted into TUI Core client/component/tui/src/components/sidepanel/* ).

If there are no cross-plugin dependencies for a TUI Plugin, then the deprecation of .vue entry pages, component trees, component internal code blocks and Javascript utilities can follow a less stringent deprecation process, for example the original implementation does not need to be left in place. If there are cross-plugin dependencies then the TUI Core deprecation steps should be taken, and a refactor to abstract reusable logic and components considered for prioritisation in order to remove the cross-plugin dependency.

Theme-based TUI component overrides

Totara TXP 13+ supports per-code-block overrides as described in the TUI front-end developer documentation. This feature can reduce the amount of work involved in maintaining Vue component customisations. It does, however, introduce a dependency risk, where the TUI Core or TUI Plugin component being overridden may contain deprecations. Therefore developers will need to be conscious of changelogs, upgrade.txt entries and Git history to understand the impact of deprecation on Theme-based overrides.