Problem/Motivation

There is a lot of code in helper functions to retrieve token information which makes more sense in a service. Let's move it to a service and also decide if we should replace core's service.

Proposed resolution

Replace core's service with our own which extends the core's service. This also adds helper methods to the service.

Remaining tasks

User interface changes

None

API changes

The functions may still remain or can be removed depending on what is required.

Data model changes

None

Comments

hussainweb created an issue. See original summary.

hussainweb’s picture

Status: Active » Needs review
StatusFileSize
new12.57 KB

Here is a patch which moves four functions to a service which replaces the core token service. The functions now call the service to return information and are marked deprecated. These functions are:

  • token_get_info
  • token_get_global_token_types
  • token_get_invalid_tokens_by_context
  • token_get_invalid_tokens

Please suggest other functions which make sense here.

hussainweb’s picture

StatusFileSize
new12.57 KB

I forgot to change one of the function calls to a method call. This patch changes it.

The last submitted patch, 2: move_token_information-2640612-2.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 3: move_token_information-2640612-3.patch, failed testing.

hussainweb’s picture

Status: Needs work » Needs review
StatusFileSize
new506 bytes
new12.63 KB

I got confused by a line and thought that was not needed anymore. I think that is what is causing these errors. This patch should hopefully fix it.

hussainweb’s picture

I also tested this patch manually and it works well.

berdir’s picture

Status: Needs review » Needs work
  1. +++ b/src/Token.php
    @@ -0,0 +1,192 @@
    +namespace Drupal\token;
    +
    +use Drupal\Core\Utility\Token as TokenBase;
    +
    +class Token extends TokenBase {
    

    Nice work. Lets add an a docblock to the class and define an interface for those additional methods.

  2. +++ b/src/Token.php
    @@ -0,0 +1,192 @@
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function getInfo($token_type = NULL, $token = NULL) {
    

    additional optional is tricky as it's not defined on the original class.

    I think having a service gives us a better way to do this, by having specific accessor methods for this. getTypeInfo($token_type) and getTokenInfo($token_type, $token) for example.

  3. +++ b/token.module
    @@ -353,52 +353,11 @@ function token_module_implements_alter(&$implementations, $hook) {
    + *
    + * @deprecated
      */
     function token_get_info($token_type = NULL, $token = NULL) {
    

    wondering how many of those functions we want to keep around.

    I think only very few modules use those API functions, so I'd be OK with removing them.

hussainweb’s picture

Status: Needs work » Needs review
StatusFileSize
new6.8 KB
new14.71 KB

Addressing points 1 and 2 in #8. Thanks for the review!

I am thinking we should remove those functions in a follow-up to give a smoother experience for other modules. If nothing else, the code will give an example of how they can start converting their code.

The interdiff is only for reference and can't be applied over previous patch because of the new interface file. The patch will apply, of course.

Status: Needs review » Needs work

The last submitted patch, 9: move_token_information-2640612-9.patch, failed testing.

hussainweb’s picture

Status: Needs work » Needs review
StatusFileSize
new830 bytes
new14.71 KB

Oops...

hussainweb’s picture

hussainweb’s picture

berdir’s picture

Status: Needs review » Fixed

Fair enough, we can remove the functions in a follow-up. Please create that.

Going through the module, some notes on other functions that we should consider to remove or cleaned up. Not necessarily all the in the same issue...

* token_type_load() seems to be an old D7 menu load callback, can be removed, is the same as getTokenTypeInfo() I think
* token_clear_cache() refers to a number of helper functions for static cache clears, many don't exist anymore.
* token_admin_menu_output_alter(), in case admin_menu ever gets ported to 8.x, it will very likely work very differently, so we should probably just drop it.
* token_form_alter() profile forms don't exist anymore in core, others can maybe be removed too, not sure. See also the related core issue there.
* token_field_widget_form_alter, has a wrong implements hook method name.
* left-over "Implements hook_entity_view_modes_info()"
* maybe token_get_entity_mapping() should be moved to this service too
* token_element_validate_token_context is deprecated
* token_form_system_actions_configure_alter() is unported, forms are called differently and that API function no longer exists

  • Berdir committed 0d03cf3 on 8.x-1.x authored by hussainweb
    Issue #2640612 by hussainweb: Move token information functions to a...
hussainweb’s picture

Thanks for all the points. I filed a follow-up to remove these functions for now: #2641132: Remove @deprecated token information functions.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.