the only thing that is left is a call to the parent class - does that mean you can actually remove this whole method and allow inheritance to happen, meaning the parent method will get called instead auto-magically?

@PhilETaylor No, we cannot remove it. We need to leave it here so that default ordering is a.id desc (see default values for the parameters of the method)

Are those params different in the parent method then? Sorry I cannot check quite now as I am mobile, and driving

Sent from my iPhone

On 16 May 2021, at 14:46, Tuan Pham Ngoc @.***> wrote:


@PhilETaylor No, we cannot remove it. We need to leave it here so that default ordering is a.id desc (see default values for the parameters of the method)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.

Yes, Phil. For parent method, the default value for these parameters are null

protected function populateState($ordering = null, $direction = null)

Awesome - thanks for explaining - I'll mark a test when I'm home later if still needed

Thanks Phil. Maybe we should ask maintainers to see if they want this clean up first. @wilsonge @HLeithner Do you think we should go with this code clean up?

Yeah if this works we should definitely clean it up this way. Just check what state the API filters on when your rolling this out (we can move it to the new search tools style state names - just need to make sure it's not breaking API filters)

@wilsonge The filters form within Users Action Logs still work in the same way as before. The only thing was changed is that if we remove these code, someone could not initialize value for a model state using this code (could be used before redirecting to com_actionlogs component)

Factory::getApplication()->setUserState('com_actionlogs.actionlogsfilter.search', 'initialize search value');

They will have to use new code such as

Factory::getApplication()->setUserState('com_actionlogs.actionlogs.filter', ['search' => 'initialize search value']);

We do not use any of these code in our code base. So will this change be OK?

I don't understand your idea/request below, could you please explain more what I need to check to make sure it won't break our API?

Just check what state the API filters on when your rolling this out (we can move it to the new search tools style state names - just need to make sure it's not breaking API filters)

I think this specific PR is fine. But some of the API sets these older style filters (e.g. https://github.com/joomla/joomla-cms/blob/4.0-dev/api/components/com_content/src/Controller/ArticlesController.php#L65). We'll need to clean them up - I guess thinking about it we should be able to automatically allow using these filters from the XML files rather than doing it manually like we are there...