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
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | move_token_information-2640612-11.patch | 14.71 KB | hussainweb |
| #11 | interdiff-9-11.txt | 830 bytes | hussainweb |
Comments
Comment #2
hussainwebHere 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:
Please suggest other functions which make sense here.
Comment #3
hussainwebI forgot to change one of the function calls to a method call. This patch changes it.
Comment #6
hussainwebI 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.
Comment #7
hussainwebI also tested this patch manually and it works well.
Comment #8
berdirNice work. Lets add an a docblock to the class and define an interface for those additional methods.
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.
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.
Comment #9
hussainwebAddressing 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.
Comment #11
hussainwebOops...
Comment #12
hussainwebComment #13
hussainwebComment #14
berdirFair 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
Comment #16
hussainwebThanks for all the points. I filed a follow-up to remove these functions for now: #2641132: Remove @deprecated token information functions.
Comment #17
hussainwebCreated the following issues which deal with your comments in #14.