[#32633] - [4.0] Helper factory
- Fixed in Code Base
- 25 Mar 2021
- Medium
- Build: 4.0-dev
- # 32633
- Diff
- Digital-Peak:j4/helper/factory
User tests: Successful: Unsuccessful:
Pull Request for pr #32291.
Summary of Changes
All over joomla are helpers manually loaded. Better to load them through a factory from the booted extension.
Testing Instructions
- Open the dashboard.
- Add the following code to the file administrator/modules/mod_quickicon/src/Helper/QuickIconHelper.php:
public static function getDataAjax()
{
$data = ['test'];
return $data;
}- Open the url /administrator/index.php?option=com_ajax&module=quickIcon&method=getData&format=json
Actual result BEFORE applying this Pull Request
- Icons are loaded.
- Ajax url shows
{"success":true,"message":null,"messages":null,"data":["test"]}
Expected result AFTER applying this Pull Request
- Icons are loaded.
- Ajax url shows
{"success":false,"message":"The file at mod_quickIcon\/helper.php does not exist.","messages":null,"data":null}
Documentation Changes Required
Should be documented on the same place where the new extension setup is documented.
Votes
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Modules Administration Front End com_ajax Libraries |
| Labels |
Added:
?
|
||
I've looked into changes and I think it looks solid.
I understand you...
It is working as described. However, there are few things:
- You do not validate and make sure the method exists before calling the method. For legacy module, we throws LogicException
$results = new LogicException(Text::sprintf('COM_AJAX_METHOD_NOT_EXISTS', $method . 'Ajax'), 404);- Look like you bypass the extension enabled check like in the legacy module:
if ($results === null && $moduleId && $table->load($moduleId) && $table->enabled)I won't comment about the architecture change here because I'm not good at it and still trying to get myself familiar with architecture changes in J4.
@joomdonation please comment inline in the code. Easier to understand.
@wilsonge do we need here also the release blocker label as the previous one had it?
| Labels |
Added:
?
|
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32633.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32633.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32633.
rdeutz
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-03-25 14:38:07 |
| Closed_By | ⇒ | rdeutz | |
| Labels |
Added:
?
Removed: ? |
||
My eyes still hurt from editing ajax.php!